On Sun 06-07-25 00:26:21, Al Viro wrote:
> On Sat, Jul 05, 2025 at 07:53:59PM +0100, Al Viro wrote:
> > On Sat, Jul 05, 2025 at 01:01:14AM +0100, Al Viro wrote:
> >
> > > 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).
> >
> > The following appears to work; writing docs now...
>
> More fun questions in the area: is there any reason we have mnt_want_write()
> doing
> int mnt_want_write(struct vfsmount *m)
> {
> int ret;
>
> sb_start_write(m->mnt_sb);
> ret = mnt_get_write_access(m);
> if (ret)
> sb_end_write(m->mnt_sb);
> return ret;
> }
> rather than
> int mnt_want_write(struct vfsmount *m)
> {
> int ret = mnt_get_write_access(m);
> if (!ret)
> sb_start_write(m->mnt_sb);
> return ret;
> }
>
> Note that mnt_want_write_file() on e.g. a regular file opened
> for write will have sb_start_write() done with mnt_get_write_access()
> already in place since open(2). So the nesting shouldn't be an issue
> here... The same order (mount then superblock) is used for overlayfs
> copyup, for that matter.
>
> So if it's a matter of waiting for thaw with mount write
> count already incremented, simple echo foo > pathname would already
> demonstrate the same, no matter of which order mnt_want_write() uses.
> What am I missing there?
>
> IIRC, that was Jan's stuff... <checks git log> yep - eb04c28288bb
> "fs: Add freezing handling to mnt_want_write() / mnt_drop_write()" is
> where it came from...
I was thinking about this for some time. I think it was just a pure caution
to hold as few resources as possible when waiting for fs to thaw - in
particular I think I wanted to avoid returning EBUSY when remounting
mountpoint of a frozen fs just because there's somebody blocked in
mnt_want_write() waiting for fs to thaw. And yes, when there's file open
for writing, we will wait for fs to thaw with write count elevated but in
that case EBUSY from remount is kind of expected outcome anyway. I was more
concerned about cases like mkdir etc. So overall I wouldn't say there's a
strong reason for the ordering I've chosen, it just seemed a bit more user
friendly.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
