All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shung-Hsi Yu <shung-hsi.yu@suse.com>
To: Vinicius Sampaio <vldsampaio@pm.me>
Cc: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
	 john.fastabend@gmail.com, andrii@kernel.org,
	martin.lau@linux.dev, eddyz87@gmail.com,  song@kernel.org,
	yonghong.song@linux.dev, kpsingh@kernel.org, sdf@fomichev.me,
	 haoluo@google.com, jolsa@kernel.org, tangyazhou518@outlook.com,
	 shenghaoyuan0928@163.com
Subject: Re: [PATCH bpf-next 2/2] bpf: Avoid redundant min()/max() in cnum signed bounds
Date: Wed, 29 Jul 2026 15:59:02 +0800	[thread overview]
Message-ID: <ammvMH-I4088A3AX@u94a> (raw)
In-Reply-To: <20260728015601.1567098-3-vldsampaio@pm.me>

On Tue, Jul 28, 2026 at 01:57:18AM +0000, Vinicius Sampaio wrote:
> This patch simplifies the smin() and smax() calculation by removing the
> integer min and max calculation on the non-overflowing branch. This is a
> valid simplification because when the arc does not cross the signed
> ST_MAX/ST_MIN boundary, the signed order of the endpoints is preserved,
> so base is the signed minimum and base + size is the signed maximum.
> Therefore, the min()/max() calls are redundant.
[...]
> --- a/kernel/bpf/cnum_defs.h
> +++ b/kernel/bpf/cnum_defs.h
> @@ -44,6 +44,9 @@ static inline bool FN(urange_overflow)(struct cnum_t cnum)
>   * cnum{T}_umin / cnum{T}_umax query an unsigned range represented by this cnum.
>   * If cnum represents a range crossing the UT_MAX/0 boundary, the unbound range
>   * [0..UT_MAX] is returned.
> + *
> + * The empty cnum doesn't produce any meaningful value, so you should avoid
> + * calling cnum{T}_umin / cnum{T}_umax with it.
>   */
>  ut FN(umin)(struct cnum_t cnum)
>  {
> @@ -67,19 +70,18 @@ static inline bool FN(srange_overflow)(struct cnum_t cnum)
>   * cnum{T}_smin / cnum{T}_smax query a signed range represented by this cnum.
>   * If cnum represents a range crossing the ST_MAX/ST_MIN boundary, the unbound range
>   * [ST_MIN..ST_MAX] is returned.
> + *
> + * The empty cnum doesn't produce any meaningful value, so you should avoid
> + * calling cnum{T}_smin / cnum{T}_smax with it.

Nit: if we are adding these it would perhaps be helpful to add similar
comment to {u,s}range_overflow as well. AFAICT
{u,s}{min,max,range_overflow} should be the only cnum function that is
not expected to take on an empty cnum.

Thought I'd prefer something along the lines of

  caller should ensure !is_empty(cnum) holds when calling

>   */
>  st FN(smin)(struct cnum_t cnum)
>  {
> -	return FN(srange_overflow)(cnum)
> -	       ? ST_MIN
> -	       : min((st)cnum.base, (st)(cnum.base + cnum.size));
> +	return FN(srange_overflow)(cnum) ? ST_MIN : (st)cnum.base;
>  }
>  
>  st FN(smax)(struct cnum_t cnum)
>  {
> -	return FN(srange_overflow)(cnum)
> -	       ? ST_MAX
> -	       : max((st)cnum.base, (st)(cnum.base + cnum.size));
> +	return FN(srange_overflow)(cnum) ? ST_MAX : (st)(cnum.base + cnum.size);
>  }

Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>

  reply	other threads:[~2026-07-29  7:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28  1:56 [PATCH bpf-next 0/2] Simplify min()/max(), contains() and fix normalize() cnum implementation Vinicius Sampaio
2026-07-28  1:56 ` [PATCH bpf-next 1/2] bpf: Simplify cnum contains() and normalize() implementation Vinicius Sampaio
2026-07-29  6:29   ` Shung-Hsi Yu
2026-07-29 19:15     ` Vinicius Sampaio
2026-07-28  1:57 ` [PATCH bpf-next 2/2] bpf: Avoid redundant min()/max() in cnum signed bounds Vinicius Sampaio
2026-07-29  7:59   ` Shung-Hsi Yu [this message]
2026-07-29 19:21     ` Vinicius Sampaio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ammvMH-I4088A3AX@u94a \
    --to=shung-hsi.yu@suse.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=sdf@fomichev.me \
    --cc=shenghaoyuan0928@163.com \
    --cc=song@kernel.org \
    --cc=tangyazhou518@outlook.com \
    --cc=vldsampaio@pm.me \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.