[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 00/11] hw/nvme: reimplement all multi-aio commands with c
From: |
Stefan Hajnoczi |
Subject: |
Re: [RFC PATCH 00/11] hw/nvme: reimplement all multi-aio commands with custom aiocbs |
Date: |
Tue, 8 Jun 2021 11:40:20 +0100 |
On Fri, Jun 04, 2021 at 08:52:26AM +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> This series reimplements flush, dsm, copy, zone reset and format nvm to
> allow cancellation. I posted an RFC back in March ("hw/block/nvme:
> convert ad-hoc aio tracking to aiocb") and I've applied some feedback
> from Stefan and reimplemented the remaining commands.
>
> The basic idea is to define custom AIOCBs for these commands. The custom
> AIOCB takes care of issuing all the "nested" AIOs one by one instead of
> blindly sending them off simultaneously without tracking the returned
> aiocbs.
>
> I've kept the RFC since I'm still new to using the block layer like
> this. I was hoping that Stefan could find some time to look over this -
> this is a huge series, so I don't expect non-nvme folks to spend a large
> amount of time on it, but I would really like feedback on my approach in
> the reimplementation of flush and format. Those commands are special in
> that may issue AIOs to multiple namespaces and thus, to multiple block
> backends. Since this device does not support iothreads, I've opted for
> simply always returning the main loop aio context, but I wonder if this
> is acceptable or not. It might be the case that this should contain an
> assert of some kind, in case someone starts adding iothread support.
This approach looks fine to me. Vladimir mentioned coroutines, which
have simpler code for sequential I/O, but don't support cancellation.
Since cancellation is the point of this series I think sticking to the
aio approach makes sense.
Regarding coroutine cancellation, it's a hard to add since there is
already a lot of coroutine code that's not written with cancellation in
mind.
I think I would approach it by adding a .cancel_cb() field to Coroutine
that does nothing by default (because existing code doesn't support
cancellation and we must wait for the operation to complete). Cases the
do support cancellation would install a .cancel_cb() across yield that
causes the operation that coroutine is waiting on to complete early.
An alternative approach is to re-enter the coroutine, but this requires
all yield points in QEMU to check for cancellation. I don't think this
is practical because converting all the code would be hard.
Anyway, the aio approach looks fine.
Stefan
signature.asc
Description: PGP signature
- [RFC PATCH 08/11] hw/nvme: reimplement the copy command to allow aio cancellation, (continued)
- [RFC PATCH 08/11] hw/nvme: reimplement the copy command to allow aio cancellation, Klaus Jensen, 2021/06/04
- [RFC PATCH 09/11] hw/nvme: reimplement zone reset to allow cancellation, Klaus Jensen, 2021/06/04
- [RFC PATCH 10/11] hw/nvme: reimplement format nvm to allow cancellation, Klaus Jensen, 2021/06/04
- [RFC PATCH 11/11] Partially revert "hw/block/nvme: drain namespaces on sq deletion", Klaus Jensen, 2021/06/04
- Re: [RFC PATCH 00/11] hw/nvme: reimplement all multi-aio commands with custom aiocbs, Vladimir Sementsov-Ogievskiy, 2021/06/07
Re: [RFC PATCH 00/11] hw/nvme: reimplement all multi-aio commands with custom aiocbs,
Stefan Hajnoczi <=