2023-09-12 17:06:46

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH] fix writing to the filesystem after unmount

On Fri 08-09-23 12:51:03, Zdenek Kabelac wrote:
> Dne 08. 09. 23 v 12:20 Jan Kara napsal(a):
> > On Fri 08-09-23 11:29:40, Zdenek Kabelac wrote:
> > > Dne 08. 09. 23 v 9:32 Jan Kara napsal(a):
> > > > On Thu 07-09-23 14:04:51, Mikulas Patocka wrote:
> > > > > On Thu, 7 Sep 2023, Christian Brauner wrote:
> > > > >
> > > > > > > I think we've got too deep down into "how to fix things" but I'm not 100%
> > > > > > We did.
> > > > > >
> > > > > > > sure what the "bug" actually is. In the initial posting Mikulas writes "the
> > > > > > > kernel writes to the filesystem after unmount successfully returned" - is
> > > > > > > that really such a big issue?
> > > > > I think it's an issue if the administrator writes a script that unmounts a
> > > > > filesystem and then copies the underyling block device somewhere. Or a
> > > > > script that unmounts a filesystem and runs fsck afterwards. Or a script
> > > > > that unmounts a filesystem and runs mkfs on the same block device.
> > > > Well, e.g. e2fsprogs use O_EXCL open so they will detect that the filesystem
> > > > hasn't been unmounted properly and complain. Which is exactly what should
> > > > IMHO happen.
> > > I'd likely propose in this particular state of unmounting of a frozen
> > > filesystem to just proceed - and drop the frozen state together with release
> > > filesystem and never issue any ioctl from such filelsystem to the device
> > > below - so it would not be a 100% valid unmount - but since the freeze
> > > should be nearly equivalent of having a proper 'unmount' being done -? it
> > > shoudn't be causing any harm either - and? all resources associated could
> > > be 'released.? IMHO it's correct to 'drop' frozen state for filesystem
> > > that is not going to exist anymore? (assuming it's the last? such user)
> > This option was also discussed in the past and it has nasty consequences as
> > well. Cleanly shutting down a filesystem usually needs to write to the
> > underlying device so either you allow the filesystem to write to the device
> > on umount breaking assumptions of the user who froze the fs or you'd have
> > to implement a special handling for this case for every filesystem to avoid
> > the writes (and put up with the fact that the filesystem will appear as
> > uncleanly shutdown on the next mount). Not particularly nice either...
>
>
> I'd say there are several options and we should aim towards the variant
> which is most usable by normal users.
>
> Making hyper complex? unmount rule logic that basically no user-space tools
> around Gnome/KDE... are able to handle well and getting it to the position
> where only the core kernel developer have all the 'wisdom' to detect and
> decode system state and then 'know what's going on'? isn't the favourite
> goal here.

I don't think we are really making forward progress in the argument which
behavior is more or less correct or useful. But maybe when we cannot agree
on the general solution we could still improve the situation that
practically matters? E.g. disputing Gnome apps telling you you can safely
remove the USB stick when you actually cannot because the filesystem on it
is frozen is actually kind of weak argument because users that freeze
filesystem on their USB stick are practically non-existent. So is there a
usecase where users are hitting these problems in practice? Maybe some user
report that triggered original Mikulas' patch? Or was that mostly a
theoretical concern?

> Freeze should be getting the filesystem into 'consistent' state - filesystem
> should? be able to 'easily' recover and finish all the ongoing? 'unfinished'
> process with the next mount without requiring full 'fsck' - otherwise it
> would be useless for i.e. snapshot.

More or less yes but e.g. grub2 isn't able to reliably read images of just
frozen filesystem because it ignores journal contents. So if this was root
filesystem this could result in unbootable system.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR