[PATCH 0/9] 9pfs: readdir optimization

Christian Schoenebeck posted 9 patches 4 years, 4 months ago
Only 0 patches received!
There is a newer version of this series
hw/9pfs/9p-synth.c     |  46 ++++++++++-
hw/9pfs/9p-synth.h     |   5 ++
hw/9pfs/9p.c           | 150 ++++++++++++++++++---------------
hw/9pfs/9p.h           |  23 ++++++
hw/9pfs/codir.c        | 183 ++++++++++++++++++++++++++++++++++++++---
hw/9pfs/coth.h         |   3 +
tests/virtio-9p-test.c | 182 +++++++++++++++++++++++++++++++++++++++-
7 files changed, 509 insertions(+), 83 deletions(-)
[PATCH 0/9] 9pfs: readdir optimization
Posted by Christian Schoenebeck 4 years, 4 months ago
As previously mentioned, I was investigating performance issues with 9pfs.
Raw file read/write of 9pfs is actually quite good, provided that client
picked a reasonable high msize (maximum message size). I would recommend
to log a warning on 9p server side if a client attached with a small msize
that would cause performance issues for that reason.

However there other aspects where 9pfs currently performs suboptimally,
especially readdir handling of 9pfs is extremely slow, a simple readdir
request of a guest typically blocks for several hundred milliseconds or
even several seconds, no matter how powerful the underlying hardware is.
The reason for this performance issue: latency.
Currently 9pfs is heavily dispatching a T_readdir request numerous times
between main I/O thread and a background I/O thread back and forth; in fact
it is actually hopping between threads even multiple times for every single
directory entry during T_readdir request handling which leads in total to
huge latencies for a single T_readdir request.

This patch series aims to address this severe performance issue of 9pfs
T_readdir request handling. The actual performance fix is patch 8. I also
provided a convenient benchmark for comparing the performance improvements
by using the 9pfs "synth" driver (see patch 6 for instructions how to run
the benchmark), so no guest OS installation is required to peform this
benchmark A/B comparison. With patch 8 I achieved a performance improvement
of factor 40 on my test machine.

** NOTE: ** These patches are not heavily tested yet, nor thouroughly
reviewed for potential security issues yet. I decided to post them already
though, because I won't have the time in the next few weeks for polishing
them. The benchmark results should demonstrate though that it is worth the
hassle. So any testing/reviews/fixes appreciated!

Christian Schoenebeck (9):
  tests/virtio-9p: v9fs_string_read() didn't terminate string
  9pfs: validate count sent by client with T_readdir
  hw/9pfs/9p-synth: added directory for readdir test
  tests/virtio-9p: added READDIR test
  tests/virtio-9p: check file names of READDIR response
  9pfs: READDIR benchmark
  hw/9pfs/9p-synth: avoid n-square issue in synth_readdir()
  9pfs: T_readdir latency optimization
  hw/9pfs/9p.c: benchmark time on T_readdir request

 hw/9pfs/9p-synth.c     |  46 ++++++++++-
 hw/9pfs/9p-synth.h     |   5 ++
 hw/9pfs/9p.c           | 150 ++++++++++++++++++---------------
 hw/9pfs/9p.h           |  23 ++++++
 hw/9pfs/codir.c        | 183 ++++++++++++++++++++++++++++++++++++++---
 hw/9pfs/coth.h         |   3 +
 tests/virtio-9p-test.c | 182 +++++++++++++++++++++++++++++++++++++++-
 7 files changed, 509 insertions(+), 83 deletions(-)

-- 
2.20.1


Re: [PATCH 0/9] 9pfs: readdir optimization
Posted by Greg Kurz 4 years, 4 months ago
Hi Christian,

It seems that there was an issue with the posting of these series. Threading
is inexistant. All the emails appear scattered and unsorted in my mailbox,
between 12/16 and 12/18... which is a bit painful. I'll try to find some time
to have a look anyway, but this greatly lowers the odds for these series to get
multiple reviews, which seems problematic given the ** NOTE: ** section you've
added to the cover. Please fix this.

Cheers,

--
Greg

