On 4/3/23 16:51, Aleksandr Mikhalitsyn wrote:
> On Mon, Mar 6, 2023 at 3:09 PM Aleksandr Mikhalitsyn
> <aleksandr.mikhalitsyn@xxxxxxxxxxxxx> wrote:
>>
>> On Fri, Mar 3, 2023 at 8:26 PM Bernd Schubert
>> <bernd.schubert@xxxxxxxxxxx> wrote:
>>>
>>>
>>>
>>> On 2/20/23 20:37, Alexander Mikhalitsyn wrote:
>>>> This ioctl aborts fuse connection and then reinitializes it,
>>>> sends FUSE_INIT request to allow a new userspace daemon
>>>> to pick up the fuse connection.
>>>>
>>>> Cc: Miklos Szeredi <mszeredi@xxxxxxxxxx>
>>>> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
>>>> Cc: Amir Goldstein <amir73il@xxxxxxxxx>
>>>> Cc: Stéphane Graber <stgraber@xxxxxxxxxx>
>>>> Cc: Seth Forshee <sforshee@xxxxxxxxxx>
>>>> Cc: Christian Brauner <brauner@xxxxxxxxxx>
>>>> Cc: Andrei Vagin <avagin@xxxxxxxxx>
>>>> Cc: Pavel Tikhomirov <ptikhomirov@xxxxxxxxxxxxx>
>>>> Cc: linux-fsdevel@xxxxxxxxxxxxxxx
>>>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>>>> Cc: criu@xxxxxxxxxx
>>>> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@xxxxxxxxxxxxx>
>>>> ---
>>>> fs/fuse/dev.c | 132 ++++++++++++++++++++++++++++++++++++++
>>>> include/uapi/linux/fuse.h | 1 +
>>>> 2 files changed, 133 insertions(+)
>>>>
>>>> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
>>>> index 737764c2295e..0f53ffd63957 100644
>>>> --- a/fs/fuse/dev.c
>>>> +++ b/fs/fuse/dev.c
>>>> @@ -2187,6 +2187,112 @@ void fuse_abort_conn(struct fuse_conn *fc)
>>>> }
>>>> EXPORT_SYMBOL_GPL(fuse_abort_conn);
>>>>
>>>> +static int fuse_reinit_conn(struct fuse_conn *fc)
>>>> +{
>>>> + struct fuse_iqueue *fiq = &fc->iq;
>>>> + struct fuse_dev *fud;
>>>> + unsigned int i;
>>>> +
>>>> + if (fc->conn_gen + 1 < fc->conn_gen)
>>>> + return -EOVERFLOW;
>>>> +
>>>> + fuse_abort_conn(fc);
>>>> + fuse_wait_aborted(fc);
>>>
>>> Shouldn't this also try to flush all data first?
>
> Dear Bernd,
>
> I've reviewed this place 2nd time and I'm not sure that we have to
> perform any flushing there, because userspace daemon can be dead or
> stuck.
> Technically, if userspace knows that daemon is alive then it can call
> fsync/sync before doing reinit.
>
> What do you think about it?
Hello Alex,
sorry for my late reply.
Hmm, I just fear that fsync/sync is a bit racy, what is if a user would
write data after the sync and that would get silently removed by
fuse_abort_conn()? Isn't what we want:
ioctl
refuse new requests -> unset fc->initialized
flush all fc queues (fc->iq.pending, fc->bg_queue, I guess with your
current patches we do not need to handle forget)
fuse_abort_conn
So what is missing is the information if the daemon is still running -
take a daemon reference and then check for PF_EXITING, as in my uring
patches? Miklos has some objections for that, though.
The alternative would be to mount read-only, then sync, then do the
ioctl and remount back. I don't know what needs to be done to get
remount working, though. Just handle it in libfuse mount.fuse and send
the mount syscall?
As I wrote before, at DDN we want to have run time daemon restart - I'm
also not opposed to entirely give up on the flush and to just work on a
restart protocol to make the new daemon to the old state (opened files
and lookup/forget count). In principle we could even transfer that in
userspace from one daemon to the other?
Thanks,
Bernd
PS: Will look at the new patches later this week.
Thanks,
Bernd
