[PATCH] fuse: add a dev ioctl for recovery

[PATCH] fuse: add a dev ioctl for recovery

[Hao Peng]

For a simple read-only file system, as long as the connection
is not broken, the recovery of the user-mode read-only file
system can be realized by putting the request of the processing
list back into the pending list.

Signed-off-by: Peng Hao <flyingpeng@tencent.com>
---
 fs/fuse/dev.c             | 38 +++++++++++++++++++++++++++++++++++++-
 include/uapi/linux/fuse.h |  1 +
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 491c092d427b..8e1b69a5b503 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2239,7 +2239,7 @@ static int fuse_device_clone(struct fuse_conn
*fc, struct file *new)
 static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
                           unsigned long arg)
 {
-       int res;
+       int res = 0;
        int oldfd;
        struct fuse_dev *fud = NULL;

@@ -2268,6 +2268,42 @@ static long fuse_dev_ioctl(struct file *file,
unsigned int cmd,
                        }
                }
                break;
+
+       case FUSE_DEV_IOC_RECOVERY:
+       {
+               struct fuse_iqueue *fiq;
+               struct fuse_pqueue *fpq;
+               struct fuse_req *req, *next;
+               LIST_HEAD(recovery);
+               unsigned int i;
+
+               fud = fuse_get_dev(file);
+               fiq = &fud->fc->iq;
+               fpq = &fud->pq;
+
+               spin_lock(&fpq->lock);
+               list_for_each_entry_safe(req, next, &fpq->io, list) {
+                       spin_lock(&req->waitq.lock);
+                       clear_bit(FR_LOCKED, &req->flags);
+                       list_move(&req->list, &recovery);
+                       spin_unlock(&req->waitq.lock);
+               }
+               for (i = 0; i < FUSE_PQ_HASH_SIZE; i++)
+                       list_splice_tail_init(&fpq->processing[i],
+                                             &recovery);
+               list_for_each_entry_safe(req, next, &recovery, list) {
+                       clear_bit(FR_SENT, &req->flags);
+               }
+               spin_unlock(&fpq->lock);
+
+               spin_lock(&fiq->lock);
+               list_for_each_entry_safe(req, next, &recovery, list) {
+                       set_bit(FR_PENDING, &req->flags);
+               }
+               list_splice(&recovery, &fiq->pending);
+               spin_unlock(&fiq->lock);
+               break;
+       }
        default:
                res = -ENOTTY;
                break;
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 36ed092227fa..fc07324efa9d 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -923,6 +923,7 @@ struct fuse_notify_retrieve_in {
 /* Device ioctls: */
 #define FUSE_DEV_IOC_MAGIC             229
 #define FUSE_DEV_IOC_CLONE             _IOR(FUSE_DEV_IOC_MAGIC, 0, uint32_t)
+#define FUSE_DEV_IOC_RECOVERY          _IOR(FUSE_DEV_IOC_MAGIC+1, 0, uint32_t)

 struct fuse_lseek_in {
        uint64_t        fh;
--
2.27.0

Re: [PATCH] fuse: add a dev ioctl for recovery

[Miklos Szeredi]

On Mon, 6 Sept 2021 at 14:36, Hao Peng <flyingpenghao@gmail.com> wrote:
>
> For a simple read-only file system, as long as the connection
> is not broken, the recovery of the user-mode read-only file
> system can be realized by putting the request of the processing
> list back into the pending list.

Thanks for the patch.

Do you have example userspace code for this?

>
> Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> ---
>  fs/fuse/dev.c             | 38 +++++++++++++++++++++++++++++++++++++-
>  include/uapi/linux/fuse.h |  1 +
>  2 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 491c092d427b..8e1b69a5b503 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -2239,7 +2239,7 @@ static int fuse_device_clone(struct fuse_conn
> *fc, struct file *new)
>  static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
>                            unsigned long arg)
>  {
> -       int res;
> +       int res = 0;
>         int oldfd;
>         struct fuse_dev *fud = NULL;
>
> @@ -2268,6 +2268,42 @@ static long fuse_dev_ioctl(struct file *file,
> unsigned int cmd,
>                         }
>                 }
>                 break;
> +
> +       case FUSE_DEV_IOC_RECOVERY:
> +       {
> +               struct fuse_iqueue *fiq;
> +               struct fuse_pqueue *fpq;
> +               struct fuse_req *req, *next;
> +               LIST_HEAD(recovery);
> +               unsigned int i;
> +
> +               fud = fuse_get_dev(file);
> +               fiq = &fud->fc->iq;
> +               fpq = &fud->pq;
> +
> +               spin_lock(&fpq->lock);
> +               list_for_each_entry_safe(req, next, &fpq->io, list) {
> +                       spin_lock(&req->waitq.lock);
> +                       clear_bit(FR_LOCKED, &req->flags);
> +                       list_move(&req->list, &recovery);
> +                       spin_unlock(&req->waitq.lock);
> +               }

I don't get it.  Recovery means the previous server process is dead.
If there are requests on the fpq->io queue, than that's obviously not
true and having two processes messing with the same requests isn't
going to do any good.

So I suggest just erroring out if fpq->io is not empty.


> +               for (i = 0; i < FUSE_PQ_HASH_SIZE; i++)
> +                       list_splice_tail_init(&fpq->processing[i],
> +                                             &recovery);
> +               list_for_each_entry_safe(req, next, &recovery, list) {
> +                       clear_bit(FR_SENT, &req->flags);
> +               }
> +               spin_unlock(&fpq->lock);
> +
> +               spin_lock(&fiq->lock);
> +               list_for_each_entry_safe(req, next, &recovery, list) {
> +                       set_bit(FR_PENDING, &req->flags);
> +               }
> +               list_splice(&recovery, &fiq->pending);
> +               spin_unlock(&fiq->lock);
> +               break;
> +       }
>         default:
>                 res = -ENOTTY;
>                 break;
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 36ed092227fa..fc07324efa9d 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -923,6 +923,7 @@ struct fuse_notify_retrieve_in {
>  /* Device ioctls: */
>  #define FUSE_DEV_IOC_MAGIC             229
>  #define FUSE_DEV_IOC_CLONE             _IOR(FUSE_DEV_IOC_MAGIC, 0, uint32_t)
> +#define FUSE_DEV_IOC_RECOVERY          _IOR(FUSE_DEV_IOC_MAGIC+1, 0, uint32_t)
>
>  struct fuse_lseek_in {
>         uint64_t        fh;
> --
> 2.27.0

Re: [PATCH] fuse: add a dev ioctl for recovery

[Hao Peng]

On Tue, Sep 7, 2021 at 5:34 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Mon, 6 Sept 2021 at 14:36, Hao Peng <flyingpenghao@gmail.com> wrote:
> >
> > For a simple read-only file system, as long as the connection
> > is not broken, the recovery of the user-mode read-only file
> > system can be realized by putting the request of the processing
> > list back into the pending list.
>
> Thanks for the patch.
>
> Do you have example userspace code for this?
>
Under development. When the fuse user-mode file system process is abnormal,
the process does not terminate (/dev/fuse will not be closed), enter
the reset procedure,
and will not open /dev/fuse again during the reinitialization.
Of course, this can only solve part of the abnormal problem.

> > Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> > ---
> >  fs/fuse/dev.c             | 38 +++++++++++++++++++++++++++++++++++++-
> >  include/uapi/linux/fuse.h |  1 +
> >  2 files changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index 491c092d427b..8e1b69a5b503 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -2239,7 +2239,7 @@ static int fuse_device_clone(struct fuse_conn
> > *fc, struct file *new)
> >  static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
> >                            unsigned long arg)
> >  {
> > -       int res;
> > +       int res = 0;
> >         int oldfd;
> >         struct fuse_dev *fud = NULL;
> >
> > @@ -2268,6 +2268,42 @@ static long fuse_dev_ioctl(struct file *file,
> > unsigned int cmd,
> >                         }
> >                 }
> >                 break;
> > +
> > +       case FUSE_DEV_IOC_RECOVERY:
> > +       {
> > +               struct fuse_iqueue *fiq;
> > +               struct fuse_pqueue *fpq;
> > +               struct fuse_req *req, *next;
> > +               LIST_HEAD(recovery);
> > +               unsigned int i;
> > +
> > +               fud = fuse_get_dev(file);
> > +               fiq = &fud->fc->iq;
> > +               fpq = &fud->pq;
> > +
> > +               spin_lock(&fpq->lock);
> > +               list_for_each_entry_safe(req, next, &fpq->io, list) {
> > +                       spin_lock(&req->waitq.lock);
> > +                       clear_bit(FR_LOCKED, &req->flags);
> > +                       list_move(&req->list, &recovery);
> > +                       spin_unlock(&req->waitq.lock);
> > +               }
>
> I don't get it.  Recovery means the previous server process is dead.
> If there are requests on the fpq->io queue, than that's obviously not
> true and having two processes messing with the same requests isn't
> going to do any good.
>
> So I suggest just erroring out if fpq->io is not empty.
I will modify it in the next version.
>
>
> > +               for (i = 0; i < FUSE_PQ_HASH_SIZE; i++)
> > +                       list_splice_tail_init(&fpq->processing[i],
> > +                                             &recovery);
> > +               list_for_each_entry_safe(req, next, &recovery, list) {
> > +                       clear_bit(FR_SENT, &req->flags);
> > +               }
> > +               spin_unlock(&fpq->lock);
> > +
> > +               spin_lock(&fiq->lock);
> > +               list_for_each_entry_safe(req, next, &recovery, list) {
> > +                       set_bit(FR_PENDING, &req->flags);
> > +               }
> > +               list_splice(&recovery, &fiq->pending);
> > +               spin_unlock(&fiq->lock);
> > +               break;
> > +       }
> >         default:
> >                 res = -ENOTTY;
> >                 break;
> > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> > index 36ed092227fa..fc07324efa9d 100644
> > --- a/include/uapi/linux/fuse.h
> > +++ b/include/uapi/linux/fuse.h
> > @@ -923,6 +923,7 @@ struct fuse_notify_retrieve_in {
> >  /* Device ioctls: */
> >  #define FUSE_DEV_IOC_MAGIC             229
> >  #define FUSE_DEV_IOC_CLONE             _IOR(FUSE_DEV_IOC_MAGIC, 0, uint32_t)
> > +#define FUSE_DEV_IOC_RECOVERY          _IOR(FUSE_DEV_IOC_MAGIC+1, 0, uint32_t)
> >
> >  struct fuse_lseek_in {
> >         uint64_t        fh;
> > --
> > 2.27.0

Re: [PATCH] fuse: add a dev ioctl for recovery

[Miklos Szeredi]

On Wed, 8 Sept 2021 at 04:25, Hao Peng <flyingpenghao@gmail.com> wrote:
>
> On Tue, Sep 7, 2021 at 5:34 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Mon, 6 Sept 2021 at 14:36, Hao Peng <flyingpenghao@gmail.com> wrote:
> > >
> > > For a simple read-only file system, as long as the connection
> > > is not broken, the recovery of the user-mode read-only file
> > > system can be realized by putting the request of the processing
> > > list back into the pending list.
> >
> > Thanks for the patch.
> >
> > Do you have example userspace code for this?
> >
> Under development. When the fuse user-mode file system process is abnormal,
> the process does not terminate (/dev/fuse will not be closed), enter
> the reset procedure,
> and will not open /dev/fuse again during the reinitialization.
> Of course, this can only solve part of the abnormal problem.

Yes, that's what I'm mainly worried about.   Replaying the few
currently pending requests is easy, but does that really help in real
situations?

Much more information is needed about what you are trying to achieve
and how, as well as a working userspace implementation to be able to
judge this patch.

Thanks,
Miklos

Re: [PATCH] fuse: add a dev ioctl for recovery

[Hao Peng]

On Wed, Sep 8, 2021 at 5:08 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Wed, 8 Sept 2021 at 04:25, Hao Peng <flyingpenghao@gmail.com> wrote:
> >
> > On Tue, Sep 7, 2021 at 5:34 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > >
> > > On Mon, 6 Sept 2021 at 14:36, Hao Peng <flyingpenghao@gmail.com> wrote:
> > > >
> > > > For a simple read-only file system, as long as the connection
> > > > is not broken, the recovery of the user-mode read-only file
> > > > system can be realized by putting the request of the processing
> > > > list back into the pending list.
> > >
> > > Thanks for the patch.
> > >
> > > Do you have example userspace code for this?
> > >
> > Under development. When the fuse user-mode file system process is abnormal,
> > the process does not terminate (/dev/fuse will not be closed), enter
> > the reset procedure,
> > and will not open /dev/fuse again during the reinitialization.
> > Of course, this can only solve part of the abnormal problem.
>
> Yes, that's what I'm mainly worried about.   Replaying the few
> currently pending requests is easy, but does that really help in real
> situations?
>
> Much more information is needed about what you are trying to achieve
> and how, as well as a working userspace implementation to be able to
> judge this patch.
>
I will provide a simple example in a few days. The effect achieved is that the
user process will not perceive the abnormal restart of the read-only file system
process based on fuse.

> Thanks,
> Miklos

Re: [PATCH] fuse: add a dev ioctl for recovery

[Hao Peng]

On Wed, Sep 8, 2021 at 5:27 PM Hao Peng <flyingpenghao@gmail.com> wrote:
>
> On Wed, Sep 8, 2021 at 5:08 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Wed, 8 Sept 2021 at 04:25, Hao Peng <flyingpenghao@gmail.com> wrote:
> > >
> > > On Tue, Sep 7, 2021 at 5:34 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > >
> > > > On Mon, 6 Sept 2021 at 14:36, Hao Peng <flyingpenghao@gmail.com> wrote:
> > > > >
> > > > > For a simple read-only file system, as long as the connection
> > > > > is not broken, the recovery of the user-mode read-only file
> > > > > system can be realized by putting the request of the processing
> > > > > list back into the pending list.
> > > >
> > > > Thanks for the patch.
> > > >
> > > > Do you have example userspace code for this?
> > > >
> > > Under development. When the fuse user-mode file system process is abnormal,
> > > the process does not terminate (/dev/fuse will not be closed), enter
> > > the reset procedure,
> > > and will not open /dev/fuse again during the reinitialization.
> > > Of course, this can only solve part of the abnormal problem.
> >
> > Yes, that's what I'm mainly worried about.   Replaying the few
> > currently pending requests is easy, but does that really help in real
> > situations?
> >
> > Much more information is needed about what you are trying to achieve
> > and how, as well as a working userspace implementation to be able to
> > judge this patch.
> >
> I will provide a simple example in a few days. The effect achieved is that the
> user process will not perceive the abnormal restart of the read-only file system
> process based on fuse.
>
> > Thanks,
> > Miklos
Hi，I have implemented a small test program to illustrate this new feature.
After downloading and compiling from
https://github.com/flying-122/libfuse/tree/flying
#gcc -o testfile testfile.c -D_GNU_SOURCE
#./example/passthrough_ll -o debug -s  /mnt3
#./testfile (on another console)
#ps aux | grep pass
#root       34889  0.0  0.0   8848   864 pts/2    S+   13:10   0:00
./example/passthrough_ll -o debug -s /mnt3
#root       34896  0.0  0.0   9880   128 pts/2    S+   13:10   0:00
./example/passthrough_ll -o debug -s /mnt3
#root       34913  0.0  0.0  12112  1060 pts/1    S+   13:10   0:00
grep --color=auto pass
// kill child process
#kill 34896
You will see that ./testfile continues to execute without noticing the
abnormal restart of the fuse file system.
Thanks.

Re: [PATCH] fuse: add a dev ioctl for recovery

[Miklos Szeredi]

On Wed, 10 Nov 2021 at 04:43, Hao Peng <flyingpenghao@gmail.com> wrote:
>
> On Wed, Sep 8, 2021 at 5:27 PM Hao Peng <flyingpenghao@gmail.com> wrote:
> >
> > On Wed, Sep 8, 2021 at 5:08 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > >
> > > On Wed, 8 Sept 2021 at 04:25, Hao Peng <flyingpenghao@gmail.com> wrote:
> > > >
> > > > On Tue, Sep 7, 2021 at 5:34 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > > >
> > > > > On Mon, 6 Sept 2021 at 14:36, Hao Peng <flyingpenghao@gmail.com> wrote:
> > > > > >
> > > > > > For a simple read-only file system, as long as the connection
> > > > > > is not broken, the recovery of the user-mode read-only file
> > > > > > system can be realized by putting the request of the processing
> > > > > > list back into the pending list.
> > > > >
> > > > > Thanks for the patch.
> > > > >
> > > > > Do you have example userspace code for this?
> > > > >
> > > > Under development. When the fuse user-mode file system process is abnormal,
> > > > the process does not terminate (/dev/fuse will not be closed), enter
> > > > the reset procedure,
> > > > and will not open /dev/fuse again during the reinitialization.
> > > > Of course, this can only solve part of the abnormal problem.
> > >
> > > Yes, that's what I'm mainly worried about.   Replaying the few
> > > currently pending requests is easy, but does that really help in real
> > > situations?
> > >
> > > Much more information is needed about what you are trying to achieve
> > > and how, as well as a working userspace implementation to be able to
> > > judge this patch.
> > >
> > I will provide a simple example in a few days. The effect achieved is that the
> > user process will not perceive the abnormal restart of the read-only file system
> > process based on fuse.
> >
> > > Thanks,
> > > Miklos
> Hi，I have implemented a small test program to illustrate this new feature.
> After downloading and compiling from
> https://github.com/flying-122/libfuse/tree/flying
> #gcc -o testfile testfile.c -D_GNU_SOURCE
> #./example/passthrough_ll -o debug -s  /mnt3
> #./testfile (on another console)
> #ps aux | grep pass
> #root       34889  0.0  0.0   8848   864 pts/2    S+   13:10   0:00
> ./example/passthrough_ll -o debug -s /mnt3
> #root       34896  0.0  0.0   9880   128 pts/2    S+   13:10   0:00
> ./example/passthrough_ll -o debug -s /mnt3
> #root       34913  0.0  0.0  12112  1060 pts/1    S+   13:10   0:00
> grep --color=auto pass
> // kill child process
> #kill 34896
> You will see that ./testfile continues to execute without noticing the
> abnormal restart of the fuse file system.

This is a very good first example demonstrating the limits of the
recovery.   The only state saved is the actual device file descriptor
and the result of the INIT negotiation.

It works if there are a fixed number of files, e.g. a read only
filesystem, where the files can be enumerated (i.e. a file or
directory can be found  based on a single 64bit index)

Is this your use case?

Are you ever planning to extend this to read-write filesystems?

Thanks,
Miklos

Re: [PATCH] fuse: add a dev ioctl for recovery

[Hao Peng]

On Wed, Nov 10, 2021 at 6:14 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Wed, 10 Nov 2021 at 04:43, Hao Peng <flyingpenghao@gmail.com> wrote:
> >
> > On Wed, Sep 8, 2021 at 5:27 PM Hao Peng <flyingpenghao@gmail.com> wrote:
> > >
> > > On Wed, Sep 8, 2021 at 5:08 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > >
> > > > On Wed, 8 Sept 2021 at 04:25, Hao Peng <flyingpenghao@gmail.com> wrote:
> > > > >
> > > > > On Tue, Sep 7, 2021 at 5:34 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > > > >
> > > > > > On Mon, 6 Sept 2021 at 14:36, Hao Peng <flyingpenghao@gmail.com> wrote:
> > > > > > >
> > > > > > > For a simple read-only file system, as long as the connection
> > > > > > > is not broken, the recovery of the user-mode read-only file
> > > > > > > system can be realized by putting the request of the processing
> > > > > > > list back into the pending list.
> > > > > >
> > > > > > Thanks for the patch.
> > > > > >
> > > > > > Do you have example userspace code for this?
> > > > > >
> > > > > Under development. When the fuse user-mode file system process is abnormal,
> > > > > the process does not terminate (/dev/fuse will not be closed), enter
> > > > > the reset procedure,
> > > > > and will not open /dev/fuse again during the reinitialization.
> > > > > Of course, this can only solve part of the abnormal problem.
> > > >
> > > > Yes, that's what I'm mainly worried about.   Replaying the few
> > > > currently pending requests is easy, but does that really help in real
> > > > situations?
> > > >
> > > > Much more information is needed about what you are trying to achieve
> > > > and how, as well as a working userspace implementation to be able to
> > > > judge this patch.
> > > >
> > > I will provide a simple example in a few days. The effect achieved is that the
> > > user process will not perceive the abnormal restart of the read-only file system
> > > process based on fuse.
> > >
> > > > Thanks,
> > > > Miklos
> > Hi，I have implemented a small test program to illustrate this new feature.
> > After downloading and compiling from
> > https://github.com/flying-122/libfuse/tree/flying
> > #gcc -o testfile testfile.c -D_GNU_SOURCE
> > #./example/passthrough_ll -o debug -s  /mnt3
> > #./testfile (on another console)
> > #ps aux | grep pass
> > #root       34889  0.0  0.0   8848   864 pts/2    S+   13:10   0:00
> > ./example/passthrough_ll -o debug -s /mnt3
> > #root       34896  0.0  0.0   9880   128 pts/2    S+   13:10   0:00
> > ./example/passthrough_ll -o debug -s /mnt3
> > #root       34913  0.0  0.0  12112  1060 pts/1    S+   13:10   0:00
> > grep --color=auto pass
> > // kill child process
> > #kill 34896
> > You will see that ./testfile continues to execute without noticing the
> > abnormal restart of the fuse file system.
>
> This is a very good first example demonstrating the limits of the
> recovery.   The only state saved is the actual device file descriptor
> and the result of the INIT negotiation.
>
> It works if there are a fixed number of files, e.g. a read only
> filesystem, where the files can be enumerated (i.e. a file or
> directory can be found  based on a single 64bit index)
>
> Is this your use case?
>
The version used is more complicated to maintain file/directory
information, and is used
for the internal read-only file system. As long as the number of files
remains the same, it
is also possible to write files. I plan to use this recovery method for lxcfs.
Thanks.
> Are you ever planning to extend this to read-write filesystems?
>
> Thanks,
> Miklos