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 47CA53016E0 for ; Mon, 18 May 2026 10:24:29 +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=1779099869; cv=none; b=Ywg390Ec0Tc7xloynf5dGNcSfO1O8fnl8a/vqyMrlibXKGruV2fjodwfAr6MOrwFehLuGlTSAHy3GX8doS5TuIhtbWZFpVVgo74yjEFI5qJobwKaLQOmHMdD1rPmAMhFRR929ESboExEJumAI6bnmis3zFQSMnmVBK7CG6RYZWU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779099869; c=relaxed/simple; bh=6Y5stbvQWHXLcA7XjV23eIeFyPchwtbQV2KWt9Ea000=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=i6J3kBD43RkMhoFR/RRUxK0+l1JYHKqlqUX8V1xGlZnwKWlvDnZyCePM4lTfGv2YeYshlXjJr9YgOoGmxod5WLanHklU6z7a5YIt1WwvnrtvtrUD+sPgHUoBfBSpYgMnPL6XUzFjw54mD43UMyScEbdd2F7lc46sl5+FHU8RDBE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=pSv2y11n; 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="pSv2y11n" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E56A7C2BCB7; Mon, 18 May 2026 10:24:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1779099869; bh=6Y5stbvQWHXLcA7XjV23eIeFyPchwtbQV2KWt9Ea000=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=pSv2y11nGB9K2eENajyD7rPgFnLZs4LLW/VdcTdSMcaj+qLfHahfeHz9JYVwSvyt1 R/ywJTAJv+PAlXbwsEEo28us5H4IyGKs+aPOhFNoo+EH/zcKJLRUMc7DYQmwn5exzj ZYW2VRhLwdoJ9AXAz6xCYXbNhks8bU1AlM6bJRc1WY0yfM7YimdLQ2gvNilYuxZm0/ a7W850FWlz87m18MFFFgew5f9Hu8avQSaNQgsziBIf8KxgKV2+2immEyR/NhtGNzvx e/k0AXGqAnVw4bhzUEowPYMzz34cXIx2EinWcFA7JKh3uN9dF4HW890zaQDX3JAA9u MkA6antazQf4A== Date: Mon, 18 May 2026 11:24:23 +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 2/3] mm: add bytes_to_page_end() helper Message-ID: References: <20260517123428.1181981-4-thorsten.blum@linux.dev> <20260517123428.1181981-5-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-5-thorsten.blum@linux.dev> Why is this a separate commit? It's tiny, I don't think it needs to be separated at all. On Sun, May 17, 2026 at 02:34:30PM +0200, Thorsten Blum wrote: > Add bytes_to_page_end() for the common PAGE_SIZE - offset_in_page() > calculation. This is a totally useless commit message. Again you're not indicating why it's needed. Please look around mm to get a sense of how commit messages should look. Also it's so common that you have exactly zero callers updated here or elsewhere? I see uses which are all pretty specific. I wonder if it's really useful to make this its own thing? And it's untyped and etc. etc. yeah, it's a no really to this. > > Suggested-by: Andy Shevchenko Was there a previous discussion that led to this series? You should really give some background somewhere (like a cover letter, and if previous revisions were sent, a history of them below the ---) > Signed-off-by: Thorsten Blum Again I'm really unconvinced you need to do any of this. > --- > include/linux/page_helpers.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/linux/page_helpers.h b/include/linux/page_helpers.h > index 102a4f3c3868..981731faa1fc 100644 > --- a/include/linux/page_helpers.h > +++ b/include/linux/page_helpers.h > @@ -6,5 +6,6 @@ > #include > > #define offset_in_page(p) ((unsigned long)(p) & ~PAGE_MASK) I mean another thing about this (and yes this was existing but since you're proposing making this rather special it's worth raising) is that it's enormously un-typesafe and 'p' is vague, what if somebody put a (struct page *) here? It'd be better if it was a static inline function, maybe there's places which use this already that require it to be a macro but perhaps not. > +#define bytes_to_page_end(p) (PAGE_SIZE - offset_in_page(p)) Here, since, you're adding it, there's no excuse at all for it being a macro. And you're implementing something just as horribly vague as the other form, it's not obvious 'p' is meant to be a pointer. Page begins with p too :) Anyway I'm not really convinced this is useful. > > #endif /* _LINUX_PAGE_HELPERS_H */ Thanks, Lorenzo