On 10/29/24 22:18, Christoph Hellwig wrote:
> This is only used by the nvmet zns passthrough code, which can trivially
> just use bio_add_pc_page and do the sanity check for the max zone append
> limit itself.
>
> All future zoned file systems should follow the btrfs lead and let the
> upper layers fill up bios unlimited by hardware constraints and split
> them to the limits in the I/O submission handler.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> block/bio.c | 33 ---------------------------------
> drivers/nvme/target/zns.c | 21 +++++++++++++--------
> include/linux/bio.h | 2 --
> 3 files changed, 13 insertions(+), 43 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 6a60d62a529d..daceb0a5c1d7 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1064,39 +1064,6 @@ int bio_add_pc_page(struct request_queue *q, struct bio *bio,
> }
> EXPORT_SYMBOL(bio_add_pc_page);
>
> -/**
> - * bio_add_zone_append_page - attempt to add page to zone-append bio
> - * @bio: destination bio
> - * @page: page to add
> - * @len: vec entry length
> - * @offset: vec entry offset
> - *
> - * Attempt to add a page to the bio_vec maplist of a bio that will be submitted
> - * for a zone-append request. This can fail for a number of reasons, such as the
> - * bio being full or the target block device is not a zoned block device or
> - * other limitations of the target block device. The target block device must
> - * allow bio's up to PAGE_SIZE, so it is always possible to add a single page
> - * to an empty bio.
> - *
> - * Returns: number of bytes added to the bio, or 0 in case of a failure.
> - */
> -int bio_add_zone_append_page(struct bio *bio, struct page *page,
> - unsigned int len, unsigned int offset)
> -{
> - struct request_queue *q = bdev_get_queue(bio->bi_bdev);
> - bool same_page = false;
> -
> - if (WARN_ON_ONCE(bio_op(bio) != REQ_OP_ZONE_APPEND))
> - return 0;
> -
> - if (WARN_ON_ONCE(!bdev_is_zoned(bio->bi_bdev)))
> - return 0;
> -
> - return bio_add_hw_page(q, bio, page, len, offset,
> - queue_max_zone_append_sectors(q), &same_page);
> -}
> -EXPORT_SYMBOL_GPL(bio_add_zone_append_page);
> -
> /**
> * __bio_add_page - add page(s) to a bio in a new segment
> * @bio: destination bio
> diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
> index af9e13be7678..3aef35b05111 100644
> --- a/drivers/nvme/target/zns.c
> +++ b/drivers/nvme/target/zns.c
> @@ -537,6 +537,7 @@ void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
> u16 status = NVME_SC_SUCCESS;
> unsigned int total_len = 0;
> struct scatterlist *sg;
> + u32 data_len = nvmet_rw_data_len(req);
> struct bio *bio;
> int sg_cnt;
>
> @@ -544,6 +545,13 @@ void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
> if (!nvmet_check_transfer_len(req, nvmet_rw_data_len(req)))
> return;
>
> + if (data_len >
> + bdev_max_zone_append_sectors(req->ns->bdev) << SECTOR_SHIFT) {
> + req->error_loc = offsetof(struct nvme_rw_command, length);
> + status = NVME_SC_INVALID_FIELD | NVME_STATUS_DNR;
> + goto out;
> + }
> +
> if (!req->sg_cnt) {
> nvmet_req_complete(req, 0);
> return;
> @@ -576,20 +584,17 @@ void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
> bio->bi_opf |= REQ_FUA;
>
> for_each_sg(req->sg, sg, req->sg_cnt, sg_cnt) {
> - struct page *p = sg_page(sg);
> - unsigned int l = sg->length;
> - unsigned int o = sg->offset;
> - unsigned int ret;
> + unsigned int len = sg->length;
>
> - ret = bio_add_zone_append_page(bio, p, l, o);
> - if (ret != sg->length) {
> + if (bio_add_pc_page(bdev_get_queue(bio->bi_bdev), bio,
> + sg_page(sg), len, sg->offset) != len) {
bio_add_pc_page() comment :- This should only be used by passthrough
bios.
Sorry I didn't understand use of passthru bio interface here.
From host/nvme.h:nvme_req_op():
REQ_OP_DRV_OUT/REQ_OP_DRV_IN are the passthru requests, and
nvme_req_op() is used in nvmet_passthru_execute_cmd() to map the
incoming nvme passthru command into block layer passthru request
i.e. REQ_OP_DRV_IN or REQ_OP_DRV_OUT:-
nvme/target/passthru.c :-
319 rq = blk_mq_alloc_request(q, nvme_req_op(req->cmd), 0);
In nvmet_bdev_execute_zone_append() bio->bi_opf set to
opf local var that is initialized at the start of the function to :-
const blk_opf_t opf = REQ_OP_ZONE_APPEND | REQ_SYNC | REQ_IDLE;
Hence this is not a passthru request but zone append request ?
if that is true should we just use the bio_add_hw_page() ? since
bio_add_pc_page() is a simple wrapper over bio_add_hw_page() without
the additional checks present in bio_add_zone_append_page() ?
unless for some reason I failed to understand REQ_OP_ZONE_APPEND
is categorized here as a passthru request, then sorry for wasting
your time ...
-ck
