[Qemu-commits] [qemu/qemu] 46488b: tests/virtio-9p: added split readdir tests

[Peter Maydell]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 46488b627ac8f13a3cbf83cfbfa011e82314b279
      
https://github.com/qemu/qemu/commit/46488b627ac8f13a3cbf83cfbfa011e82314b279
  Author: Christian Schoenebeck <qemu_oss@crudebyte.com>
  Date:   2020-08-12 (Wed, 12 Aug 2020)

  Changed paths:
    M tests/qtest/virtio-9p-test.c

  Log Message:
  -----------
  tests/virtio-9p: added split readdir tests

The previous, already existing 'basic' readdir test simply used a
'count' parameter big enough to retrieve all directory entries with a
single Treaddir request.

In the 3 new 'split' readdir tests added by this patch, directory
entries are retrieved, split over several Treaddir requests by picking
small 'count' parameters which force the server to truncate the
response. So the test client sends as many Treaddir requests as
necessary to get all directory entries.

The following 3 new tests are added (executed in this sequence):

1. Split readdir test with count=512
2. Split readdir test with count=256
3. Split readdir test with count=128

This test case sequence is chosen because the smaller the 'count' value,
the higher the chance of errors in case of implementation bugs on server
side.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: 
<569b2e05ab1d0223b14a12dfbdf3ad5e8b3ac131.1596012787.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>


  Commit: 29c9d2ca80ebac14ff55f687634cd3fa491ba8be
      
https://github.com/qemu/qemu/commit/29c9d2ca80ebac14ff55f687634cd3fa491ba8be
  Author: Christian Schoenebeck <qemu_oss@crudebyte.com>
  Date:   2020-08-12 (Wed, 12 Aug 2020)

  Changed paths:
    M hw/9pfs/9p.c
    M hw/9pfs/9p.h

  Log Message:
  -----------
  9pfs: make v9fs_readdir_response_size() public

Rename function v9fs_readdir_data_size() -> v9fs_readdir_response_size()
and make it callable from other units. So far this function is only
used by 9p.c, however subsequent patches require the function to be
callable from another 9pfs unit. And as we're at it; also make it clear
for what this function is used for.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: 
<3668ebc7d5b929a0e4f1357457060d96f50f76f4.1596012787.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>


  Commit: dd8151f4fed6ee078a33cff4caa9ac5c558940ef
      
https://github.com/qemu/qemu/commit/dd8151f4fed6ee078a33cff4caa9ac5c558940ef
  Author: Christian Schoenebeck <qemu_oss@crudebyte.com>
  Date:   2020-08-12 (Wed, 12 Aug 2020)

  Changed paths:
    M hw/9pfs/codir.c

  Log Message:
  -----------
  9pfs: split out fs driver core of v9fs_co_readdir()

The implementation of v9fs_co_readdir() has two parts: the outer
part is executed by main I/O thread, whereas the inner part is
executed by fs driver on a background I/O thread.

Move the inner part to its own new, private function do_readdir(),
so it can be shared by another upcoming new function.

This is just a preparatory patch for the subsequent patch, with the
purpose to avoid the next patch to clutter the overall diff.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: 
<a426ee06e77584fa2d8253ce5d8bea519eb3ffd4.1596012787.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>


  Commit: 2149675b195f2d9a1a4e3b966d45aba234def69b
      
https://github.com/qemu/qemu/commit/2149675b195f2d9a1a4e3b966d45aba234def69b
  Author: Christian Schoenebeck <qemu_oss@crudebyte.com>
  Date:   2020-08-12 (Wed, 12 Aug 2020)

  Changed paths:
    M hw/9pfs/9p.h
    M hw/9pfs/codir.c
    M hw/9pfs/coth.h

  Log Message:
  -----------
  9pfs: add new function v9fs_co_readdir_many()

The newly added function v9fs_co_readdir_many() retrieves multiple
directory entries with a single fs driver request. It is intended to
replace uses of v9fs_co_readdir(), the latter only retrieves a
single directory entry per fs driver request instead.

The reason for this planned replacement is that for every fs driver
request the coroutine is dispatched from main I/O thread to a
background I/O thread and eventually dispatched back to main I/O
thread. Hopping between threads adds latency. So if a 9pfs Treaddir
request reads a large amount of directory entries, this currently
sums up to huge latencies of several hundred ms or even more. So
using v9fs_co_readdir_many() instead of v9fs_co_readdir() will
provide significant performance improvements.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Message-Id: 
<73dc827a12ef577ae7e644dcf34a5c0e443ab42f.1596012787.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>


  Commit: 0c4356ba7dafc8ecb5877a42fc0d68d45ccf5951
      
https://github.com/qemu/qemu/commit/0c4356ba7dafc8ecb5877a42fc0d68d45ccf5951
  Author: Christian Schoenebeck <qemu_oss@crudebyte.com>
  Date:   2020-08-12 (Wed, 12 Aug 2020)

  Changed paths:
    M hw/9pfs/9p.c

  Log Message:
  -----------
  9pfs: T_readdir latency optimization

Make top half really top half and bottom half really bottom half:

