From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f43.google.com (mail-wr1-f43.google.com [209.85.221.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E365B37472B for ; Thu, 19 Mar 2026 09:35:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773912912; cv=none; b=J2740e2pz3YbRt2PWYMabBh18JGXfl9ILdvDV2qFPNwFFxJ5XzpSg4uUM/sToF1CVtfaFeXtZFxyV/HM4BX30fDX/QrfX+z4ykGMgKpLN6texC1m+hQml93apXleMXsNqkfGshqTfQJDh7s+gQXylTK8quGDGoymc9P23soNyhg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773912912; c=relaxed/simple; bh=hwNOGXQH89R2fhkpymstdIQmva8Np7BQfA1d47tkCtA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=DuLKtDGtT7hsEJn+dG0t9Yu1soTVjCqgTbTQX5J1H7hqV4pcO9ZzOIImlF3nafiK/HNsHPK26jZy/qP1yOlqpx+b3lXxzdfHLtYfRieu7xlHdBvOvr6r2yteszPtVfwoUknb+m8SlUYL606nXEg5VdzEvbWUt4KnTzGUQm/QYRE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=RKQa8S+Q; arc=none smtp.client-ip=209.85.221.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="RKQa8S+Q" Received: by mail-wr1-f43.google.com with SMTP id ffacd0b85a97d-43b4d73463dso481317f8f.3 for ; Thu, 19 Mar 2026 02:35:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1773912909; x=1774517709; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Zsi6Snfpldbdx/qVF6kvcZDyl3BM2Mth+ewOT8FmjRc=; b=RKQa8S+QA6StoDWl3nxQ3GDmufmRq3T8GLtQMkCqH92aH91z6Ksnuc8/igDHy0bkk0 SrFsxL7k87ESro+1FdvnGdA55NaSyAtq3c+slCmNrYyjcS95Q/6ED5r/deSwInwPaMQ5 yeM5j2q0lqjO4YILnKZp6J4svc1lBT3ZFJbdT0HqCZ2Mntcd/XIc4T1S+l8JCy752/Dv OjO1n37ueuHZ/En9uRmuMTnri2dRTl2j9UhckGi+Cc7iDGCaHUY8jN+g6xjY1O19O1Ic oLiek510v/uM5VuYDgfE64sDZX45/B7zqo+vh5XTBrgxjbBNyA25AhQp5gmTjZbqnmCV 9s7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1773912909; x=1774517709; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Zsi6Snfpldbdx/qVF6kvcZDyl3BM2Mth+ewOT8FmjRc=; b=QT+fNPvv0YSk1hbhtrnDbevAO37ObX+mN03izHHEWLNsL4u2qaX46oz+GBPwjNtUwE rheGoo8lXa0F4p1HvU6U+xuJMu8DcFRavlusgLSW5aby+Ii5L/w4cvwG6UUnvqvrkIeg 0TpQnS03AGqFblg71+QcMljrNVFRFXxlCStxDbg4O8NzRhidXu+CvsosHf33CVAs16aQ US5CFrT5WkdaPuHzBU4l2IgKpAMeQxMwOpAQQ8eLVTYB/IsaPcvvcSu7eJmtGuMSAUJt aUEU63TnI/pl3pkw+cfpzn1xHcgXNRxPC+F3PNS/SoUwTSZBKIt8PXIhnNR76Frgzhjb M2RA== X-Forwarded-Encrypted: i=1; AJvYcCUGKeO+lT+fXSRtiuZ9NBZJUcfMd5iwguq5kH4i7WYM2yHuF1+IiWKp+LKCs+l2sw1IsD3MtOQsnvXhZOw=@vger.kernel.org X-Gm-Message-State: AOJu0YymzgDIcGYLGdxhqoyz0pI+nCvujAuus5/poFii4wsRafudNkip C7V7Bsc04OaEbqK5jvgypPMl9qmSuOcY/2y7Qm29WvyBYjeQStjYVcap X-Gm-Gg: ATEYQzy8F6X9Xi1KTMfa6KYUJp3+fmb/ws3NHCRdfV+Nrfn1KWIJvU6QB79RnreuNaz Gbje/LMO46MutFmkcaFh5Gk6mUsjkbW3LgbiEoslPsHJ5OysGboeYiGZpjkm/Lqk0KpDM8dhctx OBwJcDXSz69w/R4L6po+5Ow01veXdv7WhNz7lYZY4c9BAr2EHZgGPJGZcRgF9J0dZvMtgbRkxAB wjId+bJdTmGc3YLXReLkfGvw+TEaU8Oa4aYJhroifDCLo0P46HoNdtI7ZKxeUSYf0XMfL8z/dJd 6we7L2bnpyG6VEaKAFg5gbUsOe1BB/8AGUm7UNldcD7ToNTRameXllKLNSuMw88eIDXk3MI7XqZ PVqGKXul1//ssywa4jXvaeKJWMhA+1jxfgysabF4eeNDxqZle1JC+TmiaS+Tj9GgwY8vC7uIZOP DE4ntMjw2+e6079q7lRhxt+QXEcWKvtxVKlik7ZDAQJ7eUszrJPWXeVT+JJsTbLwzALssHpWju0 LfelePccCZ/OBUDe/RXSsLaBAey8I2pwlC+oSfrNSBCKroOEAnsd6V7o1sBh6U= X-Received: by 2002:a5d:5848:0:b0:435:9cd5:bb2a with SMTP id ffacd0b85a97d-43b527af569mr11562648f8f.24.1773912908977; Thu, 19 Mar 2026 02:35:08 -0700 (PDT) Received: from mail.gmail.com (lfbn-ren-1-685-61.w81-53.abo.wanadoo.fr. [81.53.253.61]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43b5184957bsm16111061f8f.5.2026.03.19.02.35.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Mar 2026 02:35:08 -0700 (PDT) Date: Thu, 19 Mar 2026 10:35:06 +0100 From: Paul Chaignon To: Hao Sun Cc: Shung-Hsi Yu , Harishankar Vishwanathan , bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, eddyz87@gmail.com, john.fastabend@gmail.com, martin.lau@linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] bpf: Simplify tnum_step() Message-ID: References: <20260318171906.53174-1-hao.sun@inf.ethz.ch> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Thu, Mar 19, 2026 at 10:01:31AM +0100, Hao Sun wrote: Thanks for the cc Shung-Hsi. We were discussing it with Hari who's planning to double check soundness. > [...] > > > - res = w; > > > - } > > > - return res; > > > + d = z - t.value; > > > > A bit of comment explaining would be nice. > > > > The commit message is self-contained; anyone interested can git blame. > But I am unsure. If a detailed description is preferred, I can add a > comment to v2. Not disagreeing about the role of git blame, but it's not a replacement for code comments either. Having just the intuition in comments can help a lot; as you mention below, it's one less indirection ;) In its current form, I find the proof harder to follow than the previous, very verbose version. Maybe there's a good middle ground? > > > > + carry_mask = (1ULL << fls64(d & ~t.mask)) - 1; > > > + inc = ((d | carry_mask | ~t.mask) + 1) & t.mask; > > > > Maybe break out the calculation of inc, give it a name (next_submask?), > > Maybe we can break this down as: > > d = z - t.value; > carry_mask = (1ULL << fls64(d & ~t.mask)) - 1; > filled = d | carry_mask | ~t.mask; > inc = (filled + 1) & t.mask; > > `next_submask` is not precise, as we ored `~t.mask` into it. > It's always hard to name the intermediate result :) > > > and have it as preceding patch? It seems generic enough that I thought > > would have been implemented already, but bitmap_scatter() was the > > closest I could find. > > > > Perhaps could be submitted to include/linux/bitops.h in the future. > > > > The calculation above is direct, adding a bitmap helper intros an indirection. > For me, it means I would have to check out the meaning of the helper > and then get back to understanding the algorithm. I think I agree with Hao in this case. If we can reuse existing helpers, great, but otherwise it's short enough that it's probably easier to keep inlined here.