From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 52886D65C64 for ; Thu, 14 Nov 2024 09:36:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C267A6B007B; Thu, 14 Nov 2024 04:36:25 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BD70A6B0082; Thu, 14 Nov 2024 04:36:25 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AC54A6B0083; Thu, 14 Nov 2024 04:36:25 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 8FB226B007B for ; Thu, 14 Nov 2024 04:36:25 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 0FEF1C053D for ; Thu, 14 Nov 2024 09:36:25 +0000 (UTC) X-FDA: 82784192862.03.4C54659 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf09.hostedemail.com (Postfix) with ESMTP id 090A9140658 for ; Thu, 14 Nov 2024 09:35:51 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf09.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1731576895; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=mER75Q+4wG3UmVEMwASmKyWbuLRRLJol4qemiObTthM=; b=FqMjs9dlefFaF5EiOqlM1yDtekRKAilWKF9RmD6PXRDmfbzzMuEcUZ33F0/1oImP0AVi22 9Ay6leBGw0vXb22cUPvE4tZRRAdJF0eYbcI341zWzakU1RzQnfc0gpwuAqvC68vaCEsvDN MDbraMYjNIphNCQucD50FxA1iO6mHu4= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf09.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1731576895; a=rsa-sha256; cv=none; b=QAOwYcrSs3CF6e5EUZcQK73lbE8Ch94Oc1XcVkQyN8vij+3o0JL8vn1Fpq7l354p49tTcN efQ8ZUV2FlHdHLDRoLENYsP2wO+vv1qmsnkFuChl0UefY2b++otrW3ThGXmlpSxLP40EZ6 ccYxsDk07DQGo3s41knvtGoSQBNaevE= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 57D061474; Thu, 14 Nov 2024 01:36:51 -0800 (PST) Received: from [10.57.89.178] (unknown [10.57.89.178]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C74D23F59E; Thu, 14 Nov 2024 01:36:18 -0800 (PST) Message-ID: <0aabbc9e-8808-4aaa-a722-49ed3d45d15e@arm.com> Date: Thu, 14 Nov 2024 09:36:17 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH v1 04/57] mm/page_alloc: Make page_frag_cache boot-time page size compatible Content-Language: en-GB To: Vlastimil Babka , Andrew Morton , Anshuman Khandual , Ard Biesheuvel , Catalin Marinas , David Hildenbrand , Greg Marsden , Ivan Ivanov , Kalesh Singh , Marc Zyngier , Mark Rutland , Matthias Brugger , Miroslav Benes , Will Deacon Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20241014105514.3206191-1-ryan.roberts@arm.com> <20241014105912.3207374-1-ryan.roberts@arm.com> <20241014105912.3207374-4-ryan.roberts@arm.com> <1d9f9188-42b1-4f83-87e7-e02a22b67caa@suse.cz> From: Ryan Roberts In-Reply-To: <1d9f9188-42b1-4f83-87e7-e02a22b67caa@suse.cz> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 090A9140658 X-Stat-Signature: bowoj8x4gd76pttny9jra6i69y7wzwed X-HE-Tag: 1731576951-19513 X-HE-Meta: U2FsdGVkX1/myYZ4t6JFvmzOuaDIjDOJyIGExSCphUQL4lgqmGH6YOKVNWkLMM9i5nlJuNn7x8NQyZHS4zoYDggJnP7qnQhZ2jxxBrRY3THAmUBjsuLPzttdsHPh9h6QEr5W0qg5kBynow96gR4rdxKvgzJ90FTQ0cHBxUtmrv9tpZA7miGZLG1y30ctdFZxGoJyto/8BmaCnzmb67abypkI7rVNfY0BDODg4FxbvHKiGkxWSEPZYLeVq9lOQXrSzLiEg2jvbd4LXgDo/38/l2jTkEG0pNyYJNcWMdenyPZVaPqgv6QBwghFcPFIMbbFaNLRgGMwOq616HYi2HsZMx1b4E8FvM4RNIo+FRje2Bv8l5ZojeCL0NkJwEKR22wmL12HRrSVxWM4FUDUhfTO2DHvG8KhXmddzODUmpQlmHQ4s3rG987pVOkLo2OFztO90i3e8VPGKF3jhcx6mn8ll2D2dFwV4PYiEF9h7Zibn/pka2QF0a3MZtRP1rRjAtJ/fcQXBpD5INjXM9sIe2/YhllaXqYC7Hu8wyb/JO7oGC9CpUVzUU8JhouUA3MzW2jJbs/RN1/XtVFmb1VFVnkATqUJ6weq9N6LwFYL7mgNKhb318FweX2Bh1lOqxQx7UrljOtYM+Q9D/0BpYiXOryUyk5MTZL3Ll/dOd351zHkdS3Vu5fVPM8YDp1TPM6YczaY3fcAmh76U++mt+P5ScjKVLY9zztIQMLUMpBDDr5acsaTyjN0ltz6BLZEl4kaKMm9gobge6odXD62cwmOeaA3c/XhPzesJh7Mc3a9QEnRzrpS6UmWZzNcjpGEkykcYCpZ66o9Wl9nhtl43vbVg8fx3F4LmFSa35ylT8ZX0HQhQ5Or61WyRFzh50pOKGwqyQ1fWJJ31iDFsoBfzejHMJ9DUmJZr1sop246q2glxsLlNHYWCXb4b8fraCD3CqeNgHdMEjW8tzzt7rOqhZcvC9a TXlxoqeS 1sRzZruSiAMTTMHMmrlGEc+2u342PN1e+uKZ1xFUVh2JT7a9aP32chWrxrjxsd7uFY+lA5iok1pYAYAclld0LsyrRKZDzprF08OmiTBlHLNdJUDDSSRKXR71xsv8SXBBG34KwWrhfxj3+XmzdzJ91rCcKg/c6jEXZCuNa6w6pZ5iiSOktQC14REy2yGQdrf8kUFKR8v7oo8j5xZhlOybvrQGcKbt7Lx2scgsqyhyupOAqaJAkyE+JaE39+KyU1eYlUocNtRTnwHbg6uThQWkzrXvQTg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 14/11/2024 08:23, Vlastimil Babka wrote: > On 10/14/24 12:58, Ryan Roberts wrote: >> "struct page_frag_cache" has some optimizations that depend on page >> size. Let's refactor it a bit so that those optimizations can be >> determined at run-time for the case where page size is a boot-time >> parameter. For compile-time page size, the compiler should dead code >> strip and the result is very similar to before. >> >> One wrinkle is that we don't know if we need the size member until >> runtime. So remove the ifdeffery and always define offset as u32 (needed >> if PAGE_SIZE is >= 64K) and size as u16 (only used when PAGE_SIZE <= >> 32K). We move the members around a bit so that the overall size of the >> struct remains the same; 24 bytes for 64-bit and 16 bytes on 32 bit. >> >> Signed-off-by: Ryan Roberts > > Looks ok, but ideally the PAGE_FRAG_CACHE_MAX_ORDER #define should also be > replaced by some variable that's populated just once. It can be static local > to page_alloc.c as nothing else seems to use it. I can certainly do that, but wouldn't that be penalizing a compile-time page size configuration? My current change means that PAGE_FRAG_CACHE_MAX_ORDER still resolves to a compile-time constant in that situation and the compiler can eliminate conditional branches it knows will never be taken. Or perhaps you're suggesting I conditionally make it a variable if PAGE_SIZE_MIN != PAGE_SIZE_MAX? Thanks, Ryan > >> >> page_alloc >> --- >> >> ***NOTE*** >> Any confused maintainers may want to read the cover note here for context: >> https://lore.kernel.org/all/20241014105514.3206191-1-ryan.roberts@arm.com/ >> >> include/linux/mm_types.h | 13 ++++++------- >> mm/page_alloc.c | 31 ++++++++++++++++++------------- >> 2 files changed, 24 insertions(+), 20 deletions(-) >> >> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h >> index 4854249792545..0844ed7cfaa53 100644 >> --- a/include/linux/mm_types.h >> +++ b/include/linux/mm_types.h >> @@ -544,16 +544,15 @@ static inline void *folio_get_private(struct folio *folio) >> >> struct page_frag_cache { >> void * va; >> -#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) >> - __u16 offset; >> - __u16 size; >> -#else >> - __u32 offset; >> -#endif >> /* we maintain a pagecount bias, so that we dont dirty cache line >> * containing page->_refcount every time we allocate a fragment. >> */ >> - unsigned int pagecnt_bias; >> + unsigned int pagecnt_bias; >> + __u32 offset; >> + /* size only used when PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE, in which >> + * case PAGE_FRAG_CACHE_MAX_SIZE is 32K and 16 bits is sufficient. >> + */ >> + __u16 size; >> bool pfmemalloc; >> }; >> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index 91ace8ca97e21..8678103b1b396 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -4822,13 +4822,18 @@ static struct page *__page_frag_cache_refill(struct page_frag_cache *nc, >> struct page *page = NULL; >> gfp_t gfp = gfp_mask; >> >> -#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) >> - gfp_mask = (gfp_mask & ~__GFP_DIRECT_RECLAIM) | __GFP_COMP | >> - __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC; >> - page = alloc_pages_node(NUMA_NO_NODE, gfp_mask, >> - PAGE_FRAG_CACHE_MAX_ORDER); >> - nc->size = page ? PAGE_FRAG_CACHE_MAX_SIZE : PAGE_SIZE; >> -#endif >> + if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) { >> + gfp_mask = (gfp_mask & ~__GFP_DIRECT_RECLAIM) | __GFP_COMP | >> + __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC; >> + page = alloc_pages_node(NUMA_NO_NODE, gfp_mask, >> + PAGE_FRAG_CACHE_MAX_ORDER); >> + /* >> + * Cast to silence warning due to 16-bit nc->size. Not real >> + * because PAGE_SIZE only less than PAGE_FRAG_CACHE_MAX_SIZE >> + * when PAGE_FRAG_CACHE_MAX_SIZE is 32K. >> + */ >> + nc->size = (__u16)(page ? PAGE_FRAG_CACHE_MAX_SIZE : PAGE_SIZE); >> + } >> if (unlikely(!page)) >> page = alloc_pages_node(NUMA_NO_NODE, gfp, 0); >> >> @@ -4870,10 +4875,10 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc, >> if (!page) >> return NULL; >> >> -#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) >> /* if size can vary use size else just use PAGE_SIZE */ >> - size = nc->size; >> -#endif >> + if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) >> + size = nc->size; >> + >> /* Even if we own the page, we do not use atomic_set(). >> * This would break get_page_unless_zero() users. >> */ >> @@ -4897,10 +4902,10 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc, >> goto refill; >> } >> >> -#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) >> /* if size can vary use size else just use PAGE_SIZE */ >> - size = nc->size; >> -#endif >> + if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) >> + size = nc->size; >> + >> /* OK, page count is 0, we can safely set it */ >> set_page_count(page, PAGE_FRAG_CACHE_MAX_SIZE + 1); >> >