public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: linux-fsdevel@vger.kernel.org
Cc: linux-nfs@vger.kernel.org, Chuck Lever <chuck.lever@oracle.com>,
	Jeff Layton <jlayton@kernel.org>, NeilBrown <neil@brown.name>,
	Trond Myklebust <trondmy@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCHES][RFC][CFT] rpc_pipefs cleanups
Date: Fri, 13 Jun 2025 08:31:49 +0100	[thread overview]
Message-ID: <20250613073149.GI1647736@ZenIV> (raw)

	Another series pulled out of tree-in-dcache pile; it massages
rpc_pipefs to use saner primitives.  Lives in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.rpc_pipe
6.16-rc1-based, 17 commits, -314 lines of code...

Individual patches in followups.

Folks, please test and review.  In various forms this has sat in my tree
for more than a year and I'd rather get that logjam dealt with.

Overview:

	Prep/infrastructure (#1 is shared with #work.simple_recursive_removal)

1) simple_recursive_removal(): saner interaction with fsnotify
	fsnotify events are triggered before the callback, unlike in real
unlink(2)/rmdir(2) syscalls.  Obviously matters only in case when callback
is non-empty, which excludes most of the current users in the kernel.

2) new helper: simple_start_creating()
	Set the things up for kernel-initiated creation of object in a
tree-in-dcache filesystem.  With respect to locking it's an equivalent of
filename_create() - we either get a negative dentry with locked parent,
or ERR_PTR() and no locks taken.
	tracefs and debugfs had that open-coded as part of their object
creation machinery; switched to calling new helper.

      rpc_pipefs proper:

3) rpc_pipe: clean failure exits in fill_super
	->kill_sb() will be called immediately after a failure
return anyway, so we don't need to bother with notifier chain and
rpc_gssd_dummy_depopulate().  What's more, rpc_gssd_dummy_populate()
failure exits do not need to bother with __rpc_depopulate() - anything
added to the tree will be taken out by ->kill_sb().

4) rpc_{rmdir_,}depopulate(): use simple_recursive_removal() instead
	no need to give an exact list of objects to be remove when it's
simply every child of the victim directory.

5) rpc_unlink(): use simple_recursive_removal()
	Previous commit was dealing with directories; this one is for
named pipes (i.e. the thing rpc_pipefs is used for).  Note that the
callback of simple_recursive_removal() is called with the parent locked;
the victim isn't locked by the caller.

6) rpc_populate(): lift cleanup into callers
	rpc_populate() is called either from fill_super (where we
don't need to remove any files on failure - rpc_kill_sb() will take
them all out anyway) or from rpc_mkdir_populate(), where we need to
remove the directory we'd been trying to populate along with whatever
we'd put into it before we failed.  Simpler to combine that into
simple_recursive_removal() there.

7) rpc_unlink(): saner calling conventions
	* pass it pipe instead of pipe->dentry
	* zero pipe->dentry afterwards
	* it always returns 0; why bother?
	
8) rpc_mkpipe_dentry(): saner calling conventions
	Instead of returning a dentry or ERR_PTR(-E...), return 0 and
store dentry into pipe->dentry on success and return -E... on failure.
Callers are happier that way...

9) rpc_pipe: don't overdo directory locking
	Don't try to hold directories locked more than VFS requires;
lock just before getting a child to be made positive (using
simple_start_creating()) and unlock as soon as the child is created.
There's no benefit in keeping the parent locked while populating the
child - it won't stop dcache lookups anyway.

10) rpc_pipe: saner primitive for creating subdirectories
	All users of __rpc_mkdir() have the same form - start_creating(),
followed, in case of success, by __rpc_mkdir() and unlocking parent.
Combine that into a single helper, expanding __rpc_mkdir() into it, along
with the call of __rpc_create_common() in it.
	Don't mess with d_drop() + d_add() - just d_instantiate()
and be done with that.	The reason __rpc_create_common() goes for that
dance is that dentry it gets might or might not be hashed; here we know
it's hashed.

