[PATCH] ublk: optimize ublk_rq_has_data()

[PATCH] ublk: optimize ublk_rq_has_data()

[Caleb Sander Mateos]

ublk_rq_has_data() currently uses bio_has_data(), which involves 2
indirections and several branches. Use blk_rq_bytes() instead, which
performs a single indirection with no branches.

Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
---
 drivers/block/ublk_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index ef8a0705e68b..5f9b3c3876f4 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -1172,11 +1172,11 @@ static inline struct ublk_queue *ublk_get_queue(struct ublk_device *dev,
 	return dev->queues[qid];
 }
 
 static inline bool ublk_rq_has_data(const struct request *rq)
 {
-	return bio_has_data(rq->bio);
+	return blk_rq_bytes(rq);
 }
 
 static inline struct ublksrv_io_desc *
 ublk_queue_cmd_buf(struct ublk_device *ub, int q_id)
 {
-- 
2.45.2

Re: [PATCH] ublk: optimize ublk_rq_has_data()

[Ming Lei]

On Tue, Apr 21, 2026 at 01:47:42PM -0600, Caleb Sander Mateos wrote:
> ublk_rq_has_data() currently uses bio_has_data(), which involves 2
> indirections and several branches. Use blk_rq_bytes() instead, which
> performs a single indirection with no branches.
> 
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
>  drivers/block/ublk_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index ef8a0705e68b..5f9b3c3876f4 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -1172,11 +1172,11 @@ static inline struct ublk_queue *ublk_get_queue(struct ublk_device *dev,
>  	return dev->queues[qid];
>  }
>  
>  static inline bool ublk_rq_has_data(const struct request *rq)
>  {
> -	return bio_has_data(rq->bio);
> +	return blk_rq_bytes(rq);
>  }

blk_rq_bytes() doesn't return the actual payload data bytes, such as,
discard command, but bio_has_data() does.


thanks,
Ming

Re: [PATCH] ublk: optimize ublk_rq_has_data()

[Caleb Sander Mateos]

On Tue, Apr 21, 2026 at 4:51 PM Ming Lei <tom.leiming@gmail.com> wrote:
>
> On Tue, Apr 21, 2026 at 01:47:42PM -0600, Caleb Sander Mateos wrote:
> > ublk_rq_has_data() currently uses bio_has_data(), which involves 2
> > indirections and several branches. Use blk_rq_bytes() instead, which
> > performs a single indirection with no branches.
> >
> > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > ---
> >  drivers/block/ublk_drv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index ef8a0705e68b..5f9b3c3876f4 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -1172,11 +1172,11 @@ static inline struct ublk_queue *ublk_get_queue(struct ublk_device *dev,
> >       return dev->queues[qid];
> >  }
> >
> >  static inline bool ublk_rq_has_data(const struct request *rq)
> >  {
> > -     return bio_has_data(rq->bio);
> > +     return blk_rq_bytes(rq);
> >  }
>
> blk_rq_bytes() doesn't return the actual payload data bytes, such as,
> discard command, but bio_has_data() does.

So you're saying that on a discard, rq->__data_len is set to
bio->bi_iter.bi_size, which represents the number of bytes being
trimmed even though there's no data payload? I guess we could check
req_op(rq) here to at least avoid the bio indirection.

Thanks,
Caleb

Re: [PATCH] ublk: optimize ublk_rq_has_data()

[Ming Lei]

On Tue, Apr 21, 2026 at 05:31:40PM -0700, Caleb Sander Mateos wrote:
> On Tue, Apr 21, 2026 at 4:51 PM Ming Lei <tom.leiming@gmail.com> wrote:
> >
> > On Tue, Apr 21, 2026 at 01:47:42PM -0600, Caleb Sander Mateos wrote:
> > > ublk_rq_has_data() currently uses bio_has_data(), which involves 2
> > > indirections and several branches. Use blk_rq_bytes() instead, which
> > > performs a single indirection with no branches.
> > >
> > > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > > ---
> > >  drivers/block/ublk_drv.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > > index ef8a0705e68b..5f9b3c3876f4 100644
> > > --- a/drivers/block/ublk_drv.c
> > > +++ b/drivers/block/ublk_drv.c
> > > @@ -1172,11 +1172,11 @@ static inline struct ublk_queue *ublk_get_queue(struct ublk_device *dev,
> > >       return dev->queues[qid];
> > >  }
> > >
> > >  static inline bool ublk_rq_has_data(const struct request *rq)
> > >  {
> > > -     return bio_has_data(rq->bio);
> > > +     return blk_rq_bytes(rq);
> > >  }
> >
> > blk_rq_bytes() doesn't return the actual payload data bytes, such as,
> > discard command, but bio_has_data() does.
> 
> So you're saying that on a discard, rq->__data_len is set to
> bio->bi_iter.bi_size, which represents the number of bytes being
> trimmed even though there's no data payload? I guess we could check
> req_op(rq) here to at least avoid the bio indirection.

Yeah, you may add blk_has_data() or open-code it.

Thanks,
Ming