diff mbox series

blk-integrity: remove seed for user mapped buffers

Message ID 20241016201309.1090320-1-kbusch@meta.com (mailing list archive)
State New, archived
Headers show
Series blk-integrity: remove seed for user mapped buffers | expand

Commit Message

Keith Busch Oct. 16, 2024, 8:13 p.m. UTC
From: Keith Busch <kbusch@kernel.org>

The seed is only used for kernel generation and verification. That
doesn't happen for user buffers, so passing the seed around doesn't
accomplish anything.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 block/bio-integrity.c         | 13 +++++--------
 block/blk-integrity.c         |  4 ++--
 drivers/nvme/host/ioctl.c     | 17 ++++++++---------
 include/linux/bio-integrity.h |  4 ++--
 include/linux/blk-integrity.h |  5 ++---
 5 files changed, 19 insertions(+), 24 deletions(-)

Comments

Christoph Hellwig Oct. 17, 2024, 5:57 a.m. UTC | #1
On Wed, Oct 16, 2024 at 01:13:09PM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> The seed is only used for kernel generation and verification. That
> doesn't happen for user buffers, so passing the seed around doesn't
> accomplish anything.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Anuj Gupta Oct. 17, 2024, 6:01 a.m. UTC | #2
On Wed, Oct 16, 2024 at 01:13:09PM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> The seed is only used for kernel generation and verification. That
> doesn't happen for user buffers, so passing the seed around doesn't
> accomplish anything.
>

Not for nvme passthrough, but all this code is needed for io_uring
metadata series. Please see the need/justifcation [*].

[*] https://lore.kernel.org/linux-block/20241017054900.alfiqn3o37f4kkxb@green245/
Christoph Hellwig Oct. 17, 2024, 8:21 a.m. UTC | #3
On Thu, Oct 17, 2024 at 11:31:23AM +0530, Anuj Gupta wrote:
> Not for nvme passthrough, but all this code is needed for io_uring
> metadata series. Please see the need/justifcation [*].
> 
> [*] https://lore.kernel.org/linux-block/20241017054900.alfiqn3o37f4kkxb@green245/

But it could easily set it in bio_integrity_map_user.
Anuj Gupta Oct. 29, 2024, 11:57 a.m. UTC | #4
On Wed, Oct 16, 2024 at 01:13:09PM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> The seed is only used for kernel generation and verification. That
> doesn't happen for user buffers, so passing the seed around doesn't
> accomplish anything.

Looks fine. I can do the seed handling for io_uring metadata series
in the upper block layer functions, and hence won't rely on this
infra.

Reviewed-by: Anuj Gupta <anuj20.g@samsung.com>

> -- 
> 2.43.5
> 
>
Kanchan Joshi Oct. 30, 2024, 1:38 p.m. UTC | #5
Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>

Jens,
Please see if this can be picked.
Helps to reduce a dependency for io_uring metadata series.
Jens Axboe Oct. 30, 2024, 1:47 p.m. UTC | #6
On 10/30/24 7:38 AM, Kanchan Joshi wrote:
> Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
> 
> Jens,
> Please see if this can be picked.
> Helps to reduce a dependency for io_uring metadata series.

Keith, is this queued in the nvme tree?
Jens Axboe Oct. 30, 2024, 1:49 p.m. UTC | #7
On Wed, 16 Oct 2024 13:13:09 -0700, Keith Busch wrote:
> The seed is only used for kernel generation and verification. That
> doesn't happen for user buffers, so passing the seed around doesn't
> accomplish anything.
> 
> 

Applied, thanks!

[1/1] blk-integrity: remove seed for user mapped buffers
      commit: 133008e84b99e4f5f8cf3d8b768c995732df9406

Best regards,
Jens Axboe Oct. 30, 2024, 1:50 p.m. UTC | #8
On 10/30/24 7:47 AM, Jens Axboe wrote:
> On 10/30/24 7:38 AM, Kanchan Joshi wrote:
>> Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
>>
>> Jens,
>> Please see if this can be picked.
>> Helps to reduce a dependency for io_uring metadata series.
> 
> Keith, is this queued in the nvme tree?

Doesn't look like it is, picked it up.
Kanchan Joshi Oct. 30, 2024, 1:59 p.m. UTC | #9
On 10/30/2024 7:20 PM, Jens Axboe wrote:
> Doesn't look like it is, picked it up.

Thanks.
Keith Busch Oct. 30, 2024, 2:01 p.m. UTC | #10
On Wed, Oct 30, 2024 at 07:50:12AM -0600, Jens Axboe wrote:
> On 10/30/24 7:47 AM, Jens Axboe wrote:
> > On 10/30/24 7:38 AM, Kanchan Joshi wrote:
> >> Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
> >>
> >> Jens,
> >> Please see if this can be picked.
> >> Helps to reduce a dependency for io_uring metadata series.
> > 
> > Keith, is this queued in the nvme tree?
> 
> Doesn't look like it is, picked it up.

