Re: 2.6.38-rc2... NFS sillyrename is broken...

[Nick Piggin <npiggin@gmail.com>]

On Thu, Jan 27, 2011 at 10:59 AM, Trond Myklebust
<Trond.Myklebust@netapp.com> wrote:
> On Thu, 2011-01-27 at 10:50 +1100, Nick Piggin wrote:
>> On Thu, Jan 27, 2011 at 7:43 AM, Trond Myklebust
>> <Trond.Myklebust@netapp.com> wrote:
>> > On Wed, 2011-01-26 at 15:14 -0500, Trond Myklebust wrote:
>> >> The alternative would be to add a callback that can be called after
>> >> dentry_iput() if DCACHE_NFSFS_RENAMED is true, and that takes the parent
>> >> and (negative) dentry as the arguments.
>> >> sillyrename doesn't need the inode as an argument, but it definitely
>> >> needs the parent dentry so that it can check for races with
>> >> ->lookup()...
>> >
>> > The following (compile tested only!) patch illustrates what I mean.
>>
>> We could do this. CEPH also want a way to get d_parent in the inode
>> unlink path.
>>
>> I think I can actually check for dentry->d_count == 0 rather than
>> dentry->d_parent == NULL here, and avoid clearing d_parent
>> entirely. That might be the better solution for 2.6.38, because other
>> code I've missed might be expecting to use d_parent.
>
> I'm not sure I understand. By the time we hit d_kill() we know that
> dentry->d_count == 0.

The dcache patch to set d_parent to NULL was done so that when
walking the other way up the path we can check that the parent is
still valid.

But we could just check for d_count != 0 which means the parent has
to be valid as well (we have rename protection as well).