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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html