[lustre-devel] Do we need LOOKUP_CONTINUE in ll_revalidate_dentry()

[lustre-devel] Do we need LOOKUP_CONTINUE in ll_revalidate_dentry()

[James Simmons]

Doing a compare to the OpenSFS branch I noticed this difference:

diff --git a/drivers/staging/lustre/lustre/llite/dcache.c 
b/drivers/staging/lustre/lustre/llite/dcache.c
index 11b82c63..6ee0ec9 100644
--- a/drivers/staging/lustre/lustre/llite/dcache.c
+++ b/drivers/staging/lustre/lustre/llite/dcache.c
@@ -254,7 +254,7 @@ static int ll_revalidate_dentry(struct dentry *dentry,
         * to this dentry, then its lock has not been revoked and the
         * path component is valid.
         */
-       if (lookup_flags & LOOKUP_PARENT)
+       if (lookup_flags & (LOOKUP_CONTINUE | LOOKUP_PARENT))
                return 1;

        /* Symlink - always valid as long as the dentry was found */

Is that needed for newer kernels?

[lustre-devel] Do we need LOOKUP_CONTINUE in ll_revalidate_dentry()

[NeilBrown]

On Mon, Nov 26 2018, James Simmons wrote:

> Doing a compare to the OpenSFS branch I noticed this difference:
>
> diff --git a/drivers/staging/lustre/lustre/llite/dcache.c 
> b/drivers/staging/lustre/lustre/llite/dcache.c
> index 11b82c63..6ee0ec9 100644
> --- a/drivers/staging/lustre/lustre/llite/dcache.c
> +++ b/drivers/staging/lustre/lustre/llite/dcache.c
> @@ -254,7 +254,7 @@ static int ll_revalidate_dentry(struct dentry *dentry,
>          * to this dentry, then its lock has not been revoked and the
>          * path component is valid.
>          */
> -       if (lookup_flags & LOOKUP_PARENT)
> +       if (lookup_flags & (LOOKUP_CONTINUE | LOOKUP_PARENT))
>                 return 1;
>
>         /* Symlink - always valid as long as the dentry was found */
>
> Is that needed for newer kernels?

LOOKUP_CONTINUE disappeared in 2011

Commit: 49084c3bb205 ("kill LOOKUP_CONTINUE")

LOOKUP_PARENT is the new LOOKUP_CONTINUE.

NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20181126/1d212677/attachment.sig>

[lustre-devel] Do we need LOOKUP_CONTINUE in ll_revalidate_dentry()

[James Simmons]

> On Mon, Nov 26 2018, James Simmons wrote:
> 
> > Doing a compare to the OpenSFS branch I noticed this difference:
> >
> > diff --git a/drivers/staging/lustre/lustre/llite/dcache.c 
> > b/drivers/staging/lustre/lustre/llite/dcache.c
> > index 11b82c63..6ee0ec9 100644
> > --- a/drivers/staging/lustre/lustre/llite/dcache.c
> > +++ b/drivers/staging/lustre/lustre/llite/dcache.c
> > @@ -254,7 +254,7 @@ static int ll_revalidate_dentry(struct dentry *dentry,
> >          * to this dentry, then its lock has not been revoked and the
> >          * path component is valid.
> >          */
> > -       if (lookup_flags & LOOKUP_PARENT)
> > +       if (lookup_flags & (LOOKUP_CONTINUE | LOOKUP_PARENT))
> >                 return 1;
> >
> >         /* Symlink - always valid as long as the dentry was found */
> >
> > Is that needed for newer kernels?
> 
> LOOKUP_CONTINUE disappeared in 2011
> 
> Commit: 49084c3bb205 ("kill LOOKUP_CONTINUE")
> 
> LOOKUP_PARENT is the new LOOKUP_CONTINUE.

So its really

if (lookup_flags & (LOOKUP_PARENT | LOOKUP_PARENT))
	return  1;

in OpenSFS branch :-/

[lustre-devel] Do we need LOOKUP_CONTINUE in ll_revalidate_dentry()

[Andreas Dilger]

On Nov 26, 2018, at 12:51, James Simmons <jsimmons@infradead.org> wrote:
> 
> 
>> On Mon, Nov 26 2018, James Simmons wrote:
>> 
>>> Doing a compare to the OpenSFS branch I noticed this difference:
>>> 
>>> diff --git a/drivers/staging/lustre/lustre/llite/dcache.c 
>>> b/drivers/staging/lustre/lustre/llite/dcache.c
>>> index 11b82c63..6ee0ec9 100644
>>> --- a/drivers/staging/lustre/lustre/llite/dcache.c
>>> +++ b/drivers/staging/lustre/lustre/llite/dcache.c
>>> @@ -254,7 +254,7 @@ static int ll_revalidate_dentry(struct dentry *dentry,
>>>         * to this dentry, then its lock has not been revoked and the
>>>         * path component is valid.
>>>         */
>>> -       if (lookup_flags & LOOKUP_PARENT)
>>> +       if (lookup_flags & (LOOKUP_CONTINUE | LOOKUP_PARENT))
>>>                return 1;
>>> 
>>>        /* Symlink - always valid as long as the dentry was found */
>>> 
>>> Is that needed for newer kernels?
>> 
>> LOOKUP_CONTINUE disappeared in 2011
>> 
>> Commit: 49084c3bb205 ("kill LOOKUP_CONTINUE")
>> 
>> LOOKUP_PARENT is the new LOOKUP_CONTINUE.
> 
> So its really
> 
> if (lookup_flags & (LOOKUP_PARENT | LOOKUP_PARENT))
> 	return  1;
> 
> in OpenSFS branch :-/

/* Kernel 3.1 kills LOOKUP_CONTINUE, LOOKUP_PARENT is equivalent to it.
 * seem kernel commit 49084c3bb2055c401f3493c13edae14d49128ca0 */
#ifndef LOOKUP_CONTINUE
#define LOOKUP_CONTINUE LOOKUP_PARENT
#endif

That commit is in the 3.0 release, so LOOKUP_CONTINUE is needed for older
kernels.  Having the same value specified twice for newer kernels is not harmful, but this isn't needed for the upstream client.

Cheers, Andreas
---
Andreas Dilger
CTO Whamcloud