All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org, Miklos Szeredi <miklos@szeredi.hu>,
	Christian Brauner <brauner@kernel.org>
Subject: Re: [RFC] MNT_WRITE_HOLD mess
Date: Sat, 5 Jul 2025 01:01:14 +0100	[thread overview]
Message-ID: <20250705000114.GU1880847@ZenIV> (raw)
In-Reply-To: <20250704202337.GT1880847@ZenIV>

On Fri, Jul 04, 2025 at 09:23:37PM +0100, Al Viro wrote:
> On Fri, Jul 04, 2025 at 12:57:39PM -0700, Linus Torvalds wrote:
> 
> > Ugh. I don't hate the concept, but if we do this, I think it needs to
> > be better abstracted out.
> > 
> > And you may be right that things like list_for_each_entry() won't
> > care, but I would not be surprised there is list debugging code that
> > could care deeply. Or if anybody uses things like "list_is_first()",
> > it will work 99+_% of the time, but then break horribly if the low bit
> > of the prev pointer is set.
> > 
> > So we obviously use the low bits of pointers in many other situations,
> > but I do think that it needs to have some kind of clear abstraction
> > and type safety to make sure that people don't use the "normal" list
> > handling helpers silently by mistake when they won't actually work.
> 
> Point, but in this case I'd be tempted to turn the damn thing into
> pointer + unsigned long right in the struct mount, and deal with it
> explicitly.  And put a big note on it, along the lines of "we might want
> to abstract that someday".
> 
> Backporting would be easier that way, if nothing else...

FWIW, several observations around that thing:
	* mnt_get_write_access(), vfs_create_mount() and clone_mnt()
definitely do not need to touch the seqcount component of mount_lock.
read_seqlock_excl() is enough there.
	* AFAICS, the same goes for sb_prepare_remount_readonly(),
do_remount() and do_reconfigure_mnt() - no point bumping the seqcount
side of mount_lock there; only spinlock is needed.
	* failure exit in mount_setattr_prepare() needs only clearing the
bit; smp_wmb() is pointless there (especially done for each mount involved).
	* both vfs_create_mount() and clone_mnt() add mount to the tail
of ->s_mounts.  Is there any reason why that would be better than adding
to head?  I don't remember if that had been covered back in 2010/2011
discussion of per-mount r/o patchset; quick search on lore hasn't turned
up anything...  Miklos?

  reply	other threads:[~2025-07-05  0:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-04 19:44 [RFC] MNT_WRITE_HOLD mess Al Viro
2025-07-04 19:57 ` Linus Torvalds
2025-07-04 20:23   ` Al Viro
2025-07-05  0:01     ` Al Viro [this message]
2025-07-05  4:52       ` Miklos Szeredi
2025-07-05  5:57         ` Al Viro
2025-07-05  8:16           ` Al Viro
2025-07-05 18:53       ` Al Viro
2025-07-05 23:26         ` Al Viro
2025-07-07 10:26           ` Jan Kara
2025-07-06  1:26         ` [PATCH] fix a mount write count leak in ksmbd_vfs_kern_path_locked() (was Re: [RFC] MNT_WRITE_HOLD mess) Al Viro
2025-07-06  3:49           ` Namjae Jeon
2025-07-07  8:24 ` [RFC] MNT_WRITE_HOLD mess Christian Brauner

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=20250705000114.GU1880847@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=brauner@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=torvalds@linux-foundation.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.