2022-05-11 09:20:35

by Daniil Lunev

[permalink] [raw]
Subject: [PATCH 0/2] Prevent re-use of FUSE superblock after force unmount

Force unmount of fuse severes the connection between FUSE driver and its
userspace counterpart. However, open file handles will prevent the
superblock from being reclaimed. An attempt to remount the filesystem at
the same endpoint will try re-using the superblock, if still present.
Since the superblock re-use path doesn't go through the fs-specific
superblock setup code, its state in FUSE case is already disfunctional,
and that will prevent the mount from succeeding.
The patchset adds a possibility to mark the superblock "defunc", which
will prevent its re-use by the subsequent mounts, and uses the
functionality in FUSE driver.


Daniil Lunev (2):
fs/super: Add a flag to mark super block defunc
FUSE: Mark super block defunc on force unmount

fs/fuse/inode.c | 11 +++++++++--
fs/super.c | 4 ++--
include/linux/fs.h | 1 +
3 files changed, 12 insertions(+), 4 deletions(-)

--
2.31.0



2022-05-11 10:00:40

by Daniil Lunev

[permalink] [raw]
Subject: [PATCH 2/2] FUSE: Mark super block defunc on force unmount

Force unmount of FUSE severes the connection with the user space, even
if there are still open files. Subsequent remount tries to re-use the
superblock held by the open files, which is meaningless in the FUSE case
after disconnect - reused super block doesn't have userspace counterpart
attached to it and is incapable of doing any IO.

Signed-off-by: Daniil Lunev <[email protected]>
---

fs/fuse/inode.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 8c0665c5dff88..e2ad3c9b2d5c5 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -476,8 +476,15 @@ static void fuse_umount_begin(struct super_block *sb)
{
struct fuse_conn *fc = get_fuse_conn_super(sb);

- if (!fc->no_force_umount)
- fuse_abort_conn(fc);
+ if (fc->no_force_umount)
+ return;
+
+ sb->s_defunc = true;
+ if (sb->s_bdi != &noop_backing_dev_info) {
+ bdi_put(sb->s_bdi);
+ sb->s_bdi = &noop_backing_dev_info;
+ }
+ fuse_abort_conn(fc);
}

static void fuse_send_destroy(struct fuse_mount *fm)
--
2.31.0


2022-05-11 11:27:59

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH 0/2] Prevent re-use of FUSE superblock after force unmount

On Wed, 11 May 2022 at 11:37, Daniil Lunev <[email protected]> wrote:
>
> > No progress has been made in the past decade with regard to suspend.
> > I mainly put that down to lack of interest.
> >
> That is unfortunate.
>
> > It is a legitimate operation, but one that is not guaranteed to leave
> > the system in a clean state.
> Sure, I don't think I can argue about it. The current behaviour is a problem,
> however, since there is no other way to ensure the system can suspend
> reliably but force unmount - we try normal unmount first and proceed with
> force if that fails. Do you think that the approach proposed in this patchset
> is a reasonable path to mitigate the issue?

At a glance it's a gross hack. I can think of more than one way in
which this could be achieved without adding a new field to struct
super_block.

But... what I'd really prefer is if the underlying issue of fuse vs.
suspend was properly addressed instead of adding band-aids. And that
takes lots more resources, for sure, and the result is not guaranteed.
But you could at least give it a try.

Thanks,
Miklos

2022-05-11 20:57:12

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH 0/2] Prevent re-use of FUSE superblock after force unmount

On Wed, 11 May 2022 at 13:19, Daniil Lunev <[email protected]> wrote:
>
> > At a glance it's a gross hack. I can think of more than one way in
> > which this could be achieved without adding a new field to struct
> > super_block.
> Can you advise what would be a better way to achieve that?

I think it would be easiest to remove the super block from the
type->fs_supers list.

Thanks,
Miklos

2022-05-11 22:05:17

by Daniil Lunev

[permalink] [raw]
Subject: Re: [PATCH 0/2] Prevent re-use of FUSE superblock after force unmount

> At a glance it's a gross hack. I can think of more than one way in
> which this could be achieved without adding a new field to struct
> super_block.
Can you advise what would be a better way to achieve that?

> But... what I'd really prefer is if the underlying issue of fuse vs.
> suspend was properly addressed instead of adding band-aids. And that
> takes lots more resources, for sure, and the result is not guaranteed.
> But you could at least give it a try.
We do have a limited success with userspace level sequencing of processes,
but on the kernel level - it is all quite untrivial, as you mentioned.
I did some
research, and what I found pretty much a 9 years old thread which went
nowhere at the end [1]. We would also prefer if suspend just worked (and
we have a person looking into what is actually breaking with suspend), but
there is an unbounded amount of time for how long the investigation and
search for a solution may be ongoing given the complexity of the problem,
and in the meantime there is no way to work around the problem.

Thanks,
Daniil

[1] https://linux-kernel.vger.kernel.narkive.com/UeBWfN1V/patch-fuse-make-fuse-daemon-frozen-along-with-kernel-threads

2022-05-12 09:15:31

by Bernd Schubert

[permalink] [raw]
Subject: Re: [PATCH 0/2] Prevent re-use of FUSE superblock after force unmount



On 5/11/22 13:19, Daniil Lunev wrote:
>> At a glance it's a gross hack. I can think of more than one way in
>> which this could be achieved without adding a new field to struct
>> super_block.
> Can you advise what would be a better way to achieve that?
>
>> But... what I'd really prefer is if the underlying issue of fuse vs.
>> suspend was properly addressed instead of adding band-aids. And that
>> takes lots more resources, for sure, and the result is not guaranteed.
>> But you could at least give it a try.
> We do have a limited success with userspace level sequencing of processes,
> but on the kernel level - it is all quite untrivial, as you mentioned.
> I did some
> research, and what I found pretty much a 9 years old thread which went
> nowhere at the end [1]. We would also prefer if suspend just worked (and
> we have a person looking into what is actually breaking with suspend), but
> there is an unbounded amount of time for how long the investigation and
> search for a solution may be ongoing given the complexity of the problem,
> and in the meantime there is no way to work around the problem.
>
> Thanks,
> Daniil
>
> [1] https://linux-kernel.vger.kernel.narkive.com/UeBWfN1V/patch-fuse-make-fuse-daemon-frozen-along-with-kernel-threads

So that sounds like anything that is waiting for a response cannot be
frozen? Assuming there is an outstanding NFS request and the NFS server
is down, suspend would not work until the NFS server comes back?

Thanks,
Bernd

2022-05-12 14:46:43

by Daniil Lunev

[permalink] [raw]
Subject: Re: [PATCH 0/2] Prevent re-use of FUSE superblock after force unmount

> I think it would be easiest to remove the super block from the
> type->fs_supers list.
I will try tomorrow and upload an updated patchset.
Thanks,
Daniil.