2011-02-11 03:49:41

by Ian Kent

[permalink] [raw]
Subject: Re: vfs-scale, general questions (Re: NFS root lockups with -next 20110113)

On Wed, 2011-01-19 at 15:43 +0900, J. R. Okajima wrote:
> Hi,
>
> Nick Piggin:
> > Thanks for your help, can you see how I've fixed it in my vfs-scale
> > tree? What do you think?
>
> Your fix is great. I have no objection at all.
> Other than the fix, here are more generic questions about vfs-scale work.
> I am happy if you reply when you have time.
>
> - getcwd(2) needs d_lock?
> It acquires rename_lock and then tests whether the pwd is removed by
> d_unhashed(). If a race condition between vfs_rename_dir() which may
> unhash/rehash the dentry happens, then getcwd() may return the wrong
> result due to unprotected d_unhashed() call, I am afraid. rename_lock
> doesn't help this case.
>
> - what is the right order of dget() and mntget()?
> If I remember correctly, someone said "mntget() first and then
> dget(). when putting, do in reverse" in the discussion when
> path_{get,put}() were born. So it is called "the right order" in the
> commit log.
> It was many years ago. Is it still true? And should rcu-walk follow it
> too? The current implementation doesn't seem to care about this order.

I didn't spot that, where did you see this?

I'm not sure about the get but I fairly sure the dput() has to be before
the mntput() because the shrink_dcache_*() cleanup routines object to
dentrys that have a reference count of more than one.

Ian


2011-02-13 02:19:49

by J. R. Okajima

[permalink] [raw]
Subject: Re: vfs-scale, general questions (Re: NFS root lockups with -next 20110113)


Ian Kent:
> > - what is the right order of dget() and mntget()?
> > If I remember correctly, someone said "mntget() first and then
> > dget(). when putting, do in reverse" in the discussion when
> > path_{get,put}() were born. So it is called "the right order" in the
> > commit log.
> > It was many years ago. Is it still true? And should rcu-walk follow it
> > too? The current implementation doesn't seem to care about this order.
>
> I didn't spot that, where did you see this?
>
> I'm not sure about the get but I fairly sure the dput() has to be before
> the mntput() because the shrink_dcache_*() cleanup routines object to
> dentrys that have a reference count of more than one.

For dget - mntget, there are several such code. For example,
nameidata_dentry_drop_rcu()
{
struct dentry *parent = nd->path.dentry;
:::
parent->d_count++;
spin_unlock(&dentry->d_lock);
spin_unlock(&parent->d_lock);
:::
mntget(nd->path.mnt);
:::

But I am not sure the "get" order is a problem.
Nick Piggin also replied and said dget and mntget is not a problem, and
I replied if I found such "put" order, I would write again.


J. R. Okajima