| Message ID | 20250206054504.2950516-8-neilb@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | RFC: Allow concurrent and async changes in a directory | expand |
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@suse.de> > --- 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. And btw, what does this series apply to? 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. > 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 >
On Thu, 06 Feb 2025 16:42:44 +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/ > > [...] Applied to the vfs-6.15.misc branch of the vfs/vfs.git tree. Patches in the vfs-6.15.misc branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs-6.15.misc [07/19] VFS: repack LOOKUP_ bit flags. https://git.kernel.org/vfs/vfs/c/01db36d3f0da
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@suse.de> > > --- > > 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 > > >
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);
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@suse.de> --- include/linux/namei.h | 46 +++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 23 deletions(-)