On Mon, May 13, 2024 at 3:02 PM Anuj Gupta <anuj20.g@xxxxxxxxxxx> wrote: > > The user mapped intergity is copied back and unpinned by > bio_integrity_free which is a low-level routine. Do it via the submitter > rather than doing it in the low-level block layer code, to split the > submitter side from the consumer side of the bio. > > Signed-off-by: Anuj Gupta <anuj20.g@xxxxxxxxxxx> > Signed-off-by: Kanchan Joshi <joshi.k@xxxxxxxxxxx> > --- > Changes in v2: > - create a helper for unmap logic (Keith) > - return if integrity is not user-mapped (Jens) > - v1: https://lore.kernel.org/linux-block/20240510094429.2489-1-anuj20.g@xxxxxxxxxxx/ > --- > block/bio-integrity.c | 26 ++++++++++++++++++++++++-- > drivers/nvme/host/ioctl.c | 15 +++++++++++---- > include/linux/bio.h | 4 ++++ > 3 files changed, 39 insertions(+), 6 deletions(-) > > diff --git a/block/bio-integrity.c b/block/bio-integrity.c > index 2e3e8e04961e..8b528e12136f 100644 > --- a/block/bio-integrity.c > +++ b/block/bio-integrity.c > @@ -144,16 +144,38 @@ void bio_integrity_free(struct bio *bio) > struct bio_integrity_payload *bip = bio_integrity(bio); > struct bio_set *bs = bio->bi_pool; > > + if (bip->bip_flags & BIP_INTEGRITY_USER) > + return; > if (bip->bip_flags & BIP_BLOCK_INTEGRITY) > kfree(bvec_virt(bip->bip_vec)); > - else if (bip->bip_flags & BIP_INTEGRITY_USER) > - bio_integrity_unmap_user(bip); > > __bio_integrity_free(bs, bip); > bio->bi_integrity = NULL; > bio->bi_opf &= ~REQ_INTEGRITY; > } > > +/** > + * bio_integrity_unmap_free_user - Unmap and free bio user integrity payload > + * @bio: bio containing bip to be unmapped and freed > + * > + * Description: Used to unmap and free the user mapped integrity portion of a > + * bio. Submitter attaching the user integrity buffer is responsible for > + * unmapping and freeing it during completion. > + */ > +void bio_integrity_unmap_free_user(struct bio *bio) > +{ > + struct bio_integrity_payload *bip = bio_integrity(bio); > + struct bio_set *bs = bio->bi_pool; > + > + if (WARN_ON_ONCE(!(bip->bip_flags & BIP_INTEGRITY_USER))) > + return; > + bio_integrity_unmap_user(bip); > + __bio_integrity_free(bs, bip); > + bio->bi_integrity = NULL; > + bio->bi_opf &= ~REQ_INTEGRITY; > +} > +EXPORT_SYMBOL(bio_integrity_unmap_free_user); > + > /** > * bio_integrity_add_page - Attach integrity metadata > * @bio: bio to update > diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c > index 499a8bb7cac7..2dff5933cae9 100644 > --- a/drivers/nvme/host/ioctl.c > +++ b/drivers/nvme/host/ioctl.c > @@ -111,6 +111,13 @@ static struct request *nvme_alloc_user_request(struct request_queue *q, > return req; > } > > +static void nvme_unmap_bio(struct bio *bio) > +{ > + if (bio_integrity(bio)) > + bio_integrity_unmap_free_user(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, struct io_uring_cmd *ioucmd, unsigned int flags) > @@ -157,7 +164,7 @@ static int nvme_map_user_request(struct request *req, u64 ubuffer, > > out_unmap: > if (bio) > - blk_rq_unmap_user(bio); > + nvme_unmap_bio(bio); > out: > blk_mq_free_request(req); > return ret; > @@ -195,7 +202,7 @@ static int nvme_submit_user_cmd(struct request_queue *q, > if (result) > *result = le64_to_cpu(nvme_req(req)->result.u64); > if (bio) > - blk_rq_unmap_user(bio); > + nvme_unmap_bio(bio); > blk_mq_free_request(req); > > if (effects) > @@ -406,7 +413,7 @@ 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); > + nvme_unmap_bio(pdu->bio); > io_uring_cmd_done(ioucmd, pdu->status, pdu->result, issue_flags); > } > > @@ -432,7 +439,7 @@ static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req, > */ > if (blk_rq_is_poll(req)) { > if (pdu->bio) > - blk_rq_unmap_user(pdu->bio); > + nvme_unmap_bio(pdu->bio); > io_uring_cmd_iopoll_done(ioucmd, pdu->result, pdu->status); > } else { > io_uring_cmd_do_in_task_lazy(ioucmd, nvme_uring_task_cb); > diff --git a/include/linux/bio.h b/include/linux/bio.h > index d5379548d684..818e93612947 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -731,6 +731,7 @@ static inline bool bioset_initialized(struct bio_set *bs) > bip_for_each_vec(_bvl, _bio->bi_integrity, _iter) > > int bio_integrity_map_user(struct bio *bio, void __user *ubuf, ssize_t len, u32 seed); > +void bio_integrity_unmap_free_user(struct bio *bio); > extern struct bio_integrity_payload *bio_integrity_alloc(struct bio *, gfp_t, unsigned int); > extern int bio_integrity_add_page(struct bio *, struct page *, unsigned int, unsigned int); > extern bool bio_integrity_prep(struct bio *); > @@ -807,6 +808,9 @@ static inline int bio_integrity_map_user(struct bio *bio, void __user *ubuf, > { > return -EINVAL; > } > +static inline void bio_integrity_unmap_free_user(struct bio *bio) > +{ > +} > > #endif /* CONFIG_BLK_DEV_INTEGRITY */ > > -- > 2.25.1 > > Hi Jens, This has got the necessary reviews. Can this be picked up? Or do you see anything missing?
