[PATCH] ublk: move zone report data out of request pdu

[PATCH] ublk: move zone report data out of request pdu

[Ming Lei]

ublk zoned takes 16 bytes in each request pdu just for handling REPORT_ZONE
operation, this way does waste memory since request pdu is allocated
statically.

Store the transient zone report data into one global xarray, and remove
it after the report zone request is completed. This way is reasonable
since report zone is run in slow code path.

Cc: Damien Le Moal <dlemoal@kernel.org>
Cc: Andreas Hindborg <a.hindborg@samsung.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/ublk_drv.c | 62 +++++++++++++++++++++++++++++-----------
 1 file changed, 46 insertions(+), 16 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 890c08792ba8..0e295471318a 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -71,9 +71,6 @@ struct ublk_rq_data {
 	struct llist_node node;
 
 	struct kref ref;
-	__u64 sector;
-	__u32 operation;
-	__u32 nr_zones;
 };
 
 struct ublk_uring_cmd_pdu {
@@ -214,6 +211,33 @@ static inline bool ublk_queue_is_zoned(struct ublk_queue *ubq)
 
 #ifdef CONFIG_BLK_DEV_ZONED
 
+struct ublk_zoned_report_desc {
+	__u64 sector;
+	__u32 operation;
+	__u32 nr_zones;
+};
+
+static DEFINE_XARRAY(ublk_zoned_report_descs);
+
+static int ublk_zoned_insert_report_desc(const struct request *req,
+		struct ublk_zoned_report_desc *desc)
+{
+	return xa_insert(&ublk_zoned_report_descs, (unsigned long)req,
+			    desc, GFP_KERNEL);
+}
+
+static struct ublk_zoned_report_desc *ublk_zoned_erase_report_desc(
+		const struct request *req)
+{
+	return xa_erase(&ublk_zoned_report_descs, (unsigned long)req);
+}
+
+static struct ublk_zoned_report_desc *ublk_zoned_get_report_desc(
+		const struct request *req)
+{
+	return xa_load(&ublk_zoned_report_descs, (unsigned long)req);
+}
+
 static int ublk_get_nr_zones(const struct ublk_device *ub)
 {
 	const struct ublk_param_basic *p = &ub->params.basic;
@@ -308,7 +332,7 @@ static int ublk_report_zones(struct gendisk *disk, sector_t sector,
 		unsigned int zones_in_request =
 			min_t(unsigned int, remaining_zones, max_zones_per_request);
 		struct request *req;
-		struct ublk_rq_data *pdu;
+		struct ublk_zoned_report_desc desc;
 		blk_status_t status;
 
 		memset(buffer, 0, buffer_length);
@@ -319,20 +343,23 @@ static int ublk_report_zones(struct gendisk *disk, sector_t sector,
 			goto out;
 		}
 
-		pdu = blk_mq_rq_to_pdu(req);
-		pdu->operation = UBLK_IO_OP_REPORT_ZONES;
-		pdu->sector = sector;
-		pdu->nr_zones = zones_in_request;
+		desc.operation = UBLK_IO_OP_REPORT_ZONES;
+		desc.sector = sector;
+		desc.nr_zones = zones_in_request;
+		ret = ublk_zoned_insert_report_desc(req, &desc);
+		if (ret)
+			goto free_req;
 
 		ret = blk_rq_map_kern(disk->queue, req, buffer, buffer_length,
 					GFP_KERNEL);
-		if (ret) {
-			blk_mq_free_request(req);
-			goto out;
-		}
+		if (ret)
+			goto erase_desc;
 
 		status = blk_execute_rq(req, 0);
 		ret = blk_status_to_errno(status);
+erase_desc:
+		ublk_zoned_erase_report_desc(req);
+free_req:
 		blk_mq_free_request(req);
 		if (ret)
 			goto out;
@@ -366,7 +393,7 @@ static blk_status_t ublk_setup_iod_zoned(struct ublk_queue *ubq,
 {
 	struct ublksrv_io_desc *iod = ublk_get_iod(ubq, req->tag);
 	struct ublk_io *io = &ubq->ios[req->tag];
-	struct ublk_rq_data *pdu = blk_mq_rq_to_pdu(req);
+	struct ublk_zoned_report_desc *desc;
 	u32 ublk_op;
 
 	switch (req_op(req)) {
@@ -389,12 +416,15 @@ static blk_status_t ublk_setup_iod_zoned(struct ublk_queue *ubq,
 		ublk_op = UBLK_IO_OP_ZONE_RESET_ALL;
 		break;
 	case REQ_OP_DRV_IN:
-		ublk_op = pdu->operation;
+		desc = ublk_zoned_get_report_desc(req);
+		if (!desc)
+			return BLK_STS_IOERR;
+		ublk_op = desc->operation;
 		switch (ublk_op) {
 		case UBLK_IO_OP_REPORT_ZONES:
 			iod->op_flags = ublk_op | ublk_req_build_flags(req);
-			iod->nr_zones = pdu->nr_zones;
-			iod->start_sector = pdu->sector;
+			iod->nr_zones = desc->nr_zones;
+			iod->start_sector = desc->sector;
 			return BLK_STS_OK;
 		default:
 			return BLK_STS_IOERR;
-- 
2.45.2

Re: [PATCH] ublk: move zone report data out of request pdu

[Jens Axboe]

On 8/9/24 10:02 PM, Ming Lei wrote:
> ublk zoned takes 16 bytes in each request pdu just for handling REPORT_ZONE
> operation, this way does waste memory since request pdu is allocated
> statically.
> 
> Store the transient zone report data into one global xarray, and remove
> it after the report zone request is completed. This way is reasonable
> since report zone is run in slow code path.

Oof yes, that's a lot better. I think this warrants a:

Fixes: 29802d7ca33b ("ublk: enable zoned storage support")

as that's a pretty big waste, especially when most would not even be
using the zoned support.

-- 
Jens Axboe

Re: [PATCH] ublk: move zone report data out of request pdu

[Ming Lei]

On Sat, Aug 10, 2024 at 10:27 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 8/9/24 10:02 PM, Ming Lei wrote:
> > ublk zoned takes 16 bytes in each request pdu just for handling REPORT_ZONE
> > operation, this way does waste memory since request pdu is allocated
> > statically.
> >
> > Store the transient zone report data into one global xarray, and remove
> > it after the report zone request is completed. This way is reasonable
> > since report zone is run in slow code path.
>
> Oof yes, that's a lot better. I think this warrants a:
>
> Fixes: 29802d7ca33b ("ublk: enable zoned storage support")
>
> as that's a pretty big waste, especially when most would not even be
> using the zoned support.

Indeed, the patch itself is easy enough for backport, will add Fixes tag in V2.

Thanks,

Re: [PATCH] ublk: move zone report data out of request pdu

[Andreas Hindborg]

Ming Lei <ming.lei@redhat.com> writes:

> ublk zoned takes 16 bytes in each request pdu just for handling REPORT_ZONE
> operation, this way does waste memory since request pdu is allocated
> statically.
>
> Store the transient zone report data into one global xarray, and remove
> it after the report zone request is completed. This way is reasonable
> since report zone is run in slow code path.
>
> Cc: Damien Le Moal <dlemoal@kernel.org>
> Cc: Andreas Hindborg <a.hindborg@samsung.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---

Nice!

Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>


Best regards,
Andreas