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 1805B369985 for ; Mon, 18 May 2026 14:03:31 +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=1779113012; cv=none; b=oH8Kb8QOmBOdf6E+A8S3439M6bMiINi+7THNuXdjBb0pXpSu9/ezpvYpgfFoua9d5zLdz61B8ekz7K+D5ibVobVGymmJkuGHIxG4LoW8hCmf/70O+Sonb5WVZ54Z/Kjlo4f6U8Kbdmp1Xth63+thMyOLWrsiXpTqfO4yevtSbVQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779113012; c=relaxed/simple; bh=3cZJ13NVu4Ev2IK9I7mtGoNX5RlwBtAx7pjwi4jHRO8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=MRH7ddllpZ3n8Qcjwj62bbHgoqr+e+WBzgk6KLuRPV0RnvE6HgW/Cyo7sAaWIKlF2L/b+lBDciGkUkIvoRHs7+hV9A4gtRQ0t3ys/4u/xa4/a5rjJiCFd4LwpO3qQ6oycsq/WIdGznUoC6qNmUn06hStr/Gu3c5cLAc7FG+lsqA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=oOZxJzBi; 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="oOZxJzBi" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A2F75C2BCB7; Mon, 18 May 2026 14:03:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1779113011; bh=3cZJ13NVu4Ev2IK9I7mtGoNX5RlwBtAx7pjwi4jHRO8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=oOZxJzBi+y79WYD7vRPhVl4nlVY01anjd1t4JSBLOP2+g+8LBmzL05Fg3WP2yir0Z sls6QBV3GuQOo7yr8Yhl1knVixYgCx1ci65KNKfFaxEgbI7OBN8iXzxzfBZAjiJ8ZU +YGZQMzDV4OeFuzoDhv5p5HPTb51YIUCqHSgzJU3NfIOPq17pn/QYwOxA6zgZaDFZN usTAplYlAe0fC7p6lS3bdLxpOWtd0c7rBVhw2RI/aIt+uDOiwgW8ExhIPp8fAzY38w HbSBJo7PQ53DLkXDgGDQ5SxnpkAASVzT4b+mHvue6zo6gM2ZUzO5J9gr+XGY4kJQoW qo0mFHE0wAgug== Date: Mon, 18 May 2026 15:03:21 +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 1/3] mm: move offset_in_page() to page_helpers.h Message-ID: References: <20260517123428.1181981-4-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: On Mon, May 18, 2026 at 03:36:14PM +0200, Thorsten Blum wrote: > On Mon, May 18, 2026 at 01:33:54PM +0100, Lorenzo Stoakes wrote: > > On Mon, May 18, 2026 at 02:13:54PM +0200, Thorsten Blum wrote: > > > [...] > > > Patch 3/3 is one of many examples that pulls in all of mm.h just for > > > offset_in_page(). lib/string.c from the same thread [1] is another > > > example that would need to include mm.h just for offset_in_page(). > > > > And that's a problem why? Compile time? Somehow binary bloat? Conflicts? > > Confusion? > > This is more about separation of concerns than about compile time, which > can still matter here, since mm.h is large, pulls in many other headers, > and any changes to those can cause unrelated files to be recompiled. > > I don't mind including mm.h where the code already depends on MM > interfaces, but offset_in_page() itself is a small page-arithmetic > helper. Having it buried 3000+ lines into mm.h also makes it less > discoverable, which may be one reason it's not used more often. See the side thread with David, putting it in vdso/page.h is fine, and should achieve your aims. > > > > Many other files (hundreds) don't use offset_in_page(), but open-code it > > > in many different ways instead: > > > > > > (unsigned long)p & ~PAGE_MASK > > > (unsigned long)p & (PAGE_SIZE - 1) > > > (long)p & (PAGE_SIZE - 1) > > > ... > > > > > > I can't tell whether they didn't know about offset_in_page(), or > > > deliberately chose not to include mm.h. > > > > OK but this series doesn't address any of that? It just adds a new header, a > > questionable macro and uses it in one place? So those justifications are rather > > moot here. > > My intent was to make the helper available from a smaller header first, > and to use patch 3/3 as a concrete example where including all of mm.h > is unnecessary. Other call sites can then be converted incrementally > over time, rather than sending a tree-wide conversion in the same > series. Yeah I'm not sure it's at all clear anybody will pay attention to that (by not sure I mean - happy to bet a beer or 2 against :) Anyway I'm not going to demand that you update all the callsites, if you split this into 2 patches as per above + send w/cover letter this is fine. > > Thanks, > Thorsten Thanks, Lorenzo