Skip to contentSkip to conversation

dentry_stat fix

PATCH In review Maneesh Soni 2 participants 4 messages Apr 14, 2003 VFS

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 dentry_stat fix

Hello Andrew,

This patch the corrects the dentry_stat.nr_unused calculation.

In select_parent() and shrink_dcache_anon() we were not doing
any adjustments to the nr_unused count after manipulating the dentry_unused list. Now the nr_unused count is decremented if the dentry is on dentry_unused list and is removed from there.

Further in the same routines, we have to adjust the nr_unused count again if the dentry is moved to the end of d_lru list for pruning.

Regards,
Maneesh

fs/dcache.c +16 -6
diff -urN linux-2.5.67-base/fs/dcache.c linux-2.5.67-dentry_stat/fs/dcache.c
@@ -538,13 +538,18 @@
 		struct list_head *tmp = next;
 		struct dentry *dentry = list_entry(tmp, struct dentry, d_child);
 		next = tmp->next;
-		list_del_init(&dentry->d_lru);
 
-		/* don't add non zero d_count dentries 
-		 * back to d_lru list
+		if (!list_empty(&dentry->d_lru)) {
+			dentry_stat.nr_unused--;
+			list_del_init(&dentry->d_lru);
+		}
+		/* 
+		 * move only zero ref count dentries to the end 
+		 * of the unused list for prune_dcache
 		 */
 		if (!atomic_read(&dentry->d_count)) {
 			list_add(&dentry->d_lru, dentry_unused.prev);
+			dentry_stat.nr_unused++;
 			found++;
 		}
 		/*
@@ -609,13 +614,18 @@
 		spin_lock(&dcache_lock);
 		hlist_for_each(lp, head) {
 			struct dentry *this = hlist_entry(lp, struct dentry, d_hash);
-			list_del(&this->d_lru);
+			if (!list_empty(&this->d_lru)) {
+				dentry_stat.nr_unused--;
+				list_del(&this->d_lru);
+			}
 
-			/* don't add non zero d_count dentries 
-			 * back to d_lru list
+			/* 
+			 * move only zero ref count dentries to the end 
+			 * of the unused list for prune_dcache
 			 */
 			if (!atomic_read(&this->d_count)) {
 				list_add_tail(&this->d_lru, &dentry_unused);
+				dentry_stat.nr_unused++;
 				found++;
 			}
 		}
Signature · 12 lines
Maneesh Soni
IBM Linux Technology Center, 
IBM India Software Lab, Bangalore.
Phone: +91-80-5044999 email: [email protected]
http://lse.sourceforge.net/

-- 
Maneesh Soni
IBM Linux Technology Center, 
IBM India Software Lab, Bangalore.
Phone: +91-80-5044999 email: [email protected]
http://lse.sourceforge.net/
#1 lore raw
Quoted 2 lines · Maneesh Soni

This patch the corrects the dentry_stat.nr_unused calculation.

OK, I didn't even know we had a bug in there...

btw, can you explain to me why shrink_dcache_anon() and select_parent() are putting dentries at the wrong end of dentry_unused?

Do these dentries have DCACHE_REFERENCED set?

Why shouldn't they get a full two rounds of aging?

It is not clear what's going on in there. Thanks.

#2 lore raw
Quoted 8 lines · Andrew Morton
Quoted 2 lines · Maneesh Soni

This patch the corrects the dentry_stat.nr_unused calculation.

OK, I didn't even know we had a bug in there...

btw, can you explain to me why shrink_dcache_anon() and select_parent() are putting dentries at the wrong end of dentry_unused?

prune_dcache() picks up from this end in first round. It will reset the DCACHE_REFERENCED flag and will put it to the front of dentry_unused list.

Quoted 2 lines

Do these dentries have DCACHE_REFERENCED set?

They will have DCACHE_REFERENCED set as they are on the unused list. The flag get reset in the first round of aging.

Quoted 1 line

Why shouldn't they get a full two rounds of aging?

They do get.

Quoted 1 line

It is not clear what's going on in there. Thanks.

I am posting one more patch which makes the DCACHE_REFERENCED flag uses proper following this one.

Maneesh

Signature · 5 lines
Maneesh Soni
IBM Linux Technology Center, 
IBM India Software Lab, Bangalore.
Phone: +91-80-5044999 email: [email protected]
http://lse.sourceforge.net/
#3 lore raw
Quoted 4 lines · Maneesh Soni
Quoted 2 lines · Andrew Morton
Quoted 1 line · Maneesh Soni
Quoted 9 lines
Quoted 6 lines
Quoted 1 line

This patch the corrects the dentry_stat.nr_unused calculation.

OK, I didn't even know we had a bug in there...

btw, can you explain to me why shrink_dcache_anon() and select_parent() are putting dentries at the wrong end of dentry_unused?

prune_dcache() picks up from this end in first round. It will reset the DCACHE_REFERENCED flag and will put it to the front of dentry_unused list.

Sorry, but I still don't understand why they're being put at the "oldest" end of dentry_unused.

Also, shrink_dcache_anon() does:

	prune_dcache(found);

I hope we're not assuming that all the dentries which were just added will be freed? They hae the referenced bit set, and new dentries can be added...