11) rpc_pipe: saner primitive for creating regular files
	rpc_new_file(); similar to rpc_new_dir(), except that here we
pass file_operations as well.  Callers don't care about dentry, just
return 0 or -E...

12) rpc_mkpipe_dentry(): switch to start_creating()
	... and make sure we set the rpc_pipe-private part of inode up
before attaching it to dentry.

13) rpc_gssd_dummy_populate(): don't bother with rpc_populate()
	Just have it create gssd (in root), clntXX in gssd, then info
and gssd in clntXX - all with explicit
rpc_new_dir()/rpc_new_file()/rpc_mkpipe_dentry().

14) rpc_pipe: expand the calls of rpc_mkdir_populate()
	... and get rid of convoluted callbacks.

15) rpc_new_dir(): the last argument is always NULL
	All callers except the one in rpc_populate() pass explicit NULL
there; rpc_populate() passes its last argument ('private') instead,
but in the only call of rpc_populate() that creates any subdirectories
(when creating fixed subdirectories of root) private itself is NULL.

16) rpc_create_client_dir(): don't bother with rpc_populate()
	not for a single file...

17) rpc_create_client_dir(): return 0 or -E...
	Callers couldn't care less which dentry did we get - anything
valid is treated as success.

             reply	other threads:[~2025-06-13  7:31 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-13  7:31 Al Viro [this message]
2025-06-13  7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
2025-06-13  7:34   ` [PATCH 02/17] new helper: simple_start_creating() Al Viro
2025-06-13 18:31     ` Jeff Layton
2025-06-13 22:36       ` Al Viro
2025-06-13 23:46         ` Jeff Layton
2025-06-13  7:34   ` [PATCH 03/17] rpc_pipe: clean failure exits in fill_super Al Viro
2025-06-13  7:34   ` [PATCH 04/17] rpc_{rmdir_,}depopulate(): use simple_recursive_removal() instead Al Viro
2025-06-13  7:34   ` [PATCH 05/17] rpc_unlink(): use simple_recursive_removal() Al Viro
2025-06-13  7:34   ` [PATCH 06/17] rpc_populate(): lift cleanup into callers Al Viro
2025-06-13  7:34   ` [PATCH 07/17] rpc_unlink(): saner calling conventions Al Viro
2025-06-13  7:34   ` [PATCH 08/17] rpc_mkpipe_dentry(): " Al Viro
2025-06-13  7:34   ` [PATCH 09/17] rpc_pipe: don't overdo directory locking Al Viro
2025-06-13  7:34   ` [PATCH 10/17] rpc_pipe: saner primitive for creating subdirectories Al Viro
2025-06-13  7:34   ` [PATCH 11/17] rpc_pipe: saner primitive for creating regular files Al Viro
2025-06-13  7:34   ` [PATCH 12/17] rpc_mkpipe_dentry(): switch to start_creating() Al Viro
2025-06-13 19:27     ` Jeff Layton
2025-06-16 19:26       ` Al Viro
2025-06-13  7:34   ` [PATCH 13/17] rpc_gssd_dummy_populate(): don't bother with rpc_populate() Al Viro
2025-06-13  7:34   ` [PATCH 14/17] rpc_pipe: expand the calls of rpc_mkdir_populate() Al Viro
2025-06-13  7:34   ` [PATCH 15/17] rpc_new_dir(): the last argument is always NULL Al Viro
2025-06-13  7:34   ` [PATCH 16/17] rpc_create_client_dir(): don't bother with rpc_populate() Al Viro
2025-06-13  7:34   ` [PATCH 17/17] rpc_create_client_dir(): return 0 or -E Al Viro
2025-06-13  8:19   ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Amir Goldstein
2025-06-13 19:32 ` [PATCHES][RFC][CFT] rpc_pipefs cleanups Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250613073149.GI1647736@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=chuck.lever@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=torvalds@linux-foundation.org \
    --cc=trondmy@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox