Re: [PATCH v8 3/7] 9pfs: split out fs driver core of v9fs_co_readdir()

[Greg Kurz]

On Wed, 29 Jul 2020 10:11:54 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> 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>
> ---
>  hw/9pfs/codir.c | 37 +++++++++++++++++++++++--------------
>  1 file changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
> index 73f9a751e1..ff57fb8619 100644
> --- a/hw/9pfs/codir.c
> +++ b/hw/9pfs/codir.c
> @@ -18,28 +18,37 @@
>  #include "qemu/main-loop.h"
>  #include "coth.h"
>  
> +/*
> + * This must solely be executed on a background IO thread.
> + */

Well, technically this function could be called from any context
but of course calling it from the main I/O thread when handling
T_readdir would make the request synchronous, which is certainly
not what we want. So I'm not sure this comment brings much.

Anyway, the code change is okay so:

Reviewed-by: Greg Kurz <groug@kaod.org>

> +static int do_readdir(V9fsPDU *pdu, V9fsFidState *fidp, struct dirent **dent)
> +{
> +    int err = 0;
> +    V9fsState *s = pdu->s;
> +    struct dirent *entry;
> +
> +    errno = 0;
> +    entry = s->ops->readdir(&s->ctx, &fidp->fs);
> +    if (!entry && errno) {
> +        *dent = NULL;
> +        err = -errno;
> +    } else {
> +        *dent = entry;
> +    }
> +    return err;
> +}
> +
>  int coroutine_fn v9fs_co_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
>                                   struct dirent **dent)
>  {
>      int err;
> -    V9fsState *s = pdu->s;
>  
>      if (v9fs_request_cancelled(pdu)) {
>          return -EINTR;
>      }
> -    v9fs_co_run_in_worker(
> -        {
> -            struct dirent *entry;
> -
> -            errno = 0;
> -            entry = s->ops->readdir(&s->ctx, &fidp->fs);
> -            if (!entry && errno) {
> -                err = -errno;
> -            } else {
> -                *dent = entry;
> -                err = 0;
> -            }
> -        });
> +    v9fs_co_run_in_worker({
> +        err = do_readdir(pdu, fidp, dent);
> +    });
>      return err;
>  }
>