Re: [patch 0/5] [PATCH,RFC] vfs: per-superblock unused dentrieslist (2nd version)

[Neil Brown]

On Sunday June 18, akpm@xxxxxxxx wrote:
> On Mon, 19 Jun 2006 11:21:04 +1000
> Neil Brown <neilb@xxxxxxx> wrote:
> 
> > static void prune_dcache(int count, struct super_block *sb)
> > +static void prune_dcache(int count, struct list_head *list)
> >  {
> > +	int have_list = list != NULL;
> > +	struct list_head alt_head;
> >  	spin_lock(&dcache_lock);
> > +	if (list == NULL) {
> > +		/* use the dentry_unused list */
> > +		list_add(&alt_head, &dentry_unused);
> > +		list_del_init(&dentry_unused);
> > +		list = &alt_head;
> > +	}
> 
> This will make dentry_unused appear to be empty.
> 

Yep.  Appear.

> >  	for (; count ; count--) {
> >  		struct dentry *dentry;
> >  		struct list_head *tmp;
> > @@ -405,23 +417,11 @@ static void prune_dcache(int count, stru
> >  
> >  		cond_resched_lock(&dcache_lock);
> 
> And then it makes that apparent-emptiness globally visible.
> 

But who will look?  and will they care?

> Won't this cause concurrent unmounting or memory shrinking to malfunction?

I don't think so.

Unmounting always passes a list to prune_dcache, so dentry_unused
doesn't get emptied, so shrink_dcache_memory will not get confused.

If there are two concurrent calls to shrink_dcache_memory, then one
will find an empty list and do nothing, and it appears this is
possible - there is no locking between callers to shrink_slab.

That's probably not fatal, but it isn't ideal (I expect....).

I guess I don't need to create a separate list.  It seemed cleaner but
does have this awkwardness.
The following patch on top of the previous one changes that behaviour.

I'm wondering now if maybe we should really have two different
'prune_dcache' functions.
One that works on a private list and doesn't bother with
DCACHE_REFERENCED or s_umount, and one that works on the global list
and does the awkward stuff.  I might try that later and see what it
looks like.

NeilBrown


### Diffstat output
 ./fs/dcache.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff .prev/fs/dcache.c ./fs/dcache.c
--- .prev/fs/dcache.c	2006-06-19 11:19:29.000000000 +1000
+++ ./fs/dcache.c	2006-06-19 12:20:49.000000000 +1000
@@ -404,12 +404,10 @@ static void prune_dcache(int count, stru
 	int have_list = list != NULL;
 	struct list_head alt_head;
 	spin_lock(&dcache_lock);
-	if (list == NULL) {
+	if (list == NULL)
 		/* use the dentry_unused list */
-		list_add(&alt_head, &dentry_unused);
-		list_del_init(&dentry_unused);
-		list = &alt_head;
-	}
+		list = &dentry_unused;
+
 	for (; count ; count--) {
 		struct dentry *dentry;
 		struct list_head *tmp;
@@ -490,7 +488,8 @@ static void prune_dcache(int count, stru
 		break;
 	}
 	/* split any remaining entries back onto dentry_unused */
-	list_splice(list, dentry_unused.prev);
+	if (have_list)
+		list_splice(list, dentry_unused.prev);
 	spin_unlock(&dcache_lock);
 }
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/