| Message ID | 20250214193637.234702-1-csander@purestorage.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | block/merge: remove unnecessary min() with UINT_MAX | expand |
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,
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>
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) {
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) { >
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) {
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(-)