| Message ID | 20200820165901.1139109-13-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block/nvme: Various cleanups required to use multiple queues | expand |
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 <stefanha@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > 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, &ret); > > - 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 <sgarzare@redhat.com> > return ret; > } > > -- > 2.26.2 > >
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 <stefanha@redhat.com> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> 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, &ret); >> >> - 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 <sgarzare@redhat.com> Thanks! > >> return ret; >> } >> >> -- >> 2.26.2 >> >> >
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 <stefanha@redhat.com> > >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> --- > >> 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, &ret); > >> > >> - 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 <sgarzare@redhat.com> > > Thanks! > > > > >> return ret; > >> } > >> > >> -- > >> 2.26.2 > >> > >> > > >
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, &ret); - BDRV_POLL_WHILE(bs, ret == -EINPROGRESS); + AIO_WAIT_WHILE(aio_context, ret == -EINPROGRESS); return ret; }
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 <stefanha@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- block/nvme.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)