[PATCH v2] block: unmap and free user mapped integrity via submitter

[Anuj gupta]

On Mon, May 13, 2024 at 3:02 PM Anuj Gupta <anuj20.g at samsung.com> 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 at samsung.com>
> Signed-off-by: Kanchan Joshi <joshi.k at samsung.com>
> ---
> 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@samsung.com/
> ---
>  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?