Re: [PATCH v5 12/15] block/nvme: Replace BDRV_POLL_WHILE by AIO_WAIT_WHILE
On Fri, Aug 21, 2020 at 03:15:58PM +0200, Philippe Mathieu-Daudé wrote:
> On 8/21/20 12:15 PM, Stefano Garzarella wrote:
> > On Thu, Aug 20, 2020 at 06:58:58PM +0200, Philippe Mathieu-Daudé wrote:
> >> BDRV_POLL_WHILE() is defined as:
> >>
> >> #define BDRV_POLL_WHILE(bs, cond) ({ \
> >> BlockDriverState *bs_ = (bs); \
> >> AIO_WAIT_WHILE(bdrv_get_aio_context(bs_), \
> >> cond); })
> >>
> >> As we will remove the BlockDriverState use in the next commit,
> >> start by using the exploded version of BDRV_POLL_WHILE().
> >>
> >> Reviewed-by: Stefan Hajnoczi
> >> Signed-off-by: Philippe Mathieu-Daudé
> >> ---
> >> block/nvme.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/block/nvme.c b/block/nvme.c
> >> index 5b69fc75a60..456fe61f5ea 100644
> >> --- a/block/nvme.c
> >> +++ b/block/nvme.c
> >> @@ -493,6 +493,7 @@ static void nvme_cmd_sync_cb(void *opaque, int ret)
> >> static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q,
> >> NvmeCmd *cmd)
> >> {
> >> +AioContext *aio_context = bdrv_get_aio_context(bs);
> >> NVMeRequest *req;
> >> int ret = -EINPROGRESS;
> >> req = nvme_get_free_req(q);
> >> @@ -501,7 +502,7 @@ static int nvme_cmd_sync(BlockDriverState *bs,
> >> NVMeQueuePair *q,
> >> }
> >> nvme_submit_command(q, req, cmd, nvme_cmd_sync_cb, );
> >>
> >> -BDRV_POLL_WHILE(bs, ret == -EINPROGRESS);
> >> +AIO_WAIT_WHILE(aio_context, ret == -EINPROGRESS);
> >
> > Maybe I would have:
> >
> > AIO_WAIT_WHILE(bdrv_get_aio_context(bs), ret == -EINPROGRESS);
>
> I extracted aio_context in this patch because in the following
> series it is passed by the caller as an argument to nvme_cmd_sync(),
> so this makes the next series simpler to review.
Make sense!
>
> >
> > But it doesn't matter, LGTM:
> >
> > Reviewed-by: Stefano Garzarella
>
> Thanks!
>
> >
> >> return ret;
> >> }
> >>
> >> --
> >> 2.26.2
> >>
> >>
> >
>
Re: [PATCH v5 12/15] block/nvme: Replace BDRV_POLL_WHILE by AIO_WAIT_WHILE
On 8/21/20 12:15 PM, Stefano Garzarella wrote:
> On Thu, Aug 20, 2020 at 06:58:58PM +0200, Philippe Mathieu-Daudé wrote:
>> BDRV_POLL_WHILE() is defined as:
>>
>> #define BDRV_POLL_WHILE(bs, cond) ({ \
>> BlockDriverState *bs_ = (bs); \
>> AIO_WAIT_WHILE(bdrv_get_aio_context(bs_), \
>> cond); })
>>
>> As we will remove the BlockDriverState use in the next commit,
>> start by using the exploded version of BDRV_POLL_WHILE().
>>
>> Reviewed-by: Stefan Hajnoczi
>> Signed-off-by: Philippe Mathieu-Daudé
>> ---
>> block/nvme.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/nvme.c b/block/nvme.c
>> index 5b69fc75a60..456fe61f5ea 100644
>> --- a/block/nvme.c
>> +++ b/block/nvme.c
>> @@ -493,6 +493,7 @@ static void nvme_cmd_sync_cb(void *opaque, int ret)
>> static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q,
>> NvmeCmd *cmd)
>> {
>> +AioContext *aio_context = bdrv_get_aio_context(bs);
>> NVMeRequest *req;
>> int ret = -EINPROGRESS;
>> req = nvme_get_free_req(q);
>> @@ -501,7 +502,7 @@ static int nvme_cmd_sync(BlockDriverState *bs,
>> NVMeQueuePair *q,
>> }
>> nvme_submit_command(q, req, cmd, nvme_cmd_sync_cb, );
>>
>> -BDRV_POLL_WHILE(bs, ret == -EINPROGRESS);
>> +AIO_WAIT_WHILE(aio_context, ret == -EINPROGRESS);
>
> Maybe I would have:
>
> AIO_WAIT_WHILE(bdrv_get_aio_context(bs), ret == -EINPROGRESS);
I extracted aio_context in this patch because in the following
series it is passed by the caller as an argument to nvme_cmd_sync(),
so this makes the next series simpler to review.
>
> But it doesn't matter, LGTM:
>
> Reviewed-by: Stefano Garzarella
Thanks!
>
>> return ret;
>> }
>>
>> --
>> 2.26.2
>>
>>
>
Re: [PATCH v5 12/15] block/nvme: Replace BDRV_POLL_WHILE by AIO_WAIT_WHILE
On Thu, Aug 20, 2020 at 06:58:58PM +0200, Philippe Mathieu-Daudé wrote:
> BDRV_POLL_WHILE() is defined as:
>
> #define BDRV_POLL_WHILE(bs, cond) ({ \
> BlockDriverState *bs_ = (bs); \
> AIO_WAIT_WHILE(bdrv_get_aio_context(bs_), \
> cond); })
>
> As we will remove the BlockDriverState use in the next commit,
> start by using the exploded version of BDRV_POLL_WHILE().
>
> Reviewed-by: Stefan Hajnoczi
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> block/nvme.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/block/nvme.c b/block/nvme.c
> index 5b69fc75a60..456fe61f5ea 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -493,6 +493,7 @@ static void nvme_cmd_sync_cb(void *opaque, int ret)
> static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q,
> NvmeCmd *cmd)
> {
> +AioContext *aio_context = bdrv_get_aio_context(bs);
> NVMeRequest *req;
> int ret = -EINPROGRESS;
> req = nvme_get_free_req(q);
> @@ -501,7 +502,7 @@ static int nvme_cmd_sync(BlockDriverState *bs,
> NVMeQueuePair *q,
> }
> nvme_submit_command(q, req, cmd, nvme_cmd_sync_cb, );
>
> -BDRV_POLL_WHILE(bs, ret == -EINPROGRESS);
> +AIO_WAIT_WHILE(aio_context, ret == -EINPROGRESS);
Maybe I would have:
AIO_WAIT_WHILE(bdrv_get_aio_context(bs), ret == -EINPROGRESS);
But it doesn't matter, LGTM:
Reviewed-by: Stefano Garzarella
> return ret;
> }
>
> --
> 2.26.2
>
>
[PATCH v5 12/15] block/nvme: Replace BDRV_POLL_WHILE by AIO_WAIT_WHILE
BDRV_POLL_WHILE() is defined as:
#define BDRV_POLL_WHILE(bs, cond) ({ \
BlockDriverState *bs_ = (bs); \
AIO_WAIT_WHILE(bdrv_get_aio_context(bs_), \
cond); })
As we will remove the BlockDriverState use in the next commit,
start by using the exploded version of BDRV_POLL_WHILE().
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Philippe Mathieu-Daudé
---
block/nvme.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/nvme.c b/block/nvme.c
index 5b69fc75a60..456fe61f5ea 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -493,6 +493,7 @@ static void nvme_cmd_sync_cb(void *opaque, int ret)
static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q,
NvmeCmd *cmd)
{
+AioContext *aio_context = bdrv_get_aio_context(bs);
NVMeRequest *req;
int ret = -EINPROGRESS;
req = nvme_get_free_req(q);
@@ -501,7 +502,7 @@ static int nvme_cmd_sync(BlockDriverState *bs,
NVMeQueuePair *q,
}
nvme_submit_command(q, req, cmd, nvme_cmd_sync_cb, );
-BDRV_POLL_WHILE(bs, ret == -EINPROGRESS);
+AIO_WAIT_WHILE(aio_context, ret == -EINPROGRESS);
return ret;
}
--
2.26.2
