linux-kernel - Re: [PATCH] block/merge: remove unnecessary min() with UINT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CADUfDZqjDxLctcSpBzjxu8fN9NjrMq9JvKZeJOB__9ZrjyEOOg@mail.gmail.com>
Date: Sun, 16 Feb 2025 14:09:55 -0800
From: Caleb Sander Mateos <csander@...estorage.com>
To: David Laight <david.laight.linux@...il.com>
Cc: Jens Axboe <axboe@...nel.dk>, Keith Busch <kbusch@...nel.org>, linux-block@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] block/merge: remove unnecessary min() with UINT_MAX

On Sat, Feb 15, 2025 at 3:51 AM David Laight
<david.laight.linux@...il.com> wrote:
>
> On Fri, 14 Feb 2025 12:36:36 -0700
> Caleb Sander Mateos <csander@...estorage.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@...estorage.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) {
>