| Message ID | 20250228223057.615284-1-csander@purestorage.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | io_uring/rsrc: use rq_data_dir() to compute bvec dir | expand |
On 2/28/25 22:30, Caleb Sander Mateos wrote: > The macro rq_data_dir() already computes a request's data direction. > Use it in place of the if-else to set imu->dir. > > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com> > --- > io_uring/rsrc.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c > index 45bfb37bca1e..3107a03d56b8 100644 > --- a/io_uring/rsrc.c > +++ b/io_uring/rsrc.c > @@ -957,15 +957,11 @@ int io_buffer_register_bvec(struct io_uring_cmd *cmd, struct request *rq, > imu->nr_bvecs = nr_bvecs; > refcount_set(&imu->refs, 1); > imu->release = release; > imu->priv = rq; > imu->is_kbuf = true; > - > - if (op_is_write(req_op(rq))) > - imu->dir = IO_IMU_SOURCE; > - else > - imu->dir = IO_IMU_DEST; > + imu->dir = 1 << rq_data_dir(rq); rq_data_dir returns READ/WRITE, which should be fine, but it'd be nicer to be more explicit unless it's already enforced somewhere else BUILD_BUG_ON(WRITE == ITER_SOURCE); ditto for READ > > bvec = imu->bvec; > rq_for_each_bvec(bv, rq, rq_iter) > *bvec++ = bv; >
On Fri, Feb 28, 2025 at 5:23 PM Pavel Begunkov <asml.silence@gmail.com> wrote: > > On 2/28/25 22:30, Caleb Sander Mateos wrote: > > The macro rq_data_dir() already computes a request's data direction. > > Use it in place of the if-else to set imu->dir. > > > > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com> > > --- > > io_uring/rsrc.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c > > index 45bfb37bca1e..3107a03d56b8 100644 > > --- a/io_uring/rsrc.c > > +++ b/io_uring/rsrc.c > > @@ -957,15 +957,11 @@ int io_buffer_register_bvec(struct io_uring_cmd *cmd, struct request *rq, > > imu->nr_bvecs = nr_bvecs; > > refcount_set(&imu->refs, 1); > > imu->release = release; > > imu->priv = rq; > > imu->is_kbuf = true; > > - > > - if (op_is_write(req_op(rq))) > > - imu->dir = IO_IMU_SOURCE; > > - else > > - imu->dir = IO_IMU_DEST; > > + imu->dir = 1 << rq_data_dir(rq); > > rq_data_dir returns READ/WRITE, which should be fine, but it'd > be nicer to be more explicit unless it's already enforced > somewhere else > > BUILD_BUG_ON(WRITE == ITER_SOURCE); > ditto for READ The definitions of ITER_SOURCE and ITER_DEST seem pretty clear that they are aliases for WRITE/READ: #define ITER_SOURCE 1 // == WRITE #define ITER_DEST 0 // == READ So I assume other code is already relying on this equivalence. Best, Caleb
On 3/1/25 01:42, Caleb Sander Mateos wrote: > On Fri, Feb 28, 2025 at 5:23 PM Pavel Begunkov <asml.silence@gmail.com> wrote: >> >> On 2/28/25 22:30, Caleb Sander Mateos wrote: >>> The macro rq_data_dir() already computes a request's data direction. >>> Use it in place of the if-else to set imu->dir. >>> >>> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com> >>> --- >>> io_uring/rsrc.c | 6 +----- >>> 1 file changed, 1 insertion(+), 5 deletions(-) >>> >>> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c >>> index 45bfb37bca1e..3107a03d56b8 100644 >>> --- a/io_uring/rsrc.c >>> +++ b/io_uring/rsrc.c >>> @@ -957,15 +957,11 @@ int io_buffer_register_bvec(struct io_uring_cmd *cmd, struct request *rq, >>> imu->nr_bvecs = nr_bvecs; >>> refcount_set(&imu->refs, 1); >>> imu->release = release; >>> imu->priv = rq; >>> imu->is_kbuf = true; >>> - >>> - if (op_is_write(req_op(rq))) >>> - imu->dir = IO_IMU_SOURCE; >>> - else >>> - imu->dir = IO_IMU_DEST; >>> + imu->dir = 1 << rq_data_dir(rq); >> >> rq_data_dir returns READ/WRITE, which should be fine, but it'd >> be nicer to be more explicit unless it's already enforced >> somewhere else >> >> BUILD_BUG_ON(WRITE == ITER_SOURCE); >> ditto for READ > > The definitions of ITER_SOURCE and ITER_DEST seem pretty clear that > they are aliases for WRITE/READ: > #define ITER_SOURCE 1 // == WRITE > #define ITER_DEST 0 // == READ > > So I assume other code is already relying on this equivalence. And it'll be left a mystery they weren't defined through WRITE/READ in the first place. but the patch should be fine then.
On Fri, 28 Feb 2025 15:30:56 -0700, Caleb Sander Mateos wrote: > The macro rq_data_dir() already computes a request's data direction. > Use it in place of the if-else to set imu->dir. > > Applied, thanks! [1/1] io_uring/rsrc: use rq_data_dir() to compute bvec dir commit: 2fced37638a897be4e0ac724d93a23a4e38633a6 Best regards,
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c index 45bfb37bca1e..3107a03d56b8 100644 --- a/io_uring/rsrc.c +++ b/io_uring/rsrc.c @@ -957,15 +957,11 @@ int io_buffer_register_bvec(struct io_uring_cmd *cmd, struct request *rq, imu->nr_bvecs = nr_bvecs; refcount_set(&imu->refs, 1); imu->release = release; imu->priv = rq; imu->is_kbuf = true; - - if (op_is_write(req_op(rq))) - imu->dir = IO_IMU_SOURCE; - else - imu->dir = IO_IMU_DEST; + imu->dir = 1 << rq_data_dir(rq); bvec = imu->bvec; rq_for_each_bvec(bv, rq, rq_iter) *bvec++ = bv;
The macro rq_data_dir() already computes a request's data direction. Use it in place of the if-else to set imu->dir. Signed-off-by: Caleb Sander Mateos <csander@purestorage.com> --- io_uring/rsrc.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)