block/merge: remove unnecessary min() with UINT_MAX

[PATCH] block/merge: remove unnecessary min() with UINT_MAX

Posted by Caleb Sander Mateos 10 months, 1 week ago

In bvec_split_segs(), max_bytes is an unsigned, so it must be less than
or equal to UINT_MAX. Remove the unnecessary min().

Prior to commit 67927d220150 ("block/merge: count bytes instead of
sectors"), the min() was with UINT_MAX >> 9, so it did have an effect.

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

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 15cd231d560c..39b738c0e4c9 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -268,11 +268,11 @@ static inline unsigned get_max_segment_size(const struct queue_limits *lim,
  */
 static bool bvec_split_segs(const struct queue_limits *lim,
 		const struct bio_vec *bv, unsigned *nsegs, unsigned *bytes,
 		unsigned max_segs, unsigned max_bytes)
 {
-	unsigned max_len = min(max_bytes, UINT_MAX) - *bytes;
+	unsigned max_len = max_bytes - *bytes;
 	unsigned len = min(bv->bv_len, max_len);
 	unsigned total_len = 0;
 	unsigned seg_size = 0;
 
 	while (len && *nsegs < max_segs) {
-- 
2.45.2

Re: [PATCH] block/merge: remove unnecessary min() with UINT_MAX

Posted by David Laight 10 months, 1 week ago

On Fri, 14 Feb 2025 12:36:36 -0700
Caleb Sander Mateos <csander@purestorage.com> wrote:

> In bvec_split_segs(), max_bytes is an unsigned, so it must be less than
> or equal to UINT_MAX. Remove the unnecessary min().
> 
> Prior to commit 67927d220150 ("block/merge: count bytes instead of
> sectors"), the min() was with UINT_MAX >> 9, so it did have an effect.
> 
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
>  block/blk-merge.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 15cd231d560c..39b738c0e4c9 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -268,11 +268,11 @@ static inline unsigned get_max_segment_size(const struct queue_limits *lim,
>   */
>  static bool bvec_split_segs(const struct queue_limits *lim,
>  		const struct bio_vec *bv, unsigned *nsegs, unsigned *bytes,
>  		unsigned max_segs, unsigned max_bytes)
>  {
> -	unsigned max_len = min(max_bytes, UINT_MAX) - *bytes;
> +	unsigned max_len = max_bytes - *bytes;

More interestingly, what stops *bytes being larger than max_bytes?

	David

>  	unsigned len = min(bv->bv_len, max_len);
>  	unsigned total_len = 0;
>  	unsigned seg_size = 0;
>  
>  	while (len && *nsegs < max_segs) {

Re: [PATCH] block/merge: remove unnecessary min() with UINT_MAX

Posted by Caleb Sander Mateos 10 months ago

On Sat, Feb 15, 2025 at 3:51 AM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Fri, 14 Feb 2025 12:36:36 -0700
> Caleb Sander Mateos <csander@purestorage.com> wrote:
>
> > In bvec_split_segs(), max_bytes is an unsigned, so it must be less than
> > or equal to UINT_MAX. Remove the unnecessary min().
> >
> > Prior to commit 67927d220150 ("block/merge: count bytes instead of
> > sectors"), the min() was with UINT_MAX >> 9, so it did have an effect.
> >
> > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > ---
> >  block/blk-merge.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/block/blk-merge.c b/block/blk-merge.c
> > index 15cd231d560c..39b738c0e4c9 100644
> > --- a/block/blk-merge.c
> > +++ b/block/blk-merge.c
> > @@ -268,11 +268,11 @@ static inline unsigned get_max_segment_size(const struct queue_limits *lim,
> >   */
> >  static bool bvec_split_segs(const struct queue_limits *lim,
> >               const struct bio_vec *bv, unsigned *nsegs, unsigned *bytes,
> >               unsigned max_segs, unsigned max_bytes)
> >  {
> > -     unsigned max_len = min(max_bytes, UINT_MAX) - *bytes;
> > +     unsigned max_len = max_bytes - *bytes;
>
> More interestingly, what stops *bytes being larger than max_bytes?

bvec_split_segs() has two callers, bio_split_rw_at() and
blk_recalc_rq_segments(). In both, *bytes = 0 originally.
bio_split_rw_at() has another code path which increments *bytes, but
it makes sure not to exceed max_bytes. So when bvec_split_segs() is
called, *bytes <= max_bytes. The logic in bvec_split_segs() won't
increment *bytes by more than len, which is at most max_len =
max_bytes - *bytes. So inductively, *bytes <= max_bytes before every
call to bvec_split_segs().

>
>         David
>
> >       unsigned len = min(bv->bv_len, max_len);
> >       unsigned total_len = 0;
> >       unsigned seg_size = 0;
> >
> >       while (len && *nsegs < max_segs) {
>

Re: [PATCH] block/merge: remove unnecessary min() with UINT_MAX

Posted by John Garry 10 months, 1 week ago

On 14/02/2025 19:36, Caleb Sander Mateos wrote:
> In bvec_split_segs(), max_bytes is an unsigned, so it must be less than
> or equal to UINT_MAX. Remove the unnecessary min().
> 
> Prior to commit 67927d220150 ("block/merge: count bytes instead of
> sectors"), the min() was with UINT_MAX >> 9, so it did have an effect.
> 
> Signed-off-by: Caleb Sander Mateos<csander@purestorage.com>

FWIW:
Reviewed-by: John Garry <john.g.garry@oracle.com>

Re: [PATCH] block/merge: remove unnecessary min() with UINT_MAX

Posted by Jens Axboe 10 months, 1 week ago

On Fri, 14 Feb 2025 12:36:36 -0700, Caleb Sander Mateos wrote:
> In bvec_split_segs(), max_bytes is an unsigned, so it must be less than
> or equal to UINT_MAX. Remove the unnecessary min().
> 
> Prior to commit 67927d220150 ("block/merge: count bytes instead of
> sectors"), the min() was with UINT_MAX >> 9, so it did have an effect.
> 
> 
> [...]

Applied, thanks!

[1/1] block/merge: remove unnecessary min() with UINT_MAX
      commit: 43c70b104093c324b1a000762ce943d16ce788f9

Best regards,
-- 
Jens Axboe