Messages in this thread Patch in this message | | | Date | Mon, 30 Jan 2006 15:54:18 +0100 | From | Jan Blunck <> | Subject | Re: [PATCH] shrink_dcache_parent() races against shrink_dcache_memory() |
| |
On Mon, Jan 30, Balbir Singh wrote:
> > static inline void prune_one_dentry(struct dentry * dentry) > > { > > + struct super_block *sb = dentry->d_sb; > > struct dentry * parent; > > > > __d_drop(dentry); > > list_del(&dentry->d_u.d_child); > > dentry_stat.nr_dentry--; /* For d_free, below */ > > + sb->s_prunes++; > > dentry_iput(dentry); > > parent = dentry->d_parent; > > d_free(dentry); > > if (parent != dentry) > > dput(parent); > > spin_lock(&dcache_lock); > > + sb->s_prunes--; > > + wake_up(&sb->s_wait_prunes); > > } > > > > We can think about optimizing this to > if (!sb->sprunes) > wake_up(&sb->s_wait_prunes); >
Hardly. This is only the case when two or more shrinkers are active in parallel. If that was the case often, we would have seen this much more frequent IMHO.
> > @@ -634,8 +666,12 @@ void shrink_dcache_parent(struct dentry > > { > > int found; > > > > + again: > > while ((found = select_parent(parent)) != 0) > > prune_dcache(found); > > + > > + if (wait_on_prunes(parent->d_sb)) > > + goto again; > > } > > Is the goto again required? At this point select_parent() should have pruned > all entries, except those missed due to the race. These should be captured > by sb->s_prunes. Once the code comes out of wait_on_prunes() everything > should be ok since a dput has happened on the missed parent dentries.
Yes, because the last select_parent might returned zero because the parent of the dentry which is just pruned isn't dereferenced yet. Although we can change it to something like
do { while(select_parent()) } while(wait_on_prunes())
> > +++ linux-2.6/include/linux/fs.h > > @@ -833,6 +833,9 @@ struct super_block { > > struct list_head s_instances; > > struct quota_info s_dquot; /* Diskquota specific options */ > > > > + int s_prunes; > > Can this be an unsigned int? Perhaps you might to mention that is protected > by the dcache_lock. >
Yes, will fix that.
Regards, Jan
-- Jan Blunck jblunck@suse.de SuSE LINUX AG - A Novell company Maxfeldstr. 5 +49-911-74053-608 D-90409 Nürnberg http://www.suse.de - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |