Re: [PATCH 07/19] VFS: repack LOOKUP_ bit flags.

[NeilBrown]

On Thu, 06 Feb 2025, Christian Brauner wrote:
> On Thu, Feb 06, 2025 at 04:42:44PM +1100, NeilBrown wrote:
> > The LOOKUP_ bits are not in order, which can make it awkward when adding
> > new bits.  Two bits have recently been added to the end which makes them
> > look like "scoping flags", but in fact they aren't.
> > 
> > Also LOOKUP_PARENT is described as "internal use only" but is used in
> > fs/nfs/
> > 
> > This patch:
> >  - Moves these three flags into the "pathwalk mode" section
> >  - changes all bits to use the BIT(n) macro
> >  - Allocates bits in order leaving gaps between the sections,
> >    and documents those gaps.
> > 
> > Signed-off-by: NeilBrown <neilb@xxxxxxx>
> > ---
> 
> This is also a worthwhile cleanup independent of the rest of the series.
> But you've added LOOKUP_INTENT_FLAGS prior to packing the flags. Imho,
> this patch should've gone before the addition of LOOKUP_INTENT_FLAGS.

I'll fix that and submit separately - thanks.

> 
> And btw, what does this series apply to?

It was based on
Commit 92514ef226f5 ("Merge tag 'for-6.14-rc1-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux")

which was the current upstream at the time.

> Doesn't apply to next-20250206 nor to current mainline.
> I get the usual
> 
> Patch failed at 0012 VFS: enhance d_splice_alias to accommodate shared-lock updates
> error: sha1 information is lacking or useless (fs/dcache.c).
> error: could not build fake ancestor
> 
> when trying to look at this locally.

Probably your tree was missing
Commit 902e09c8acde ("fix braino in "9p: fix ->rename_sem exclusion"")

Thanks,
NeilBrown


> 
> >  include/linux/namei.h | 46 +++++++++++++++++++++----------------------
> >  1 file changed, 23 insertions(+), 23 deletions(-)
> > 
> > diff --git a/include/linux/namei.h b/include/linux/namei.h
> > index 839a64d07f8c..0d81e571a159 100644
> > --- a/include/linux/namei.h
> > +++ b/include/linux/namei.h
> > @@ -18,38 +18,38 @@ enum { MAX_NESTED_LINKS = 8 };
> >  enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> >  
> >  /* pathwalk mode */
> > -#define LOOKUP_FOLLOW		0x0001	/* follow links at the end */
> > -#define LOOKUP_DIRECTORY	0x0002	/* require a directory */
> > -#define LOOKUP_AUTOMOUNT	0x0004  /* force terminal automount */
> > -#define LOOKUP_EMPTY		0x4000	/* accept empty path [user_... only] */
> > -#define LOOKUP_DOWN		0x8000	/* follow mounts in the starting point */
> > -#define LOOKUP_MOUNTPOINT	0x0080	/* follow mounts in the end */
> > -
> > -#define LOOKUP_REVAL		0x0020	/* tell ->d_revalidate() to trust no cache */
> > -#define LOOKUP_RCU		0x0040	/* RCU pathwalk mode; semi-internal */
> > +#define LOOKUP_FOLLOW		BIT(0)	/* follow links at the end */
> > +#define LOOKUP_DIRECTORY	BIT(1)	/* require a directory */
> > +#define LOOKUP_AUTOMOUNT	BIT(2)  /* force terminal automount */
> > +#define LOOKUP_EMPTY		BIT(3)	/* accept empty path [user_... only] */
> > +#define LOOKUP_LINKAT_EMPTY	BIT(4) /* Linkat request with empty path. */
> > +#define LOOKUP_DOWN		BIT(5)	/* follow mounts in the starting point */
> > +#define LOOKUP_MOUNTPOINT	BIT(6)	/* follow mounts in the end */
> > +#define LOOKUP_REVAL		BIT(7)	/* tell ->d_revalidate() to trust no cache */
> > +#define LOOKUP_RCU		BIT(8)	/* RCU pathwalk mode; semi-internal */
> > +#define LOOKUP_CACHED		BIT(9) /* Only do cached lookup */
> > +#define LOOKUP_PARENT		BIT(10)	/* Looking up final parent in path */
> > +/* 5 spare bits for pathwalk */
> >  
> >  /* These tell filesystem methods that we are dealing with the final component... */
> > -#define LOOKUP_OPEN		0x0100	/* ... in open */
> > -#define LOOKUP_CREATE		0x0200	/* ... in object creation */
> > -#define LOOKUP_EXCL		0x0400	/* ... in target must not exist */
> > -#define LOOKUP_RENAME_TARGET	0x0800	/* ... in destination of rename() */
> > +#define LOOKUP_OPEN		BIT(16)	/* ... in open */
> > +#define LOOKUP_CREATE		BIT(17)	/* ... in object creation */
> > +#define LOOKUP_EXCL		BIT(18)	/* ... in target must not exist */
> > +#define LOOKUP_RENAME_TARGET	BIT(19)	/* ... in destination of rename() */
> >  
> >  #define LOOKUP_INTENT_FLAGS	(LOOKUP_OPEN | LOOKUP_CREATE | LOOKUP_EXCL |	\
> >  				 LOOKUP_RENAME_TARGET)
> > -
> > -/* internal use only */
> > -#define LOOKUP_PARENT		0x0010
> > +/* 4 spare bits for intent */
> >  
> >  /* Scoping flags for lookup. */
> > -#define LOOKUP_NO_SYMLINKS	0x010000 /* No symlink crossing. */
> > -#define LOOKUP_NO_MAGICLINKS	0x020000 /* No nd_jump_link() crossing. */
> > -#define LOOKUP_NO_XDEV		0x040000 /* No mountpoint crossing. */
> > -#define LOOKUP_BENEATH		0x080000 /* No escaping from starting point. */
> > -#define LOOKUP_IN_ROOT		0x100000 /* Treat dirfd as fs root. */
> > -#define LOOKUP_CACHED		0x200000 /* Only do cached lookup */
> > -#define LOOKUP_LINKAT_EMPTY	0x400000 /* Linkat request with empty path. */
> > +#define LOOKUP_NO_SYMLINKS	BIT(24) /* No symlink crossing. */
> > +#define LOOKUP_NO_MAGICLINKS	BIT(25) /* No nd_jump_link() crossing. */
> > +#define LOOKUP_NO_XDEV		BIT(26) /* No mountpoint crossing. */
> > +#define LOOKUP_BENEATH		BIT(27) /* No escaping from starting point. */
> > +#define LOOKUP_IN_ROOT		BIT(28) /* Treat dirfd as fs root. */
> >  /* LOOKUP_* flags which do scope-related checks based on the dirfd. */
> >  #define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
> > +/* 3 spare bits for scoping */
> >  
> >  extern int path_pts(struct path *path);
> >  
> > -- 
> > 2.47.1
> > 
>