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?
