Question: d_revalidate in rcu-walk

Question: d_revalidate in rcu-walk

[shyodx1989]

Sorry for the noise, the original email was rejected by linux-fsdevel server since I forget to send
it in plain text mode :(

At 2016-09-03 18:46:05, "shyodx1989" <shyodx1989@163.com> wrote:
Hi, folks,

When I read rcu-walk code, I'm confused about the scenario of returning -ECHILD in d_revalidate
of local filesystem. path-walk.txt says that "inode is also RCU protected so we can load d_inode
and use the inode for limited things", and i_mode, i_uid, i_gid and i_op can be tested or loaded.

But most filesystems, which have d_revalidate, return -ECHILD if LOOKUP_RCU is set instead of
checking if it is safe for rcu-walk. However commit 5c0f360b083fb33d05d1bff4b138b82d715eb419
"jfs_ci_revalidate() is safe from RCU mode" removes the check. So why is jfs_ci_revalidate safe in
RCU  mode and if we only check d_inode (like the following code), what should we care about to
tell if d_revalidate is safe for rcu-walk or not and? Ref-walk is much slower than rcu-walk, maybe
it's better not to return -ECHILD directly if not necessary.

d_revalidate(dentry, flags)
{
        if (flags & LOOKUP_RCU)
                return -ECHILD
        if (!d_inode_rcu(dentry))
                return 0;
        return 1;
}

thanks,
Sheng

Re: Question: d_revalidate in rcu-walk

[Al Viro]

On Sat, Sep 03, 2016 at 06:46:05PM +0800, shyodx1989 wrote:

> 
> But most filesystems, which have d_revalidate, return -ECHILD if LOOKUP_RCU is set instead of
> checking if it is safe for rcu-walk.

For a good and simple reason that the work they would have to do in their
->d_revalidate() can't be done without blocking.  Which can't be done under
rcu_read_lock(), thus the "sorry, you have to leave RCU mode for that", aka
-ECHILD.

>However commit 5c0f360b083fb33d05d1bff4b138b82d715eb419
> "jfs_ci_revalidate() is safe from RCU mode" removes the check. So why is jfs_ci_revalidate safe in
> RCU  mode

Because JFS ->d_revalidate() doesn't need anything blocking.

> and if we only check d_inode (like the following code), what should we care about to
> tell if d_revalidate is safe for rcu-walk or not and? Ref-walk is much slower than rcu-walk, maybe
> it's better not to return -ECHILD directly if not necessary.
> 
> 
> d_revalidate(dentry, flags)
> {
>         if (flags & LOOKUP_RCU)
>                 return -ECHILD
>         if (!d_inode_rcu(dentry))
>                 return 0;
>         return 1;
> }

Huh?  Which filesystem would that be?  Sure, in such case -ECHILD is pointless;
who does that?  Some of them might be possible to drop ECHILD, but that needs
some care.  Note, BTW, that things like dput() are blocking, so the things like
trying to grab parent, etc. can get tricky.

Which ->d_revalidate() instance do you have in mind?

Re:Re: Question: d_revalidate in rcu-walk

[shyodx1989]

At 2016-09-03 21:03:12, "Al Viro" <viro@ZenIV.linux.org.uk> wrote:
>On Sat, Sep 03, 2016 at 06:46:05PM +0800, shyodx1989 wrote:
>
>> 
>> But most filesystems, which have d_revalidate, return -ECHILD if LOOKUP_RCU is set instead of
>> checking if it is safe for rcu-walk.
>
>For a good and simple reason that the work they would have to do in their
>->d_revalidate() can't be done without blocking.  Which can't be done under
>rcu_read_lock(), thus the "sorry, you have to leave RCU mode for that", aka
>-ECHILD.
>
>>However commit 5c0f360b083fb33d05d1bff4b138b82d715eb419
>> "jfs_ci_revalidate() is safe from RCU mode" removes the check. So why is jfs_ci_revalidate safe in
>> RCU  mode
>
>Because JFS ->d_revalidate() doesn't need anything blocking.
>
>> and if we only check d_inode (like the following code), what should we care about to
>> tell if d_revalidate is safe for rcu-walk or not and? Ref-walk is much slower than rcu-walk, maybe
>> it's better not to return -ECHILD directly if not necessary.
>> 
>> 
>> d_revalidate(dentry, flags)
>> {
>>         if (flags & LOOKUP_RCU)
>>                 return -ECHILD
>>         if (!d_inode_rcu(dentry))
>>                 return 0;
>>         return 1;
>> }
>
>Huh?  Which filesystem would that be?  Sure, in such case -ECHILD is pointless;
>who does that?  Some of them might be possible to drop ECHILD, but that needs
>some care.  Note, BTW, that things like dput() are blocking, so the things like
>trying to grab parent, etc. can get tricky.
>
>Which ->d_revalidate() instance do you have in mind?

Re: Question: d_revalidate in rcu-walk

[Sheng Yong]

Thanks for your reply. There is something wrong with my mail client. It 
sent an empty email which I don't know why :( I hope it works this time.
在 2016/9/3 21:03, Al Viro 写道:
> On Sat, Sep 03, 2016 at 06:46:05PM +0800, shyodx1989 wrote:
>
>>
>> But most filesystems, which have d_revalidate, return -ECHILD if LOOKUP_RCU is set instead of
>> checking if it is safe for rcu-walk.
>
> For a good and simple reason that the work they would have to do in their
> ->d_revalidate() can't be done without blocking.  Which can't be done under
> rcu_read_lock(), thus the "sorry, you have to leave RCU mode for that", aka
> -ECHILD.

So, basically, ECHILD does not care about wheter d_inode, d_flags or 
other d_xxx members are updated or invalidated. If d_revalidate is not 
blocked, thus the task will not be switched out to break grace period, 
it is safe to call d_revalidate in RCU mode. And during rcu-walk, the 
dentry may be out of date, the seqcount could help to detect if someone 
is updating or using the dentry.

>
>> However commit 5c0f360b083fb33d05d1bff4b138b82d715eb419
>> "jfs_ci_revalidate() is safe from RCU mode" removes the check. So why is jfs_ci_revalidate safe in
>> RCU  mode
>
> Because JFS ->d_revalidate() doesn't need anything blocking.
>
>> and if we only check d_inode (like the following code), what should we care about to
>> tell if d_revalidate is safe for rcu-walk or not and? Ref-walk is much slower than rcu-walk, maybe
>> it's better not to return -ECHILD directly if not necessary.
>>
>>
>> d_revalidate(dentry, flags)
>> {
>>          if (flags & LOOKUP_RCU)
>>                  return -ECHILD
>>          if (!d_inode_rcu(dentry))
>>                  return 0;
>>          return 1;
>> }
>
> Huh?  Which filesystem would that be?  Sure, in such case -ECHILD is pointless;
> who does that?  Some of them might be possible to drop ECHILD, but that needs
> some care.  Note, BTW, that things like dput() are blocking, so the things like
> trying to grab parent, etc. can get tricky.

In fact, there are no filesystems do that. I just wanted to revalidate 
negative dentries during lookup for my own test, and was wondering why 
JFS behaves different with other local filesystems.

thanks,
Sheng

>
> Which ->d_revalidate() instance do you have in mind?
>