* [Virtio-fs] [PATCH] virtiofsd: enable PARALLEL_DIROPS during INIT
@ 2019-06-07 2:38 Liu Bo
2019-06-07 15:21 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 9+ messages in thread
From: Liu Bo @ 2019-06-07 2:38 UTC (permalink / raw)
To: virtio-fs
lookup is a RO operations, PARALLEL_DIROPS can be enabled.
Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
---
contrib/virtiofsd/fuse_lowlevel.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
index c460c4c..93ce788 100644
--- a/contrib/virtiofsd/fuse_lowlevel.c
+++ b/contrib/virtiofsd/fuse_lowlevel.c
@@ -2089,6 +2089,8 @@ static void do_init(fuse_req_t req, fuse_ino_t nodeid,
if (se->conn.want & FUSE_CAP_ASYNC_READ)
outarg.flags |= FUSE_ASYNC_READ;
+ if (se->conn.want & FUSE_CAP_PARALLEL_DIROPS)
+ outarg.flags |= FUSE_PARALLEL_DIROPS;
if (se->conn.want & FUSE_CAP_POSIX_LOCKS)
outarg.flags |= FUSE_POSIX_LOCKS;
if (se->conn.want & FUSE_CAP_ATOMIC_O_TRUNC)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: enable PARALLEL_DIROPS during INIT
2019-06-07 2:38 [Virtio-fs] [PATCH] virtiofsd: enable PARALLEL_DIROPS during INIT Liu Bo
@ 2019-06-07 15:21 ` Dr. David Alan Gilbert
2019-06-07 18:15 ` Liu Bo
0 siblings, 1 reply; 9+ messages in thread
From: Dr. David Alan Gilbert @ 2019-06-07 15:21 UTC (permalink / raw)
To: Liu Bo; +Cc: virtio-fs
* Liu Bo (bo.liu@linux.alibaba.com) wrote:
> lookup is a RO operations, PARALLEL_DIROPS can be enabled.
>
> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
I'm not too sure what this is doing; and why doesn't upstream libfuse
have this?
Dave
> ---
> contrib/virtiofsd/fuse_lowlevel.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
> index c460c4c..93ce788 100644
> --- a/contrib/virtiofsd/fuse_lowlevel.c
> +++ b/contrib/virtiofsd/fuse_lowlevel.c
> @@ -2089,6 +2089,8 @@ static void do_init(fuse_req_t req, fuse_ino_t nodeid,
>
> if (se->conn.want & FUSE_CAP_ASYNC_READ)
> outarg.flags |= FUSE_ASYNC_READ;
> + if (se->conn.want & FUSE_CAP_PARALLEL_DIROPS)
> + outarg.flags |= FUSE_PARALLEL_DIROPS;
> if (se->conn.want & FUSE_CAP_POSIX_LOCKS)
> outarg.flags |= FUSE_POSIX_LOCKS;
> if (se->conn.want & FUSE_CAP_ATOMIC_O_TRUNC)
> --
> 1.8.3.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: enable PARALLEL_DIROPS during INIT
2019-06-07 15:21 ` Dr. David Alan Gilbert
@ 2019-06-07 18:15 ` Liu Bo
2019-06-07 18:33 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 9+ messages in thread
From: Liu Bo @ 2019-06-07 18:15 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: virtio-fs
On Fri, Jun 07, 2019 at 04:21:47PM +0100, Dr. David Alan Gilbert wrote:
> * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > lookup is a RO operations, PARALLEL_DIROPS can be enabled.
> >
> > Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
>
> I'm not too sure what this is doing; and why doesn't upstream libfuse
> have this?
I'm not sure why libfuse doesn't include the capability, but AFAICT this
is needed for practical use.
Say that we have 100 threads doing 'ls /mnt/virtiofs/', then they are
serialized by a mutex in
---
fuse_lookup
fuse_lock_inode
if (!get_fuse_conn(inode)->parallel_dirops)
mutex_lock(&get_fuse_inode(inode)->mutex);
---
What fuse_lookup does is a plain lookup with loading inode if necessary,
so I don't see anything wrong making operations parallel.
This is found by a fsmark test,
----
NFILES=100000
time $FSMARK -D 10000 -S0 -n $NFILES -s 0 -L 5 -l /tmp/fs_log.txt \
-d $MNT/0 -d $MNT/1 \
-d $MNT/2 -d $MNT/3 \
-d $MNT/4 -d $MNT/5 \
-d $MNT/6 -d $MNT/7 \
-d $MNT/8 -d $MNT/9 \
-d $MNT/10 -d $MNT/11 \
-d $MNT/12 -d $MNT/13 \
-d $MNT/14 -d $MNT/15
----
the vanilla virtiofs was super slow due to the mutex lock.
thanks,
-liubo
>
> Dave
>
> > ---
> > contrib/virtiofsd/fuse_lowlevel.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
> > index c460c4c..93ce788 100644
> > --- a/contrib/virtiofsd/fuse_lowlevel.c
> > +++ b/contrib/virtiofsd/fuse_lowlevel.c
> > @@ -2089,6 +2089,8 @@ static void do_init(fuse_req_t req, fuse_ino_t nodeid,
> >
> > if (se->conn.want & FUSE_CAP_ASYNC_READ)
> > outarg.flags |= FUSE_ASYNC_READ;
> > + if (se->conn.want & FUSE_CAP_PARALLEL_DIROPS)
> > + outarg.flags |= FUSE_PARALLEL_DIROPS;
> > if (se->conn.want & FUSE_CAP_POSIX_LOCKS)
> > outarg.flags |= FUSE_POSIX_LOCKS;
> > if (se->conn.want & FUSE_CAP_ATOMIC_O_TRUNC)
> > --
> > 1.8.3.1
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Virtio-fs] [PATCH] virtiofsd: enable PARALLEL_DIROPS during INIT
2019-06-07 18:15 ` Liu Bo
@ 2019-06-07 18:33 ` Dr. David Alan Gilbert
2019-06-07 19:17 ` Miklos Szeredi
0 siblings, 1 reply; 9+ messages in thread
From: Dr. David Alan Gilbert @ 2019-06-07 18:33 UTC (permalink / raw)
To: Liu Bo, mszeredi; +Cc: virtio-fs
* Liu Bo (bo.liu@linux.alibaba.com) wrote:
> On Fri, Jun 07, 2019 at 04:21:47PM +0100, Dr. David Alan Gilbert wrote:
> > * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > > lookup is a RO operations, PARALLEL_DIROPS can be enabled.
> > >
> > > Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> >
> > I'm not too sure what this is doing; and why doesn't upstream libfuse
> > have this?
>
> I'm not sure why libfuse doesn't include the capability, but AFAICT this
> is needed for practical use.
Miklos: Do you know what the rules are around PARALLEL_DIROPS and why
the upstream libfuse doesn't have it eanbled?
Dave
> Say that we have 100 threads doing 'ls /mnt/virtiofs/', then they are
> serialized by a mutex in
> ---
> fuse_lookup
> fuse_lock_inode
> if (!get_fuse_conn(inode)->parallel_dirops)
> mutex_lock(&get_fuse_inode(inode)->mutex);
> ---
>
> What fuse_lookup does is a plain lookup with loading inode if necessary,
> so I don't see anything wrong making operations parallel.
>
> This is found by a fsmark test,
> ----
> NFILES=100000
>
> time $FSMARK -D 10000 -S0 -n $NFILES -s 0 -L 5 -l /tmp/fs_log.txt \
> -d $MNT/0 -d $MNT/1 \
> -d $MNT/2 -d $MNT/3 \
> -d $MNT/4 -d $MNT/5 \
> -d $MNT/6 -d $MNT/7 \
> -d $MNT/8 -d $MNT/9 \
> -d $MNT/10 -d $MNT/11 \
> -d $MNT/12 -d $MNT/13 \
> -d $MNT/14 -d $MNT/15
> ----
>
> the vanilla virtiofs was super slow due to the mutex lock.
>
> thanks,
> -liubo
>
>
> >
> > Dave
> >
> > > ---
> > > contrib/virtiofsd/fuse_lowlevel.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
> > > index c460c4c..93ce788 100644
> > > --- a/contrib/virtiofsd/fuse_lowlevel.c
> > > +++ b/contrib/virtiofsd/fuse_lowlevel.c
> > > @@ -2089,6 +2089,8 @@ static void do_init(fuse_req_t req, fuse_ino_t nodeid,
> > >
> > > if (se->conn.want & FUSE_CAP_ASYNC_READ)
> > > outarg.flags |= FUSE_ASYNC_READ;
> > > + if (se->conn.want & FUSE_CAP_PARALLEL_DIROPS)
> > > + outarg.flags |= FUSE_PARALLEL_DIROPS;
> > > if (se->conn.want & FUSE_CAP_POSIX_LOCKS)
> > > outarg.flags |= FUSE_POSIX_LOCKS;
> > > if (se->conn.want & FUSE_CAP_ATOMIC_O_TRUNC)
> > > --
> > > 1.8.3.1
> > >
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: enable PARALLEL_DIROPS during INIT
2019-06-07 18:33 ` Dr. David Alan Gilbert
@ 2019-06-07 19:17 ` Miklos Szeredi
2019-06-07 19:23 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 9+ messages in thread
From: Miklos Szeredi @ 2019-06-07 19:17 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: virtio-fs
On Fri, Jun 7, 2019 at 8:33 PM Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > On Fri, Jun 07, 2019 at 04:21:47PM +0100, Dr. David Alan Gilbert wrote:
> > > * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > > > lookup is a RO operations, PARALLEL_DIROPS can be enabled.
> > > >
> > > > Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> > >
> > > I'm not too sure what this is doing; and why doesn't upstream libfuse
> > > have this?
> >
> > I'm not sure why libfuse doesn't include the capability, but AFAICT this
> > is needed for practical use.
>
> Miklos: Do you know what the rules are around PARALLEL_DIROPS and why
> the upstream libfuse doesn't have it eanbled?
Not sure why libfuse3 wouldn't have it enabled by default, probably an
omission. Having said that, the reason this is opt in is that old
filesystems might rely on the serialized behavior, so back
compatibility requires that we have this turned off by default, so now
libfuse3 has the same problem...
Thanks,
Miklos
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: enable PARALLEL_DIROPS during INIT
2019-06-07 19:17 ` Miklos Szeredi
@ 2019-06-07 19:23 ` Dr. David Alan Gilbert
2019-06-07 19:29 ` Liu Bo
2019-06-07 19:35 ` Miklos Szeredi
0 siblings, 2 replies; 9+ messages in thread
From: Dr. David Alan Gilbert @ 2019-06-07 19:23 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: virtio-fs
* Miklos Szeredi (mszeredi@redhat.com) wrote:
> On Fri, Jun 7, 2019 at 8:33 PM Dr. David Alan Gilbert
> <dgilbert@redhat.com> wrote:
> >
> > * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > > On Fri, Jun 07, 2019 at 04:21:47PM +0100, Dr. David Alan Gilbert wrote:
> > > > * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > > > > lookup is a RO operations, PARALLEL_DIROPS can be enabled.
> > > > >
> > > > > Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> > > >
> > > > I'm not too sure what this is doing; and why doesn't upstream libfuse
> > > > have this?
> > >
> > > I'm not sure why libfuse doesn't include the capability, but AFAICT this
> > > is needed for practical use.
> >
> > Miklos: Do you know what the rules are around PARALLEL_DIROPS and why
> > the upstream libfuse doesn't have it eanbled?
>
> Not sure why libfuse3 wouldn't have it enabled by default, probably an
> omission. Having said that, the reason this is opt in is that old
> filesystems might rely on the serialized behavior, so back
> compatibility requires that we have this turned off by default, so now
> libfuse3 has the same problem...
So for us (virtio-fs) is it always safe?
Dave
> Thanks,
> Miklos
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: enable PARALLEL_DIROPS during INIT
2019-06-07 19:23 ` Dr. David Alan Gilbert
@ 2019-06-07 19:29 ` Liu Bo
2019-06-07 19:35 ` Miklos Szeredi
1 sibling, 0 replies; 9+ messages in thread
From: Liu Bo @ 2019-06-07 19:29 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: virtio-fs, Miklos Szeredi
On Fri, Jun 07, 2019 at 08:23:46PM +0100, Dr. David Alan Gilbert wrote:
> * Miklos Szeredi (mszeredi@redhat.com) wrote:
> > On Fri, Jun 7, 2019 at 8:33 PM Dr. David Alan Gilbert
> > <dgilbert@redhat.com> wrote:
> > >
> > > * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > > > On Fri, Jun 07, 2019 at 04:21:47PM +0100, Dr. David Alan Gilbert wrote:
> > > > > * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > > > > > lookup is a RO operations, PARALLEL_DIROPS can be enabled.
> > > > > >
> > > > > > Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> > > > >
> > > > > I'm not too sure what this is doing; and why doesn't upstream libfuse
> > > > > have this?
> > > >
> > > > I'm not sure why libfuse doesn't include the capability, but AFAICT this
> > > > is needed for practical use.
> > >
> > > Miklos: Do you know what the rules are around PARALLEL_DIROPS and why
> > > the upstream libfuse doesn't have it eanbled?
> >
> > Not sure why libfuse3 wouldn't have it enabled by default, probably an
> > omission. Having said that, the reason this is opt in is that old
> > filesystems might rely on the serialized behavior, so back
> > compatibility requires that we have this turned off by default, so now
> > libfuse3 has the same problem...
>
> So for us (virtio-fs) is it always safe?
>
FYI, my first round of limited testing (dbench4, fsmark, fstests) didn't say no.
thanks,
-liubo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: enable PARALLEL_DIROPS during INIT
2019-06-07 19:23 ` Dr. David Alan Gilbert
2019-06-07 19:29 ` Liu Bo
@ 2019-06-07 19:35 ` Miklos Szeredi
2019-06-10 9:04 ` Dr. David Alan Gilbert
1 sibling, 1 reply; 9+ messages in thread
From: Miklos Szeredi @ 2019-06-07 19:35 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: virtio-fs
On Fri, Jun 7, 2019 at 9:23 PM Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Miklos Szeredi (mszeredi@redhat.com) wrote:
> > On Fri, Jun 7, 2019 at 8:33 PM Dr. David Alan Gilbert
> > <dgilbert@redhat.com> wrote:
> > >
> > > * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > > > On Fri, Jun 07, 2019 at 04:21:47PM +0100, Dr. David Alan Gilbert wrote:
> > > > > * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > > > > > lookup is a RO operations, PARALLEL_DIROPS can be enabled.
> > > > > >
> > > > > > Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> > > > >
> > > > > I'm not too sure what this is doing; and why doesn't upstream libfuse
> > > > > have this?
> > > >
> > > > I'm not sure why libfuse doesn't include the capability, but AFAICT this
> > > > is needed for practical use.
> > >
> > > Miklos: Do you know what the rules are around PARALLEL_DIROPS and why
> > > the upstream libfuse doesn't have it eanbled?
> >
> > Not sure why libfuse3 wouldn't have it enabled by default, probably an
> > omission. Having said that, the reason this is opt in is that old
> > filesystems might rely on the serialized behavior, so back
> > compatibility requires that we have this turned off by default, so now
> > libfuse3 has the same problem...
>
> So for us (virtio-fs) is it always safe?
Yes, since no shared data is used between readdir instances.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: enable PARALLEL_DIROPS during INIT
2019-06-07 19:35 ` Miklos Szeredi
@ 2019-06-10 9:04 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 9+ messages in thread
From: Dr. David Alan Gilbert @ 2019-06-10 9:04 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: virtio-fs
* Miklos Szeredi (mszeredi@redhat.com) wrote:
> On Fri, Jun 7, 2019 at 9:23 PM Dr. David Alan Gilbert
> <dgilbert@redhat.com> wrote:
> >
> > * Miklos Szeredi (mszeredi@redhat.com) wrote:
> > > On Fri, Jun 7, 2019 at 8:33 PM Dr. David Alan Gilbert
> > > <dgilbert@redhat.com> wrote:
> > > >
> > > > * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > > > > On Fri, Jun 07, 2019 at 04:21:47PM +0100, Dr. David Alan Gilbert wrote:
> > > > > > * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > > > > > > lookup is a RO operations, PARALLEL_DIROPS can be enabled.
> > > > > > >
> > > > > > > Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> > > > > >
> > > > > > I'm not too sure what this is doing; and why doesn't upstream libfuse
> > > > > > have this?
> > > > >
> > > > > I'm not sure why libfuse doesn't include the capability, but AFAICT this
> > > > > is needed for practical use.
> > > >
> > > > Miklos: Do you know what the rules are around PARALLEL_DIROPS and why
> > > > the upstream libfuse doesn't have it eanbled?
> > >
> > > Not sure why libfuse3 wouldn't have it enabled by default, probably an
> > > omission. Having said that, the reason this is opt in is that old
> > > filesystems might rely on the serialized behavior, so back
> > > compatibility requires that we have this turned off by default, so now
> > > libfuse3 has the same problem...
> >
> > So for us (virtio-fs) is it always safe?
>
> Yes, since no shared data is used between readdir instances.
OK, in that case
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
and added to my tree.
> Thanks,
> Miklos
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-06-10 9:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-07 2:38 [Virtio-fs] [PATCH] virtiofsd: enable PARALLEL_DIROPS during INIT Liu Bo
2019-06-07 15:21 ` Dr. David Alan Gilbert
2019-06-07 18:15 ` Liu Bo
2019-06-07 18:33 ` Dr. David Alan Gilbert
2019-06-07 19:17 ` Miklos Szeredi
2019-06-07 19:23 ` Dr. David Alan Gilbert
2019-06-07 19:29 ` Liu Bo
2019-06-07 19:35 ` Miklos Szeredi
2019-06-10 9:04 ` Dr. David Alan Gilbert
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.