Re: [PATCH] shrink_dcache_parent() races against shrink_dcache_memory()

From: Jan Blunck
Date: Mon Jan 30 2006 - 09:53:04 EST


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@xxxxxxx
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/