mbox series

[0/6,RFC,v2] tidy up various VFS lookup functions

Message ID 20250319031545.2999807-1-neil@brown.name (mailing list archive)
Headers show
Series tidy up various VFS lookup functions | expand

Message

NeilBrown March 19, 2025, 3:01 a.m. UTC
This a revised version of a previous posting.  I have dropped the change
to some lookup functions to pass a vfsmount.  I have also dropped the
changes to nfsd and cachefiles which passed a mnt_idmap other than
&nop_mnt_idmap.  Those modules now explicitly pass &nop_mnt_idmap to
some lookup functions where previously that was implicit.

============== Revised cover letter.

VFS has some functions with names containing "lookup_one_len" and others
without the "_len".  This difference has nothing to do with "len".  This
is an historical accident but can be confusing.

The functions without "_len" take a "mnt_idmap" pointer.  This is found
in the "vfsmount" and that is an important question when choosing which
to use: do you have a vfsmount, or are you "inside" the filesystem.  A
related question is "is permission checking relevant here?".

nfsd and cachefiles *do* have a vfsmount but *don't* use the non-_len
functions.  They pass nop_mnt_idmap and refuse to work on filesystems
which have any other idmap.

This series changes nfsd and cachefile to use the lookup_one family of
functions and to explictily pass &nop_mnt_idmap which is consistent with
all other vfs interfaces used where &nop_mnt_idmap is explicitly passed.

The remaining uses of the "_one" functions do not require permission
checks so these are renamed to be "_noperm" and the permission checking
is removed.

This series also changes these lookup function to take a qstr instead of
separate name and len.  In many cases this simplifies the call.

I haven't included changes to afs because there are patches in vfs.all
which make a lot of changes to lookup in afs.  I think (if they are seen
as a good idea) these patches should aim to land after the afs patches
and any further fixup in afs can happen then.

These patches are based on vfs-6.15.async.dir as they touch mkdir
related code.  There is a small conflict with the recently posted patch
to remove locking from try_lookup_one_len() calls.

Thanks,
NeilBrown

 [PATCH 1/6] VFS: improve interface for lookup_one functions
 [PATCH 2/6] nfsd: Use lookup_one() rather than lookup_one_len()
 [PATCH 3/6] cachefiles: Use lookup_one() rather than lookup_one_len()
 [PATCH 4/6] VFS: rename lookup_one_len family to lookup_noperm and
 [PATCH 5/6] Use try_lookup_noperm() instead of d_hash_and_lookup()
 [PATCH 6/6] VFS: change lookup_one_common and lookup_noperm_common to

Comments

Christian Brauner March 19, 2025, 8:42 a.m. UTC | #1
On Wed, Mar 19, 2025 at 02:01:31PM +1100, NeilBrown wrote:
> This a revised version of a previous posting.  I have dropped the change
> to some lookup functions to pass a vfsmount.  I have also dropped the

Thank you for compromising! I appreciate it.

> I haven't included changes to afs because there are patches in vfs.all
> which make a lot of changes to lookup in afs.  I think (if they are seen
> as a good idea) these patches should aim to land after the afs patches
> and any further fixup in afs can happen then.

If you're fine with this then I suggest we delay this to v6.16. So I've
moved this to the vfs-6.16.async.dir branch which won't show up in -next
before the v6.15 merge window has concluded. I'm pushing this out now.
NeilBrown March 19, 2025, 9:23 a.m. UTC | #2
On Wed, 19 Mar 2025, Christian Brauner wrote:
> On Wed, Mar 19, 2025 at 02:01:31PM +1100, NeilBrown wrote:
> > This a revised version of a previous posting.  I have dropped the change
> > to some lookup functions to pass a vfsmount.  I have also dropped the
> 
> Thank you for compromising! I appreciate it.
> 
> > I haven't included changes to afs because there are patches in vfs.all
> > which make a lot of changes to lookup in afs.  I think (if they are seen
> > as a good idea) these patches should aim to land after the afs patches
> > and any further fixup in afs can happen then.
> 
> If you're fine with this then I suggest we delay this to v6.16. So I've
> moved this to the vfs-6.16.async.dir branch which won't show up in -next
> before the v6.15 merge window has concluded. I'm pushing this out now.
> 

I'm completely find with that - thanks.

NeilBrown