Each T_readdir request handling is hopping between threads (main
I/O thread and background I/O driver threads) several times for
every individual directory entry, which sums up to huge latencies
for handling just a single T_readdir request.

Instead of doing that, collect now all required directory entries
(including all potentially required stat buffers for each entry) in
one rush on a background I/O thread from fs driver by calling the
previously added function v9fs_co_readdir_many() instead of
v9fs_co_readdir(), then assemble the entire resulting network
response message for the readdir request on main I/O thread. The
fs driver is still aborting the directory entry retrieval loop
(on the background I/O thread inside of v9fs_co_readdir_many())
as soon as it would exceed the client's requested maximum R_readdir
response size. So this will not introduce a performance penalty on
another end.

Also: No longer seek initial directory position in v9fs_readdir(),
as this is now handled (more consistently) by
v9fs_co_readdir_many() instead.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Message-Id: 
<c7c3d1cf4e86611538cef44897842819d9359d7a.1596012787.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>


  Commit: d2c5cf7ca15490b4737a5393e51abf0301b98971
      
https://github.com/qemu/qemu/commit/d2c5cf7ca15490b4737a5393e51abf0301b98971
  Author: Christian Schoenebeck <qemu_oss@crudebyte.com>
  Date:   2020-08-12 (Wed, 12 Aug 2020)

  Changed paths:
    M hw/9pfs/9p.c
    M hw/9pfs/9p.h

  Log Message:
  -----------
  9pfs: differentiate readdir lock between 9P2000.u vs. 9P2000.L

Previous patch suggests that it might make sense to use a different mutex
type now while handling readdir requests, depending on the precise
protocol variant, as v9fs_do_readdir_with_stat() (used by 9P2000.u) uses
a CoMutex to avoid deadlocks that might happen with QemuMutex otherwise,
whereas do_readdir_many() (used by 9P2000.L) should better use a
QemuMutex, as the precise behaviour of a failed CoMutex lock on fs driver
side would not be clear.

And to avoid the wrong lock type being used, be now strict and error out
if a 9P2000.L client sends a Tread on a directory, and likeweise error out
if a 9P2000.u client sends a Treaddir request.

This patch is just intended as transitional measure, as currently 9P2000.u
vs. 9P2000.L implementations currently differ where the main logic of
fetching directory entries is located at (9P2000.u still being more top
half focused, while 9P2000.L already being bottom half focused in regards
to fetching directory entries that is).

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Message-Id: 
<9a2ddc347e533b0d801866afd9dfac853d2d4106.1596012787.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>


  Commit: da9f2eda2551c1cbd98f72730e5b754f2149a85c
      
https://github.com/qemu/qemu/commit/da9f2eda2551c1cbd98f72730e5b754f2149a85c
  Author: Christian Schoenebeck <qemu_oss@crudebyte.com>
  Date:   2020-08-12 (Wed, 12 Aug 2020)

  Changed paths:
    M hw/9pfs/coth.h

  Log Message:
  -----------
  9pfs: clarify latency of v9fs_co_run_in_worker()

As we just fixed a severe performance issue with Treaddir request
handling, clarify this overall issue as a comment on
v9fs_co_run_in_worker() with the intention to hopefully prevent
such performance mistakes in future (and fixing other yet
outstanding ones).

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Message-Id: 
<4d34d332e1aaa8a2cf8dc0b5da4fd7727f2a86e8.1596012787.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>


  Commit: 30aa19446d82358a30eac3b556b4d6641e00b7c1
      
https://github.com/qemu/qemu/commit/30aa19446d82358a30eac3b556b4d6641e00b7c1
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2020-08-24 (Mon, 24 Aug 2020)

  Changed paths:
    M hw/9pfs/9p.c
    M hw/9pfs/9p.h
    M hw/9pfs/codir.c
    M hw/9pfs/coth.h
    M tests/qtest/virtio-9p-test.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/cschoenebeck/tags/pull-9p-20200812' 
into staging

9pfs: Fix severe performance issue of Treaddir requests.

# gpg: Signature made Wed 12 Aug 2020 11:06:21 BST
# gpg:                using RSA key 96D8D110CF7AF8084F88590134C2B58765A47395
# gpg:                issuer "qemu_oss@crudebyte.com"
# gpg: Good signature from "Christian Schoenebeck <qemu_oss@crudebyte.com>" 
[unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: ECAB 1A45 4014 1413 BA38  4926 30DB 47C3 A012 D5F4
#      Subkey fingerprint: 96D8 D110 CF7A F808 4F88  5901 34C2 B587 65A4 7395

* remotes/cschoenebeck/tags/pull-9p-20200812:
  9pfs: clarify latency of v9fs_co_run_in_worker()
  9pfs: differentiate readdir lock between 9P2000.u vs. 9P2000.L
  9pfs: T_readdir latency optimization
  9pfs: add new function v9fs_co_readdir_many()
  9pfs: split out fs driver core of v9fs_co_readdir()
  9pfs: make v9fs_readdir_response_size() public
  tests/virtio-9p: added split readdir tests

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Compare: https://github.com/qemu/qemu/compare/df82aa7fe10e...30aa19446d82