On Wed, 18 Dec 2019 00:11:10 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> As previously mentioned, I was investigating performance issues with 9pfs.
> Raw file read/write of 9pfs is actually quite good, provided that client
> picked a reasonable high msize (maximum message size). I would recommend
> to log a warning on 9p server side if a client attached with a small msize
> that would cause performance issues for that reason.
> 
> However there other aspects where 9pfs currently performs suboptimally,
> especially readdir handling of 9pfs is extremely slow, a simple readdir
> request of a guest typically blocks for several hundred milliseconds or
> even several seconds, no matter how powerful the underlying hardware is.
> The reason for this performance issue: latency.
> Currently 9pfs is heavily dispatching a T_readdir request numerous times
> between main I/O thread and a background I/O thread back and forth; in fact
> it is actually hopping between threads even multiple times for every single
> directory entry during T_readdir request handling which leads in total to
> huge latencies for a single T_readdir request.
> 
> This patch series aims to address this severe performance issue of 9pfs
> T_readdir request handling. The actual performance fix is patch 8. I also
> provided a convenient benchmark for comparing the performance improvements
> by using the 9pfs "synth" driver (see patch 6 for instructions how to run
> the benchmark), so no guest OS installation is required to peform this
> benchmark A/B comparison. With patch 8 I achieved a performance improvement
> of factor 40 on my test machine.
> 
> ** NOTE: ** These patches are not heavily tested yet, nor thouroughly
> reviewed for potential security issues yet. I decided to post them already
> though, because I won't have the time in the next few weeks for polishing
> them. The benchmark results should demonstrate though that it is worth the
> hassle. So any testing/reviews/fixes appreciated!
> 
> Christian Schoenebeck (9):
>   tests/virtio-9p: v9fs_string_read() didn't terminate string
>   9pfs: validate count sent by client with T_readdir
>   hw/9pfs/9p-synth: added directory for readdir test
>   tests/virtio-9p: added READDIR test
>   tests/virtio-9p: check file names of READDIR response
>   9pfs: READDIR benchmark
>   hw/9pfs/9p-synth: avoid n-square issue in synth_readdir()
>   9pfs: T_readdir latency optimization
>   hw/9pfs/9p.c: benchmark time on T_readdir request
> 
>  hw/9pfs/9p-synth.c     |  46 ++++++++++-
>  hw/9pfs/9p-synth.h     |   5 ++
>  hw/9pfs/9p.c           | 150 ++++++++++++++++++---------------
>  hw/9pfs/9p.h           |  23 ++++++
>  hw/9pfs/codir.c        | 183 ++++++++++++++++++++++++++++++++++++++---
>  hw/9pfs/coth.h         |   3 +
>  tests/virtio-9p-test.c | 182 +++++++++++++++++++++++++++++++++++++++-
>  7 files changed, 509 insertions(+), 83 deletions(-)
> 


Re: [PATCH 0/9] 9pfs: readdir optimization
Posted by Christian Schoenebeck 4 years, 4 months ago
On Mittwoch, 18. Dezember 2019 08:59:37 CET you wrote:
> Hi Christian,
> 
> It seems that there was an issue with the posting of these series. Threading
> is inexistant. All the emails appear scattered and unsorted in my mailbox,
> between 12/16 and 12/18... which is a bit painful. I'll try to find some
> time to have a look anyway, but this greatly lowers the odds for these
> series to get multiple reviews, which seems problematic given the ** NOTE:
> ** section you've added to the cover. Please fix this.

Yeah, I accidentally dropped the --thread switch this night. Sorry for that.

If you want I can resend these patches as v2 or something? Like mentioned, 
except of fixing the threading, I won't have the time to do any more polishing 
in the next few weeks at least.

Best regards,
Christian Schoenebeck



Re: [PATCH 0/9] 9pfs: readdir optimization
Posted by Greg Kurz 4 years, 4 months ago
On Wed, 18 Dec 2019 13:05:29 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Mittwoch, 18. Dezember 2019 08:59:37 CET you wrote:
> > Hi Christian,
> > 
> > It seems that there was an issue with the posting of these series. Threading
> > is inexistant. All the emails appear scattered and unsorted in my mailbox,
> > between 12/16 and 12/18... which is a bit painful. I'll try to find some
> > time to have a look anyway, but this greatly lowers the odds for these
> > series to get multiple reviews, which seems problematic given the ** NOTE:
> > ** section you've added to the cover. Please fix this.
> 
> Yeah, I accidentally dropped the --thread switch this night. Sorry for that.
> 
> If you want I can resend these patches as v2 or something? Like mentioned, 
> except of fixing the threading, I won't have the time to do any more polishing 
> in the next few weeks at least.
> 

Yeah please resend with the treading fixed at least.

> Best regards,
> Christian Schoenebeck
> 
>