* [PATCHv2 1/5] block: add request polling helper
2023-04-07 19:16 [PATCHv2 0/5] nvme io_uring_cmd polling enhancements Keith Busch
@ 2023-04-07 19:16 ` Keith Busch
2023-04-07 19:16 ` [PATCHv2 2/5] nvme: simplify passthrough bio cleanup Keith Busch
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Keith Busch @ 2023-04-07 19:16 UTC (permalink / raw)
To: linux-nvme, linux-block, io-uring, axboe, hch; +Cc: sagi, joshi.k, Keith Busch
From: Keith Busch <[email protected]>
This will be used by drivers that allocate polling requests.
Signed-off-by: Keith Busch <[email protected]>
---
block/blk-mq.c | 18 ++++++++++++++++++
include/linux/blk-mq.h | 2 ++
2 files changed, 20 insertions(+)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 1b304f66f4e8e..67707a488b7e5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4737,6 +4737,24 @@ int blk_mq_poll(struct request_queue *q, blk_qc_t cookie, struct io_comp_batch *
return 0;
}
+int blk_rq_poll(struct request *rq, struct io_comp_batch *iob,
+ unsigned int poll_flags)
+{
+ struct request_queue *q = rq->q;
+ int ret;
+
+ if (!blk_rq_is_poll(rq))
+ return 0;
+ if (!percpu_ref_tryget(&q->q_usage_counter))
+ return 0;
+
+ ret = blk_mq_poll(q, blk_rq_to_qc(rq), iob, poll_flags);
+ blk_queue_exit(q);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(blk_rq_poll);
+
unsigned int blk_mq_rq_cpu(struct request *rq)
{
return rq->mq_ctx->cpu;
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 06caacd77ed66..579818fa1f91d 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -722,6 +722,8 @@ int blk_mq_alloc_sq_tag_set(struct blk_mq_tag_set *set,
void blk_mq_free_tag_set(struct blk_mq_tag_set *set);
void blk_mq_free_request(struct request *rq);
+int blk_rq_poll(struct request *rq, struct io_comp_batch *iob,
+ unsigned int poll_flags);
bool blk_mq_queue_inflight(struct request_queue *q);
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCHv2 2/5] nvme: simplify passthrough bio cleanup
2023-04-07 19:16 [PATCHv2 0/5] nvme io_uring_cmd polling enhancements Keith Busch
2023-04-07 19:16 ` [PATCHv2 1/5] block: add request polling helper Keith Busch
@ 2023-04-07 19:16 ` Keith Busch
2023-04-08 0:22 ` kernel test robot
` (2 more replies)
2023-04-07 19:16 ` [PATCHv2 3/5] nvme: unify nvme request end_io Keith Busch
` (2 subsequent siblings)
4 siblings, 3 replies; 11+ messages in thread
From: Keith Busch @ 2023-04-07 19:16 UTC (permalink / raw)
To: linux-nvme, linux-block, io-uring, axboe, hch; +Cc: sagi, joshi.k, Keith Busch
From: Keith Busch <[email protected]>
Set the bio's bi_end_io to handle the cleanup so that uring_cmd doesn't
need this complex pdu->{bio,req} switchero and restore.
Signed-off-by: Keith Busch <[email protected]>
---
drivers/nvme/host/ioctl.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index d24ea2e051564..278c57ee0db91 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -159,6 +159,11 @@ static struct request *nvme_alloc_user_request(struct request_queue *q,
return req;
}
+static void nvme_uring_bio_end_io(struct bio *bio)
+{
+ blk_rq_unmap_user(bio);
+}
+
static int nvme_map_user_request(struct request *req, u64 ubuffer,
unsigned bufflen, void __user *meta_buffer, unsigned meta_len,
u32 meta_seed, void **metap, struct io_uring_cmd *ioucmd,
@@ -204,6 +209,7 @@ static int nvme_map_user_request(struct request *req, u64 ubuffer,
*metap = meta;
}
+ bio->bi_end_io = nvme_uring_bio_end_io;
return ret;
out_unmap:
@@ -249,8 +255,6 @@ static int nvme_submit_user_cmd(struct request_queue *q,
if (meta)
ret = nvme_finish_user_metadata(req, meta_buffer, meta,
meta_len, ret);
- if (bio)
- blk_rq_unmap_user(bio);
blk_mq_free_request(req);
if (effects)
@@ -443,10 +447,7 @@ struct nvme_uring_data {
* Expect build errors if this grows larger than that.
*/
struct nvme_uring_cmd_pdu {
- union {
- struct bio *bio;
- struct request *req;
- };
+ struct request *req;
u32 meta_len;
u32 nvme_status;
union {
@@ -482,8 +483,6 @@ static void nvme_uring_task_meta_cb(struct io_uring_cmd *ioucmd,
if (pdu->meta_len)
status = nvme_finish_user_metadata(req, pdu->u.meta_buffer,
pdu->u.meta, pdu->meta_len, status);
- if (req->bio)
- blk_rq_unmap_user(req->bio);
blk_mq_free_request(req);
io_uring_cmd_done(ioucmd, status, result, issue_flags);
@@ -494,9 +493,6 @@ static void nvme_uring_task_cb(struct io_uring_cmd *ioucmd,
{
struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
- if (pdu->bio)
- blk_rq_unmap_user(pdu->bio);
-
io_uring_cmd_done(ioucmd, pdu->nvme_status, pdu->u.result, issue_flags);
}
@@ -507,7 +503,6 @@ static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req,
struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
void *cookie = READ_ONCE(ioucmd->cookie);
- req->bio = pdu->bio;
if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
pdu->nvme_status = -EINTR;
else
@@ -533,9 +528,6 @@ static enum rq_end_io_ret nvme_uring_cmd_end_io_meta(struct request *req,
struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
void *cookie = READ_ONCE(ioucmd->cookie);
- req->bio = pdu->bio;
- pdu->req = req;
-
/*
* For iopoll, complete it directly.
* Otherwise, move the completion to task work.
@@ -624,8 +616,8 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
req->bio->bi_opf |= REQ_POLLED;
}
}
- /* to free bio on completion, as req->bio will be null at that time */
- pdu->bio = req->bio;
+
+ pdu->req = req;
pdu->meta_len = d.metadata_len;
req->end_io_data = ioucmd;
if (pdu->meta_len) {
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCHv2 2/5] nvme: simplify passthrough bio cleanup
2023-04-07 19:16 ` [PATCHv2 2/5] nvme: simplify passthrough bio cleanup Keith Busch
@ 2023-04-08 0:22 ` kernel test robot
2023-04-10 11:25 ` Kanchan Joshi
2023-04-10 21:02 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2023-04-08 0:22 UTC (permalink / raw)
To: Keith Busch, linux-nvme, linux-block, io-uring, axboe, hch
Cc: oe-kbuild-all, sagi, joshi.k, Keith Busch
Hi Keith,
kernel test robot noticed the following build warnings:
[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on next-20230406]
[cannot apply to linus/master v6.3-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Keith-Busch/block-add-request-polling-helper/20230408-031926
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link: https://lore.kernel.org/r/20230407191636.2631046-3-kbusch%40meta.com
patch subject: [PATCHv2 2/5] nvme: simplify passthrough bio cleanup
config: ia64-allyesconfig (https://download.01.org/0day-ci/archive/20230408/[email protected]/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/9a32e7ca02dd8cff559b273fe161b5347b5b5c97
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Keith-Busch/block-add-request-polling-helper/20230408-031926
git checkout 9a32e7ca02dd8cff559b273fe161b5347b5b5c97
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/nvme/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
drivers/nvme/host/ioctl.c: In function 'nvme_submit_user_cmd':
>> drivers/nvme/host/ioctl.c:232:21: warning: variable 'bio' set but not used [-Wunused-but-set-variable]
232 | struct bio *bio;
| ^~~
drivers/nvme/host/ioctl.c: In function 'nvme_uring_cmd_end_io_meta':
drivers/nvme/host/ioctl.c:528:36: warning: unused variable 'pdu' [-Wunused-variable]
528 | struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
| ^~~
vim +/bio +232 drivers/nvme/host/ioctl.c
2405252a680e21 Christoph Hellwig 2021-04-10 222
bcad2565b5d647 Christoph Hellwig 2022-05-11 223 static int nvme_submit_user_cmd(struct request_queue *q,
7b7fdb8e2dbc15 Christoph Hellwig 2023-01-08 224 struct nvme_command *cmd, u64 ubuffer, unsigned bufflen,
7b7fdb8e2dbc15 Christoph Hellwig 2023-01-08 225 void __user *meta_buffer, unsigned meta_len, u32 meta_seed,
7b7fdb8e2dbc15 Christoph Hellwig 2023-01-08 226 u64 *result, unsigned timeout, unsigned int flags)
bcad2565b5d647 Christoph Hellwig 2022-05-11 227 {
62281b9ed671be Christoph Hellwig 2022-12-14 228 struct nvme_ns *ns = q->queuedata;
bc8fb906b0ff93 Keith Busch 2022-09-19 229 struct nvme_ctrl *ctrl;
bcad2565b5d647 Christoph Hellwig 2022-05-11 230 struct request *req;
bcad2565b5d647 Christoph Hellwig 2022-05-11 231 void *meta = NULL;
bcad2565b5d647 Christoph Hellwig 2022-05-11 @232 struct bio *bio;
bc8fb906b0ff93 Keith Busch 2022-09-19 233 u32 effects;
bcad2565b5d647 Christoph Hellwig 2022-05-11 234 int ret;
bcad2565b5d647 Christoph Hellwig 2022-05-11 235
470e900c8036ff Kanchan Joshi 2022-09-30 236 req = nvme_alloc_user_request(q, cmd, 0, 0);
bcad2565b5d647 Christoph Hellwig 2022-05-11 237 if (IS_ERR(req))
bcad2565b5d647 Christoph Hellwig 2022-05-11 238 return PTR_ERR(req);
bcad2565b5d647 Christoph Hellwig 2022-05-11 239
470e900c8036ff Kanchan Joshi 2022-09-30 240 req->timeout = timeout;
470e900c8036ff Kanchan Joshi 2022-09-30 241 if (ubuffer && bufflen) {
470e900c8036ff Kanchan Joshi 2022-09-30 242 ret = nvme_map_user_request(req, ubuffer, bufflen, meta_buffer,
7b7fdb8e2dbc15 Christoph Hellwig 2023-01-08 243 meta_len, meta_seed, &meta, NULL, flags);
470e900c8036ff Kanchan Joshi 2022-09-30 244 if (ret)
470e900c8036ff Kanchan Joshi 2022-09-30 245 return ret;
470e900c8036ff Kanchan Joshi 2022-09-30 246 }
470e900c8036ff Kanchan Joshi 2022-09-30 247
bcad2565b5d647 Christoph Hellwig 2022-05-11 248 bio = req->bio;
bc8fb906b0ff93 Keith Busch 2022-09-19 249 ctrl = nvme_req(req)->ctrl;
bcad2565b5d647 Christoph Hellwig 2022-05-11 250
62281b9ed671be Christoph Hellwig 2022-12-14 251 effects = nvme_passthru_start(ctrl, ns, cmd->common.opcode);
62281b9ed671be Christoph Hellwig 2022-12-14 252 ret = nvme_execute_rq(req, false);
2405252a680e21 Christoph Hellwig 2021-04-10 253 if (result)
2405252a680e21 Christoph Hellwig 2021-04-10 254 *result = le64_to_cpu(nvme_req(req)->result.u64);
bcad2565b5d647 Christoph Hellwig 2022-05-11 255 if (meta)
bcad2565b5d647 Christoph Hellwig 2022-05-11 256 ret = nvme_finish_user_metadata(req, meta_buffer, meta,
bcad2565b5d647 Christoph Hellwig 2022-05-11 257 meta_len, ret);
2405252a680e21 Christoph Hellwig 2021-04-10 258 blk_mq_free_request(req);
bc8fb906b0ff93 Keith Busch 2022-09-19 259
bc8fb906b0ff93 Keith Busch 2022-09-19 260 if (effects)
bc8fb906b0ff93 Keith Busch 2022-09-19 261 nvme_passthru_end(ctrl, effects, cmd, ret);
bc8fb906b0ff93 Keith Busch 2022-09-19 262
2405252a680e21 Christoph Hellwig 2021-04-10 263 return ret;
2405252a680e21 Christoph Hellwig 2021-04-10 264 }
2405252a680e21 Christoph Hellwig 2021-04-10 265
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCHv2 2/5] nvme: simplify passthrough bio cleanup
2023-04-07 19:16 ` [PATCHv2 2/5] nvme: simplify passthrough bio cleanup Keith Busch
2023-04-08 0:22 ` kernel test robot
@ 2023-04-10 11:25 ` Kanchan Joshi
2023-04-11 17:46 ` Keith Busch
2023-04-10 21:02 ` kernel test robot
2 siblings, 1 reply; 11+ messages in thread
From: Kanchan Joshi @ 2023-04-10 11:25 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, linux-block, io-uring, axboe, hch, sagi, Keith Busch
[-- Attachment #1: Type: text/plain, Size: 2028 bytes --]
On Fri, Apr 07, 2023 at 12:16:33PM -0700, Keith Busch wrote:
>From: Keith Busch <[email protected]>
>
>Set the bio's bi_end_io to handle the cleanup so that uring_cmd doesn't
>need this complex pdu->{bio,req} switchero and restore.
>
>Signed-off-by: Keith Busch <[email protected]>
>---
> drivers/nvme/host/ioctl.c | 26 +++++++++-----------------
> 1 file changed, 9 insertions(+), 17 deletions(-)
>
>diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
>index d24ea2e051564..278c57ee0db91 100644
>--- a/drivers/nvme/host/ioctl.c
>+++ b/drivers/nvme/host/ioctl.c
>@@ -159,6 +159,11 @@ static struct request *nvme_alloc_user_request(struct request_queue *q,
> return req;
> }
>
>+static void nvme_uring_bio_end_io(struct bio *bio)
>+{
>+ blk_rq_unmap_user(bio);
>+}
>+
> static int nvme_map_user_request(struct request *req, u64 ubuffer,
> unsigned bufflen, void __user *meta_buffer, unsigned meta_len,
> u32 meta_seed, void **metap, struct io_uring_cmd *ioucmd,
>@@ -204,6 +209,7 @@ static int nvme_map_user_request(struct request *req, u64 ubuffer,
> *metap = meta;
> }
>
>+ bio->bi_end_io = nvme_uring_bio_end_io;
> return ret;
>
> out_unmap:
>@@ -249,8 +255,6 @@ static int nvme_submit_user_cmd(struct request_queue *q,
> if (meta)
> ret = nvme_finish_user_metadata(req, meta_buffer, meta,
> meta_len, ret);
>- if (bio)
>- blk_rq_unmap_user(bio);
Is it safe to call blk_rq_unamp_user in irq context?
Agree that current code does some complex stuff, but that's to ensure
what the code-comment [1] says.
Also for polled-io, new-code will hit this warn-on [2] on calling
bio_put_percpu_cache.
[1]
623 * A matching blk_rq_unmap_user() must be issued at the end of I/O, while
624 * still in process context.
625 */
626 int blk_rq_map_user_iov(struct request_queue *q, struct request *rq,
627 struct rq_map_data *map_data,
[2]
773 if ((bio->bi_opf & REQ_POLLED) && !WARN_ON_ONCE(in_interrupt())) {
774 bio->bi_next = cache->free_list;
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCHv2 2/5] nvme: simplify passthrough bio cleanup
2023-04-10 11:25 ` Kanchan Joshi
@ 2023-04-11 17:46 ` Keith Busch
0 siblings, 0 replies; 11+ messages in thread
From: Keith Busch @ 2023-04-11 17:46 UTC (permalink / raw)
To: Kanchan Joshi
Cc: Keith Busch, linux-nvme, linux-block, io-uring, axboe, hch, sagi
On Mon, Apr 10, 2023 at 04:55:03PM +0530, Kanchan Joshi wrote:
> On Fri, Apr 07, 2023 at 12:16:33PM -0700, Keith Busch wrote:
> > +static void nvme_uring_bio_end_io(struct bio *bio)
> > +{
> > + blk_rq_unmap_user(bio);
> > +}
> > +
> > static int nvme_map_user_request(struct request *req, u64 ubuffer,
> > unsigned bufflen, void __user *meta_buffer, unsigned meta_len,
> > u32 meta_seed, void **metap, struct io_uring_cmd *ioucmd,
> > @@ -204,6 +209,7 @@ static int nvme_map_user_request(struct request *req, u64 ubuffer,
> > *metap = meta;
> > }
> >
> > + bio->bi_end_io = nvme_uring_bio_end_io;
> > return ret;
> >
> > out_unmap:
> > @@ -249,8 +255,6 @@ static int nvme_submit_user_cmd(struct request_queue *q,
> > if (meta)
> > ret = nvme_finish_user_metadata(req, meta_buffer, meta,
> > meta_len, ret);
> > - if (bio)
> > - blk_rq_unmap_user(bio);
>
> Is it safe to call blk_rq_unamp_user in irq context?
Doh! I boxed my thinking into the polling mode that completely neglected the
more common use case. Thanks, now back to the drawing board for me...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv2 2/5] nvme: simplify passthrough bio cleanup
2023-04-07 19:16 ` [PATCHv2 2/5] nvme: simplify passthrough bio cleanup Keith Busch
2023-04-08 0:22 ` kernel test robot
2023-04-10 11:25 ` Kanchan Joshi
@ 2023-04-10 21:02 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2023-04-10 21:02 UTC (permalink / raw)
To: Keith Busch, linux-nvme, linux-block, io-uring, axboe, hch
Cc: oe-kbuild-all, sagi, joshi.k, Keith Busch
Hi Keith,
kernel test robot noticed the following build warnings:
[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on next-20230406]
[cannot apply to linus/master v6.3-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Keith-Busch/block-add-request-polling-helper/20230408-031926
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link: https://lore.kernel.org/r/20230407191636.2631046-3-kbusch%40meta.com
patch subject: [PATCHv2 2/5] nvme: simplify passthrough bio cleanup
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20230411/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/9a32e7ca02dd8cff559b273fe161b5347b5b5c97
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Keith-Busch/block-add-request-polling-helper/20230408-031926
git checkout 9a32e7ca02dd8cff559b273fe161b5347b5b5c97
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 olddefconfig
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/nvme/host/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
drivers/nvme/host/ioctl.c: In function 'nvme_submit_user_cmd':
drivers/nvme/host/ioctl.c:232:21: warning: variable 'bio' set but not used [-Wunused-but-set-variable]
232 | struct bio *bio;
| ^~~
drivers/nvme/host/ioctl.c: In function 'nvme_uring_cmd_end_io_meta':
>> drivers/nvme/host/ioctl.c:528:36: warning: unused variable 'pdu' [-Wunused-variable]
528 | struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
| ^~~
vim +/pdu +528 drivers/nvme/host/ioctl.c
c0a7ba77e81b84 Jens Axboe 2022-09-21 523
c0a7ba77e81b84 Jens Axboe 2022-09-21 524 static enum rq_end_io_ret nvme_uring_cmd_end_io_meta(struct request *req,
c0a7ba77e81b84 Jens Axboe 2022-09-21 525 blk_status_t err)
c0a7ba77e81b84 Jens Axboe 2022-09-21 526 {
c0a7ba77e81b84 Jens Axboe 2022-09-21 527 struct io_uring_cmd *ioucmd = req->end_io_data;
c0a7ba77e81b84 Jens Axboe 2022-09-21 @528 struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
c0a7ba77e81b84 Jens Axboe 2022-09-21 529 void *cookie = READ_ONCE(ioucmd->cookie);
c0a7ba77e81b84 Jens Axboe 2022-09-21 530
c0a7ba77e81b84 Jens Axboe 2022-09-21 531 /*
c0a7ba77e81b84 Jens Axboe 2022-09-21 532 * For iopoll, complete it directly.
c0a7ba77e81b84 Jens Axboe 2022-09-21 533 * Otherwise, move the completion to task work.
c0a7ba77e81b84 Jens Axboe 2022-09-21 534 */
c0a7ba77e81b84 Jens Axboe 2022-09-21 535 if (cookie != NULL && blk_rq_is_poll(req))
9d2789ac9d60c0 Jens Axboe 2023-03-20 536 nvme_uring_task_meta_cb(ioucmd, IO_URING_F_UNLOCKED);
c0a7ba77e81b84 Jens Axboe 2022-09-21 537 else
c0a7ba77e81b84 Jens Axboe 2022-09-21 538 io_uring_cmd_complete_in_task(ioucmd, nvme_uring_task_meta_cb);
c0a7ba77e81b84 Jens Axboe 2022-09-21 539
de671d6116b521 Jens Axboe 2022-09-21 540 return RQ_END_IO_NONE;
456cba386e94f2 Kanchan Joshi 2022-05-11 541 }
456cba386e94f2 Kanchan Joshi 2022-05-11 542
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv2 3/5] nvme: unify nvme request end_io
2023-04-07 19:16 [PATCHv2 0/5] nvme io_uring_cmd polling enhancements Keith Busch
2023-04-07 19:16 ` [PATCHv2 1/5] block: add request polling helper Keith Busch
2023-04-07 19:16 ` [PATCHv2 2/5] nvme: simplify passthrough bio cleanup Keith Busch
@ 2023-04-07 19:16 ` Keith Busch
2023-04-10 11:34 ` Kanchan Joshi
2023-04-07 19:16 ` [PATCHv2 4/5] nvme: use blk-mq polling for uring commands Keith Busch
2023-04-07 19:16 ` [PATCHv2 5/5] io_uring: remove uring_cmd cookie Keith Busch
4 siblings, 1 reply; 11+ messages in thread
From: Keith Busch @ 2023-04-07 19:16 UTC (permalink / raw)
To: linux-nvme, linux-block, io-uring, axboe, hch; +Cc: sagi, joshi.k, Keith Busch
From: Keith Busch <[email protected]>
We can finish the metadata copy inline with the completion. After that,
there's really nothing else different between the meta and non-meta data
end_io callbacks, so unify them.
Signed-off-by: Keith Busch <[email protected]>
---
drivers/nvme/host/ioctl.c | 57 +++++++--------------------------------
1 file changed, 9 insertions(+), 48 deletions(-)
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 278c57ee0db91..a1e0a14dadedc 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -465,29 +465,6 @@ static inline struct nvme_uring_cmd_pdu *nvme_uring_cmd_pdu(
return (struct nvme_uring_cmd_pdu *)&ioucmd->pdu;
}
-static void nvme_uring_task_meta_cb(struct io_uring_cmd *ioucmd,
- unsigned issue_flags)
-{
- struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
- struct request *req = pdu->req;
- int status;
- u64 result;
-
- if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
- status = -EINTR;
- else
- status = nvme_req(req)->status;
-
- result = le64_to_cpu(nvme_req(req)->result.u64);
-
- if (pdu->meta_len)
- status = nvme_finish_user_metadata(req, pdu->u.meta_buffer,
- pdu->u.meta, pdu->meta_len, status);
- blk_mq_free_request(req);
-
- io_uring_cmd_done(ioucmd, status, result, issue_flags);
-}
-
static void nvme_uring_task_cb(struct io_uring_cmd *ioucmd,
unsigned issue_flags)
{
@@ -502,11 +479,16 @@ static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req,
struct io_uring_cmd *ioucmd = req->end_io_data;
struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
void *cookie = READ_ONCE(ioucmd->cookie);
+ int status = nvme_req(req)->status;
if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
- pdu->nvme_status = -EINTR;
- else
- pdu->nvme_status = nvme_req(req)->status;
+ status = -EINTR;
+
+ if (pdu->meta_len)
+ status = nvme_finish_user_metadata(req, pdu->u.meta_buffer,
+ pdu->u.meta, pdu->meta_len, status);
+
+ pdu->nvme_status = status;
pdu->u.result = le64_to_cpu(nvme_req(req)->result.u64);
/*
@@ -521,25 +503,6 @@ static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req,
return RQ_END_IO_FREE;
}
-static enum rq_end_io_ret nvme_uring_cmd_end_io_meta(struct request *req,
- blk_status_t err)
-{
- struct io_uring_cmd *ioucmd = req->end_io_data;
- struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
- void *cookie = READ_ONCE(ioucmd->cookie);
-
- /*
- * For iopoll, complete it directly.
- * Otherwise, move the completion to task work.
- */
- if (cookie != NULL && blk_rq_is_poll(req))
- nvme_uring_task_meta_cb(ioucmd, IO_URING_F_UNLOCKED);
- else
- io_uring_cmd_complete_in_task(ioucmd, nvme_uring_task_meta_cb);
-
- return RQ_END_IO_NONE;
-}
-
static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
struct io_uring_cmd *ioucmd, unsigned int issue_flags, bool vec)
{
@@ -620,12 +583,10 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
pdu->req = req;
pdu->meta_len = d.metadata_len;
req->end_io_data = ioucmd;
+ req->end_io = nvme_uring_cmd_end_io;
if (pdu->meta_len) {
pdu->u.meta = meta;
pdu->u.meta_buffer = nvme_to_user_ptr(d.metadata);
- req->end_io = nvme_uring_cmd_end_io_meta;
- } else {
- req->end_io = nvme_uring_cmd_end_io;
}
blk_execute_rq_nowait(req, false);
return -EIOCBQUEUED;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCHv2 3/5] nvme: unify nvme request end_io
2023-04-07 19:16 ` [PATCHv2 3/5] nvme: unify nvme request end_io Keith Busch
@ 2023-04-10 11:34 ` Kanchan Joshi
0 siblings, 0 replies; 11+ messages in thread
From: Kanchan Joshi @ 2023-04-10 11:34 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, linux-block, io-uring, axboe, hch, sagi, Keith Busch
[-- Attachment #1: Type: text/plain, Size: 2220 bytes --]
On Fri, Apr 07, 2023 at 12:16:34PM -0700, Keith Busch wrote:
>From: Keith Busch <[email protected]>
>
>We can finish the metadata copy inline with the completion. After that,
>there's really nothing else different between the meta and non-meta data
>end_io callbacks, so unify them.
>
>Signed-off-by: Keith Busch <[email protected]>
>---
> drivers/nvme/host/ioctl.c | 57 +++++++--------------------------------
> 1 file changed, 9 insertions(+), 48 deletions(-)
>
>diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
>index 278c57ee0db91..a1e0a14dadedc 100644
>--- a/drivers/nvme/host/ioctl.c
>+++ b/drivers/nvme/host/ioctl.c
>@@ -465,29 +465,6 @@ static inline struct nvme_uring_cmd_pdu *nvme_uring_cmd_pdu(
> return (struct nvme_uring_cmd_pdu *)&ioucmd->pdu;
> }
>
>-static void nvme_uring_task_meta_cb(struct io_uring_cmd *ioucmd,
>- unsigned issue_flags)
>-{
>- struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
>- struct request *req = pdu->req;
>- int status;
>- u64 result;
>-
>- if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
>- status = -EINTR;
>- else
>- status = nvme_req(req)->status;
>-
>- result = le64_to_cpu(nvme_req(req)->result.u64);
>-
>- if (pdu->meta_len)
>- status = nvme_finish_user_metadata(req, pdu->u.meta_buffer,
>- pdu->u.meta, pdu->meta_len, status);
>- blk_mq_free_request(req);
>-
>- io_uring_cmd_done(ioucmd, status, result, issue_flags);
>-}
>-
> static void nvme_uring_task_cb(struct io_uring_cmd *ioucmd,
> unsigned issue_flags)
> {
>@@ -502,11 +479,16 @@ static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req,
> struct io_uring_cmd *ioucmd = req->end_io_data;
> struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
> void *cookie = READ_ONCE(ioucmd->cookie);
>+ int status = nvme_req(req)->status;
>
> if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
>- pdu->nvme_status = -EINTR;
>- else
>- pdu->nvme_status = nvme_req(req)->status;
>+ status = -EINTR;
>+
>+ if (pdu->meta_len)
>+ status = nvme_finish_user_metadata(req, pdu->u.meta_buffer,
>+ pdu->u.meta, pdu->meta_len, status);
nvme_finish_user_metadata does copy_to_user.
Here also the attempt was not to touch that memory in interrupt context.
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv2 4/5] nvme: use blk-mq polling for uring commands
2023-04-07 19:16 [PATCHv2 0/5] nvme io_uring_cmd polling enhancements Keith Busch
` (2 preceding siblings ...)
2023-04-07 19:16 ` [PATCHv2 3/5] nvme: unify nvme request end_io Keith Busch
@ 2023-04-07 19:16 ` Keith Busch
2023-04-07 19:16 ` [PATCHv2 5/5] io_uring: remove uring_cmd cookie Keith Busch
4 siblings, 0 replies; 11+ messages in thread
From: Keith Busch @ 2023-04-07 19:16 UTC (permalink / raw)
To: linux-nvme, linux-block, io-uring, axboe, hch; +Cc: sagi, joshi.k, Keith Busch
From: Keith Busch <[email protected]>
Drivers can now poll requests directly, and the nvme uring command
unconditionally saves the request now, so use that. We just need to
clear it on completion to prevent any races among multiple threads
polling the same queues during a tagset teardown.
The first advantage is that unshared and multipath namespaces can use
the same polling callback, and multipath is guaranteed to get the same
queue as the command was submitted on. Previously multipath polling might
check a different path and poll the wrong info.
The other advantage is that we don't need a bio payload in order to
poll, allowing commands like 'flush' and 'write zeroes' to be submitted
on the same high priority queue as read and write commands.
This can also allow for a future optimization for the driver since we no
longer need to create special hidden block devices to back nvme-generic
char dev's with unsupported command sets.
Signed-off-by: Keith Busch <[email protected]>
---
drivers/nvme/host/ioctl.c | 58 +++++------------------------------
drivers/nvme/host/multipath.c | 2 +-
drivers/nvme/host/nvme.h | 2 --
3 files changed, 9 insertions(+), 53 deletions(-)
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index a1e0a14dadedc..3604166752e4b 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -478,9 +478,9 @@ static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req,
{
struct io_uring_cmd *ioucmd = req->end_io_data;
struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
- void *cookie = READ_ONCE(ioucmd->cookie);
int status = nvme_req(req)->status;
+ WRITE_ONCE(pdu->req, NULL);
if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
status = -EINTR;
@@ -495,7 +495,7 @@ static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req,
* For iopoll, complete it directly.
* Otherwise, move the completion to task work.
*/
- if (cookie != NULL && blk_rq_is_poll(req))
+ if (blk_rq_is_poll(req))
nvme_uring_task_cb(ioucmd, IO_URING_F_UNLOCKED);
else
io_uring_cmd_complete_in_task(ioucmd, nvme_uring_task_cb);
@@ -554,7 +554,6 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
if (issue_flags & IO_URING_F_IOPOLL)
rq_flags |= REQ_POLLED;
-retry:
req = nvme_alloc_user_request(q, &c, rq_flags, blk_flags);
if (IS_ERR(req))
return PTR_ERR(req);
@@ -568,19 +567,7 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
return ret;
}
- if (issue_flags & IO_URING_F_IOPOLL && rq_flags & REQ_POLLED) {
- if (unlikely(!req->bio)) {
- /* we can't poll this, so alloc regular req instead */
- blk_mq_free_request(req);
- rq_flags &= ~REQ_POLLED;
- goto retry;
- } else {
- WRITE_ONCE(ioucmd->cookie, req->bio);
- req->bio->bi_opf |= REQ_POLLED;
- }
- }
-
- pdu->req = req;
+ WRITE_ONCE(pdu->req, req);
pdu->meta_len = d.metadata_len;
req->end_io_data = ioucmd;
req->end_io = nvme_uring_cmd_end_io;
@@ -735,18 +722,14 @@ int nvme_ns_chr_uring_cmd_iopoll(struct io_uring_cmd *ioucmd,
struct io_comp_batch *iob,
unsigned int poll_flags)
{
- struct bio *bio;
+ struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
+ struct request *req;
int ret = 0;
- struct nvme_ns *ns;
- struct request_queue *q;
rcu_read_lock();
- bio = READ_ONCE(ioucmd->cookie);
- ns = container_of(file_inode(ioucmd->file)->i_cdev,
- struct nvme_ns, cdev);
- q = ns->queue;
- if (test_bit(QUEUE_FLAG_POLL, &q->queue_flags) && bio && bio->bi_bdev)
- ret = bio_poll(bio, iob, poll_flags);
+ req = READ_ONCE(pdu->req);
+ if (req && blk_rq_is_poll(req))
+ ret = blk_rq_poll(req, iob, poll_flags);
rcu_read_unlock();
return ret;
}
@@ -838,31 +821,6 @@ int nvme_ns_head_chr_uring_cmd(struct io_uring_cmd *ioucmd,
srcu_read_unlock(&head->srcu, srcu_idx);
return ret;
}
-
-int nvme_ns_head_chr_uring_cmd_iopoll(struct io_uring_cmd *ioucmd,
- struct io_comp_batch *iob,
- unsigned int poll_flags)
-{
- struct cdev *cdev = file_inode(ioucmd->file)->i_cdev;
- struct nvme_ns_head *head = container_of(cdev, struct nvme_ns_head, cdev);
- int srcu_idx = srcu_read_lock(&head->srcu);
- struct nvme_ns *ns = nvme_find_path(head);
- struct bio *bio;
- int ret = 0;
- struct request_queue *q;
-
- if (ns) {
- rcu_read_lock();
- bio = READ_ONCE(ioucmd->cookie);
- q = ns->queue;
- if (test_bit(QUEUE_FLAG_POLL, &q->queue_flags) && bio
- && bio->bi_bdev)
- ret = bio_poll(bio, iob, poll_flags);
- rcu_read_unlock();
- }
- srcu_read_unlock(&head->srcu, srcu_idx);
- return ret;
-}
#endif /* CONFIG_NVME_MULTIPATH */
int nvme_dev_uring_cmd(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 9171452e2f6d4..f17be1c72f4de 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -470,7 +470,7 @@ static const struct file_operations nvme_ns_head_chr_fops = {
.unlocked_ioctl = nvme_ns_head_chr_ioctl,
.compat_ioctl = compat_ptr_ioctl,
.uring_cmd = nvme_ns_head_chr_uring_cmd,
- .uring_cmd_iopoll = nvme_ns_head_chr_uring_cmd_iopoll,
+ .uring_cmd_iopoll = nvme_ns_chr_uring_cmd_iopoll,
};
static int nvme_add_ns_head_cdev(struct nvme_ns_head *head)
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index bf46f122e9e1e..ca4ea89333660 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -847,8 +847,6 @@ long nvme_dev_ioctl(struct file *file, unsigned int cmd,
unsigned long arg);
int nvme_ns_chr_uring_cmd_iopoll(struct io_uring_cmd *ioucmd,
struct io_comp_batch *iob, unsigned int poll_flags);
-int nvme_ns_head_chr_uring_cmd_iopoll(struct io_uring_cmd *ioucmd,
- struct io_comp_batch *iob, unsigned int poll_flags);
int nvme_ns_chr_uring_cmd(struct io_uring_cmd *ioucmd,
unsigned int issue_flags);
int nvme_ns_head_chr_uring_cmd(struct io_uring_cmd *ioucmd,
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCHv2 5/5] io_uring: remove uring_cmd cookie
2023-04-07 19:16 [PATCHv2 0/5] nvme io_uring_cmd polling enhancements Keith Busch
` (3 preceding siblings ...)
2023-04-07 19:16 ` [PATCHv2 4/5] nvme: use blk-mq polling for uring commands Keith Busch
@ 2023-04-07 19:16 ` Keith Busch
4 siblings, 0 replies; 11+ messages in thread
From: Keith Busch @ 2023-04-07 19:16 UTC (permalink / raw)
To: linux-nvme, linux-block, io-uring, axboe, hch; +Cc: sagi, joshi.k, Keith Busch
From: Keith Busch <[email protected]>
No users of this field anymore, so remove it.
Signed-off-by: Keith Busch <[email protected]>
---
include/linux/io_uring.h | 8 ++------
io_uring/uring_cmd.c | 1 -
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
index 35b9328ca3352..235307bf6a072 100644
--- a/include/linux/io_uring.h
+++ b/include/linux/io_uring.h
@@ -25,12 +25,8 @@ enum io_uring_cmd_flags {
struct io_uring_cmd {
struct file *file;
const void *cmd;
- union {
- /* callback to defer completions to task context */
- void (*task_work_cb)(struct io_uring_cmd *cmd, unsigned);
- /* used for polled completion */
- void *cookie;
- };
+ /* callback to defer completions to task context */
+ void (*task_work_cb)(struct io_uring_cmd *cmd, unsigned);
u32 cmd_op;
u32 flags;
u8 pdu[32]; /* available inline for free use */
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index f7a96bc76ea13..94586f691984c 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -127,7 +127,6 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
return -EOPNOTSUPP;
issue_flags |= IO_URING_F_IOPOLL;
req->iopoll_completed = 0;
- WRITE_ONCE(ioucmd->cookie, NULL);
}
ret = file->f_op->uring_cmd(ioucmd, issue_flags);
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread