On Sonntag, 19. Juli 2020 15:20:11 CEST Christian Schoenebeck wrote:
> 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.
>
> 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_...@crudebyte.com>
> ---
> hw/9pfs/9p.c | 4 ++--
> hw/9pfs/9p.h | 27 ++++++++++++++++++++++-----
> 2 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index cc4094b971..a0881ddc88 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -314,8 +314,8 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t
> fid) f->next = s->fid_list;
> s->fid_list = f;
>
> - v9fs_readdir_init(&f->fs.dir);
> - v9fs_readdir_init(&f->fs_reclaim.dir);
> + v9fs_readdir_init(s->proto_version, &f->fs.dir);
> + v9fs_readdir_init(s->proto_version, &f->fs_reclaim.dir);
>
> return f;
> }
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index 93b7030edf..3dd1b50b1a 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -197,22 +197,39 @@ typedef struct V9fsXattr
>
> typedef struct V9fsDir {
> DIR *stream;
> - CoMutex readdir_mutex;
> + P9ProtoVersion proto_version;
> + /* readdir mutex type used for 9P2000.u protocol variant */
> + CoMutex readdir_mutex_u;
> + /* readdir mutex type used for 9P2000.L protocol variant */
> + QemuMutex readdir_mutex_L;
> } V9fsDir;
>
> static inline void v9fs_readdir_lock(V9fsDir *dir)
> {
> - qemu_co_mutex_lock(&dir->readdir_mutex);
> + if (dir->proto_version == V9FS_PROTO_2000U) {
> + qemu_co_mutex_lock(&dir->readdir_mutex_u);
> + } else {
> + qemu_mutex_lock(&dir->readdir_mutex_L);
> + }
> }
I wonder whether I should make a minor addition to this patch: returnig an
error to client if client sends T_readdir while not having opened the session
as 9P2000.L, and likewise returning an error if client sends T_read on a
directory while not using a 9P2000.u session. That would prevent the
thoretical issue of a broken client using the wrong mutex type.
It's maybe overkill, since the plan was actually to bury this patch in git
history by subsequently removing the lock entirely, but as I am preparing a v8
anyway, and this is like 2 lines more, so probably not a big deal to add it.
> static inline void v9fs_readdir_unlock(V9fsDir *dir)
> {
> - qemu_co_mutex_unlock(&dir->readdir_mutex);
> + if (dir->proto_version == V9FS_PROTO_2000U) {
> + qemu_co_mutex_unlock(&dir->readdir_mutex_u);
> + } else {
> + qemu_mutex_unlock(&dir->readdir_mutex_L);
> + }
> }
>
> -static inline void v9fs_readdir_init(V9fsDir *dir)
> +static inline void v9fs_readdir_init(P9ProtoVersion proto_version, V9fsDir
> *dir) {
> - qemu_co_mutex_init(&dir->readdir_mutex);
> + dir->proto_version = proto_version;
> + if (proto_version == V9FS_PROTO_2000U) {
> + qemu_co_mutex_init(&dir->readdir_mutex_u);
> + } else {
> + qemu_mutex_init(&dir->readdir_mutex_L);
> + }
> }
>
> /**
Best regards,
Christian Schoenebeck