Thanks. It has block api changes, so I usually don't presume to apply
those patches through our nvme tree.
diff mbox series

Patch

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 88e3ad73c3854..2a4bd66116920 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -199,7 +199,7 @@  EXPORT_SYMBOL(bio_integrity_add_page);
 
 static int bio_integrity_copy_user(struct bio *bio, struct bio_vec *bvec,
 				   int nr_vecs, unsigned int len,
-				   unsigned int direction, u32 seed)
+				   unsigned int direction)
 {
 	bool write = direction == ITER_SOURCE;
 	struct bio_integrity_payload *bip;
@@ -247,7 +247,6 @@  static int bio_integrity_copy_user(struct bio *bio, struct bio_vec *bvec,
 	}
 
 	bip->bip_flags |= BIP_COPY_USER;
-	bip->bip_iter.bi_sector = seed;
 	bip->bip_vcnt = nr_vecs;
 	return 0;
 free_bip:
@@ -258,7 +257,7 @@  static int bio_integrity_copy_user(struct bio *bio, struct bio_vec *bvec,
 }
 
 static int bio_integrity_init_user(struct bio *bio, struct bio_vec *bvec,
-				   int nr_vecs, unsigned int len, u32 seed)
+				   int nr_vecs, unsigned int len)
 {
 	struct bio_integrity_payload *bip;
 
@@ -267,7 +266,6 @@  static int bio_integrity_init_user(struct bio *bio, struct bio_vec *bvec,
 		return PTR_ERR(bip);
 
 	memcpy(bip->bip_vec, bvec, nr_vecs * sizeof(*bvec));
-	bip->bip_iter.bi_sector = seed;
 	bip->bip_iter.bi_size = len;
 	bip->bip_vcnt = nr_vecs;
 	return 0;
@@ -303,8 +301,7 @@  static unsigned int bvec_from_pages(struct bio_vec *bvec, struct page **pages,
 	return nr_bvecs;
 }
 
-int bio_integrity_map_user(struct bio *bio, void __user *ubuf, ssize_t bytes,
-			   u32 seed)
+int bio_integrity_map_user(struct bio *bio, void __user *ubuf, ssize_t bytes)
 {
 	struct request_queue *q = bdev_get_queue(bio->bi_bdev);
 	unsigned int align = blk_lim_dma_alignment_and_pad(&q->limits);
@@ -350,9 +347,9 @@  int bio_integrity_map_user(struct bio *bio, void __user *ubuf, ssize_t bytes,
 
 	if (copy)
 		ret = bio_integrity_copy_user(bio, bvec, nr_bvecs, bytes,
-					      direction, seed);
+					      direction);
 	else
-		ret = bio_integrity_init_user(bio, bvec, nr_bvecs, bytes, seed);
+		ret = bio_integrity_init_user(bio, bvec, nr_bvecs, bytes);
 	if (ret)
 		goto release_pages;
 	if (bvec != stack_vec)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 83b696ba0cac3..b180cac61a9dd 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -113,9 +113,9 @@  int blk_rq_map_integrity_sg(struct request *rq, struct scatterlist *sglist)
 EXPORT_SYMBOL(blk_rq_map_integrity_sg);
 
 int blk_rq_integrity_map_user(struct request *rq, void __user *ubuf,
-			      ssize_t bytes, u32 seed)
+			      ssize_t bytes)
 {
-	int ret = bio_integrity_map_user(rq->bio, ubuf, bytes, seed);
+	int ret = bio_integrity_map_user(rq->bio, ubuf, bytes);
 
 	if (ret)
 		return ret;
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index b9b79ccfabf8a..f697d2d1d7e42 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -114,7 +114,7 @@  static struct request *nvme_alloc_user_request(struct request_queue *q,
 
 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)
+		struct io_uring_cmd *ioucmd, unsigned int flags)
 {
 	struct request_queue *q = req->q;
 	struct nvme_ns *ns = q->queuedata;
@@ -152,8 +152,7 @@  static int nvme_map_user_request(struct request *req, u64 ubuffer,
 		bio_set_dev(bio, bdev);
 
 	if (has_metadata) {
-		ret = blk_rq_integrity_map_user(req, meta_buffer, meta_len,
-						meta_seed);
+		ret = blk_rq_integrity_map_user(req, meta_buffer, meta_len);
 		if (ret)
 			goto out_unmap;
 	}
@@ -170,7 +169,7 @@  static int nvme_map_user_request(struct request *req, u64 ubuffer,
 
 static int nvme_submit_user_cmd(struct request_queue *q,
 		struct nvme_command *cmd, u64 ubuffer, unsigned bufflen,
-		void __user *meta_buffer, unsigned meta_len, u32 meta_seed,
+		void __user *meta_buffer, unsigned meta_len,
 		u64 *result, unsigned timeout, unsigned int flags)
 {
 	struct nvme_ns *ns = q->queuedata;
@@ -187,7 +186,7 @@  static int nvme_submit_user_cmd(struct request_queue *q,
 	req->timeout = timeout;
 	if (ubuffer && bufflen) {
 		ret = nvme_map_user_request(req, ubuffer, bufflen, meta_buffer,
-				meta_len, meta_seed, NULL, flags);
+				meta_len, NULL, flags);
 		if (ret)
 			return ret;
 	}
@@ -268,7 +267,7 @@  static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 	c.rw.lbatm = cpu_to_le16(io.appmask);
 
 	return nvme_submit_user_cmd(ns->queue, &c, io.addr, length, metadata,
-			meta_len, lower_32_bits(io.slba), NULL, 0, 0);
+			meta_len, NULL, 0, 0);
 }
 
 static bool nvme_validate_passthru_nsid(struct nvme_ctrl *ctrl,
@@ -323,7 +322,7 @@  static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 
 	status = nvme_submit_user_cmd(ns ? ns->queue : ctrl->admin_q, &c,
 			cmd.addr, cmd.data_len, nvme_to_user_ptr(cmd.metadata),
-			cmd.metadata_len, 0, &result, timeout, 0);
+			cmd.metadata_len, &result, timeout, 0);
 
 	if (status >= 0) {
 		if (put_user(result, &ucmd->result))
@@ -370,7 +369,7 @@  static int nvme_user_cmd64(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 
 	status = nvme_submit_user_cmd(ns ? ns->queue : ctrl->admin_q, &c,
 			cmd.addr, cmd.data_len, nvme_to_user_ptr(cmd.metadata),
-			cmd.metadata_len, 0, &cmd.result, timeout, flags);
+			cmd.metadata_len, &cmd.result, timeout, flags);
 
 	if (status >= 0) {
 		if (put_user(cmd.result, &ucmd->result))
@@ -504,7 +503,7 @@  static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 	if (d.addr && d.data_len) {
 		ret = nvme_map_user_request(req, d.addr,
 			d.data_len, nvme_to_user_ptr(d.metadata),
-			d.metadata_len, 0, ioucmd, vec);
+			d.metadata_len, ioucmd, vec);
 		if (ret)
 			return ret;
 	}
diff --git a/include/linux/bio-integrity.h b/include/linux/bio-integrity.h
index dd831c269e994..dbf0f74c15291 100644
--- a/include/linux/bio-integrity.h
+++ b/include/linux/bio-integrity.h
@@ -72,7 +72,7 @@  struct bio_integrity_payload *bio_integrity_alloc(struct bio *bio, gfp_t gfp,
 		unsigned int nr);
 int bio_integrity_add_page(struct bio *bio, struct page *page, unsigned int len,
 		unsigned int offset);
-int bio_integrity_map_user(struct bio *bio, void __user *ubuf, ssize_t len, u32 seed);
+int bio_integrity_map_user(struct bio *bio, void __user *ubuf, ssize_t len);
 void bio_integrity_unmap_user(struct bio *bio);
 bool bio_integrity_prep(struct bio *bio);
 void bio_integrity_advance(struct bio *bio, unsigned int bytes_done);
@@ -99,7 +99,7 @@  static inline void bioset_integrity_free(struct bio_set *bs)
 }
 
 static inline int bio_integrity_map_user(struct bio *bio, void __user *ubuf,
-					 ssize_t len, u32 seed)
+					 ssize_t len)
 {
 	return -EINVAL;
 }
diff --git a/include/linux/blk-integrity.h b/include/linux/blk-integrity.h
index 676f8f860c474..c7eae0bfb013f 100644
--- a/include/linux/blk-integrity.h
+++ b/include/linux/blk-integrity.h
@@ -28,7 +28,7 @@  static inline bool queue_limits_stack_integrity_bdev(struct queue_limits *t,
 int blk_rq_map_integrity_sg(struct request *, struct scatterlist *);
 int blk_rq_count_integrity_sg(struct request_queue *, struct bio *);
 int blk_rq_integrity_map_user(struct request *rq, void __user *ubuf,
-			      ssize_t bytes, u32 seed);
+			      ssize_t bytes);
 
 static inline bool
 blk_integrity_queue_supports_integrity(struct request_queue *q)
@@ -104,8 +104,7 @@  static inline int blk_rq_map_integrity_sg(struct request *q,
 }
 static inline int blk_rq_integrity_map_user(struct request *rq,
 					    void __user *ubuf,
-					    ssize_t bytes,
-					    u32 seed)
+					    ssize_t bytes)
 {
 	return -EINVAL;
 }