From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 09D6C3F23CF for ; Mon, 18 May 2026 10:33:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779100412; cv=none; b=SEmCl5pM9lrxHFgLaZuJo0Ofh0ebqORkh/1NnBLovK9KJIYVIDc0sW6DfMzb06JZ0SOc4aG6Umpx7urzWz5rf5V1i0uMELku+Cht8T1vfDJXs1KiGp1RWCsceb5dMesBh0PAUVTHo9Tb1+a3OJz7PRUlDDtv2Y922Mti0oykjek= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779100412; c=relaxed/simple; bh=u93+sg/d4a/yYhdFKkbXPqWWlGAKMuSIVbj8UC10Akc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lk8nPvCZoPmcUCQgXLAwudxY0+mQqMKQEBE7CmvB8+Je9m1eHOkjFieVmbqsmovI/ZGpo6xAg7JIJR4MZT0Ygf1Uom9nSNte7ccpadVLnxVqmWl7mijnzAfpVVNPgUP14TMM58zdX3ZVR4n9RO1G1mmpC0ObfdAFpG7NJZNRVTM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=VLVkSYKG; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="VLVkSYKG" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8ADDEC2BCC6; Mon, 18 May 2026 10:33:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1779100410; bh=u93+sg/d4a/yYhdFKkbXPqWWlGAKMuSIVbj8UC10Akc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=VLVkSYKGhlOIw9YJkL938jmsDiqk6f2uyJLs3o0oBRFaVCY8p98MNs5ttwnOF85Gl 1352rh3QR/kX3Si1BI8Ljc+AtOETgVhYXzh0L+r/1Va1M1Io/5bqVbT2Es/qX1nLC8 KZAb2fKwOPjYefzh3zOjJQsxD/hwpizZVIjaDKA4GYyDQQih1BvokkqAQ9otFi43ww NKnm/swgf8Vf59lzxvQvKtX9PrMYoZIynpzRX1kR5jw60wSqYgVlACBENJr03fNQUo EnegsOss8k9G/2v9GDhsyvTH4M4kCzcK8YNQS0XFDekmcSB2JBLyAdx1qD2DHiNgFy c1dsm4vHzcCbQ== Date: Mon, 18 May 2026 11:33:24 +0100 From: Lorenzo Stoakes To: Thorsten Blum Cc: Andrew Morton , David Hildenbrand , "Liam R. Howlett" , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , Yury Norov , Rasmus Villemoes , Andy Shevchenko , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH 3/3] lib/bitmap: use bytes_to_page_end() helper Message-ID: References: <20260517123428.1181981-4-thorsten.blum@linux.dev> <20260517123428.1181981-6-thorsten.blum@linux.dev> 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: <20260517123428.1181981-6-thorsten.blum@linux.dev> On Sun, May 17, 2026 at 02:34:31PM +0200, Thorsten Blum wrote: > bitmap-str.c includes linux/mm.h for offset_in_page() and kfree(). I'd specifically say that it's including linux/mm.h which in turn includes linux/slab.h and that you're fixing this, because saying you include mm.h for kfree() is a little confusing. > Instead, include linux/page_helpers.h and linux/slab.h directly, and > use bytes_to_page_end() to simplify the code. This is again, a useless commit message. You're not explaining why what for, how etc., you're putting in words what the code is doing. And I'm still very confused as to what the motive is here overall. What's so problematic about including that header? Longer compile times, somehow? If so, what are the measurements before/after this change? What was the problem that led to it? Etc. We could end up with hundreds of super specific headers files for things, so there really has to be a good reason for it. > > Signed-off-by: Thorsten Blum > --- > lib/bitmap-str.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/lib/bitmap-str.c b/lib/bitmap-str.c > index be745209507a..bf245a3eae4a 100644 > --- a/lib/bitmap-str.c > +++ b/lib/bitmap-str.c > @@ -7,7 +7,8 @@ > #include > #include > #include > -#include > +#include > +#include > #include > > #include "kstrtox.h" > @@ -58,7 +59,7 @@ EXPORT_SYMBOL(bitmap_parse_user); > int bitmap_print_to_pagebuf(bool list, char *buf, const unsigned long *maskp, > int nmaskbits) > { > - ptrdiff_t len = PAGE_SIZE - offset_in_page(buf); > + ptrdiff_t len = bytes_to_page_end(buf); yeah this is kind of nasty, going from a clear thing to a 'I wonder how that is implemented' mystery meat function. I think the original is better. > > return list ? scnprintf(buf, len, "%*pbl\n", nmaskbits, maskp) : > scnprintf(buf, len, "%*pb\n", nmaskbits, maskp); Thanks, Lorenzo