linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] what's the reason to delay switching current->files in close_range(2)?
@ 2025-10-06 22:18 Al Viro
  0 siblings, 0 replies; only message in thread
From: Al Viro @ 2025-10-06 22:18 UTC (permalink / raw)
  To: Christian Brauner; +Cc: linux-fsdevel

In 60997c3d45d9 "close_range: add CLOSE_RANGE_UNSHARE":
    The patch makes it so that if CLOSE_RANGE_UNSHARE is requested and we do in
    fact currently share our file descriptor table we create a new private copy.
    We then close all fds in the requested range and finally after we're done we
    install the new fd table.

Is there any reason to do things in that order?  IOW, what's wrong with
replacing the descriptor table as soon as we'd done dup_fd() (unshared_fd()
in your original version) and doing the closing after that?

I'm trying to come up with reasons for that, and I don't see any, TBH...

The reason I'm asking is that doing it your way complicates the analysis
in there - e.g. __put_unused_fd(files, _) almost always has files equal to
current->files with files->file_lock held by caller; the sole exception
is __put_unused_fd() called from file_close_fd_locked(), called from
__range_close(), called from close_range(2) with CLOSE_RANGE_UNSHARE
with current->files->count greater than 1.  On that call chain and on that
call chain alone it is given the result of dup_fd() that will shortly
(and unconditionally) be installed as current->files.  ->file_lock
is, thankfully, held.

Sure, it's visible only to the caller, so we can simply adjust the
predicate to "equal to current->files or not visible in any shared
data structures", but... what's the point?

Note that delayed replacement of descriptor table does not buy us
anything in terms of concurrent modifications - another thread could
modify our descriptor table only if they had their ->files pointing
to it, which obviously won't be possible until our thread spawns
a new child with CLONE_FILES.

Am I missing something subtle here?

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2025-10-06 22:18 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-06 22:18 [RFC] what's the reason to delay switching current->files in close_range(2)? Al Viro

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).