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 4F24BCF11F5 for ; Thu, 10 Oct 2024 14:34:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C05A76B0082; Thu, 10 Oct 2024 10:34:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B8F5E6B0088; Thu, 10 Oct 2024 10:34:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A08476B0089; Thu, 10 Oct 2024 10:34:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 802986B0082 for ; Thu, 10 Oct 2024 10:34:08 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id DADBB1405DA for ; Thu, 10 Oct 2024 14:34:04 +0000 (UTC) X-FDA: 82657937376.05.851C526 Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by imf12.hostedemail.com (Postfix) with ESMTP id 3B13D40007 for ; Thu, 10 Oct 2024 14:34:04 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=a3qcwVcr; spf=pass (imf12.hostedemail.com: domain of alexander.duyck@gmail.com designates 209.85.128.49 as permitted sender) smtp.mailfrom=alexander.duyck@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728570708; 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:dkim-signature; bh=VRcqszNxyUIAAOVOEfCGxTgUEsOz9kwyuMwhZN6S4qM=; b=T5Qozmu8UCJ9xZT0Qg1cRARqUpa8ZAfyA68Sc45OkDKEzboWSzoI6XYXDNw00xvKlEuDNA juaMvt5mkAlsx3jj78dbkzyqe0ABy+bQf2rMhcBNzEPwKTnvhKxaRn701FkBLA7NFvatwr F6oIhlIgMJTWlj1z347eDcUIVVRjOAw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728570708; a=rsa-sha256; cv=none; b=q4pLRo+RA8TiQdM9kJ5LvFmYqntCuHxabTLlIax5N4tneW0T23yck/ifH/pEB7rh5wjXDR c9RGKIE9aGa73EPyuBhyP28AqN1K1CHclLk7+B8yn3sDVgMKqvYmAbY8hlBePneJN2UP7X oDZfjFZzhgDShWeoLNMUAoPAZ6h6MAE= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=a3qcwVcr; spf=pass (imf12.hostedemail.com: domain of alexander.duyck@gmail.com designates 209.85.128.49 as permitted sender) smtp.mailfrom=alexander.duyck@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-wm1-f49.google.com with SMTP id 5b1f17b1804b1-43115887867so6823555e9.0 for ; Thu, 10 Oct 2024 07:34:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1728570845; x=1729175645; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=VRcqszNxyUIAAOVOEfCGxTgUEsOz9kwyuMwhZN6S4qM=; b=a3qcwVcrVimutAlK+14U8eSO2r+EsVFeLryPH0sUGQmScjqhvh2woAfFTW8peKA6pn iZ3QpiVRydz6kxglVv3/dnJ35SQseWn//Kt3DM3Gs/rDRdn6iB1zM+h+iof2v5WJgtVf kXtBUZJchpZG0k/M87x6ONsP6CA1hW+tnY1KpYfpIfWxpt3sXDv/u0z60FEn8nrK4329 H0EzLt+FvGZ/ItTBQrG+1zC42JR3uyDefbYBjUNNQF8VuLDglhKlY01ILWZRmXIf75Od MXkN8FCnb215YjEYjp7LvREMoReoRB4pcpyR0uDpXcbSNm51JSVr+/R96QY4ydihV2uO crOg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728570845; x=1729175645; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=VRcqszNxyUIAAOVOEfCGxTgUEsOz9kwyuMwhZN6S4qM=; b=KB1mtyxmMPkjAqkzc01MzpCnWyEDsbi6H7wEOnVsA7B48huotw3up9royBYYJN2pYs 2Bt0rSjW1AxR/noup03x77w0RcfNlbzKkjeOhKWHYJan/rhAxevGXQQgxgTpNngO3DC9 03nKOyjN5bz5wcOINo7LwvfEEQAxLIama3lXJL00yZs0UDC9yPmHurnWTpVr55hTPlTh 6uQmQA7fsubugwp6Wm+i+ctpgX2muT3zzI5VlkPGohQGiAK848qx2uLMlAN3yMFGfjj2 lqiySL/qZfcRrUNj2fOaV1+dBdGZpQ3baroHoK5FuBCFjrU4Q5T6PmKUkFAeI7AnGYBb MCXg== X-Forwarded-Encrypted: i=1; AJvYcCWkAiaJj1knI17G64cB/JJiQ5SRiiiePvOIXGN9LFMKJSVb4TlbaejwsoVPdVhH1wuLVVuoH7dKQA==@kvack.org X-Gm-Message-State: AOJu0YyxgRMw9aFCTPm71dzKJ0eWbeQMmkyq+0EBOgHIiNfx2lYxIBKy wY+nh0NYKZASoR/jFXfsKWMH9HUeW2wyngtVxeXzIiyoy8c5XMRXTGnf1YvVo0HaNzEh5PwyJ2B zZXC1yTHNabWqQQ4BxPq9eUZQSsXTeQ== X-Google-Smtp-Source: AGHT+IFiCX82jVoo9UNG1nkAn8eMWn7soS/rn0DAy4Oe60tWsJi3sd+f5FD995Ap7EhQqDBAkC60hHgbhQLOulTcWX4= X-Received: by 2002:a05:600c:1911:b0:426:5b17:8458 with SMTP id 5b1f17b1804b1-43115abf3b6mr30732445e9.12.1728570844539; Thu, 10 Oct 2024 07:34:04 -0700 (PDT) MIME-Version: 1.0 References: <20241008112049.2279307-1-linyunsheng@huawei.com> <20241008112049.2279307-7-linyunsheng@huawei.com> <8bc47d27-b8ea-4573-937a-0056bdd8ea2c@huawei.com> In-Reply-To: <8bc47d27-b8ea-4573-937a-0056bdd8ea2c@huawei.com> From: Alexander Duyck Date: Thu, 10 Oct 2024 07:33:27 -0700 Message-ID: Subject: Re: [PATCH net-next v20 06/14] mm: page_frag: reuse existing space for 'size' and 'pfmemalloc' To: Yunsheng Lin Cc: davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Andrew Morton , linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 3B13D40007 X-Stat-Signature: qf3ay5rms3i3d8zu6h87pibo1yskcc3a X-HE-Tag: 1728570844-500191 X-HE-Meta: U2FsdGVkX191N1J9tj/BFJd3mAzut8Le84UB6u7H0OC84mA1K54kvm+WMDOFQnOmTpdyDQJM/9QMsFKjZlXHqrZHtpjO64uxfideBwbEr5K5i3/Hu+sMdXSU+MqM/Ef+8MKGrCds2E3atIvHEaNjBRYDcEjq+xr4Ljm2ppwsjyMfFxpI0cJvOunE4IOUIvCU871yxZv1qEroLfOaCw1aUBSF/c8Vi6yRT4bHm5wkqe900OnNzyoow9P+eYNp0XjjZbSpHi6UNzfq+uSt/Z8fJlhXzNEEIex/bBTt8812kdQbg+Spl8vbc5cctwtC2JXeIuASxdcUcKHZOpEA4UaY5UlKtevcak5eHi6MF/rOGLxBxxfHHfJN4OIFm0tjt16d6spo7MULDowQ7Yz14tQCqpTBtXKmXwqcKg+NlXl7kqvA9eTQJrXJ1eZFxDP431IXbsons4z82PYkfanyVK8IR6ISv8PCT8BqTwFwFp9A7WWc459L3kU6TkbtnXY/l3je/5Lxiyjdy+H5bsPyveOU+C28if34L7p6ada3/0JZ4fXYaarJo6Z6D6HesVxJNFxjEoAyVYLF2NV7L1GGLCy5gmb9U9igphhRPv8J34nxeQJXthk4DmaMZuKqx3f53hEmGrWNcn6jng1iBFP0O9hhgWMRb/U/CTB1/yc6fhWltIv8LVvBNBZe3KOpMbkT+nxSftEu3kSp2L45uXOMwXz5DputtxpACQSedruYFrPQLi1MY4u0Kmc+Jpop+d2xMM+lGgVZZ4qEFjkouGTAJ/y8hO5LZvtGfPsLSbVbOcURnzZsbGq9lKPvlt2hf6ihPBVpn2gcVFPcVLuy2XbCXCJD+Qz26VtB0MtXXRud+PLSwSwOThX2nG0LlKU+DhoS4BbRcJROUrL3JojsyPS2Uu67D5jzf81ygHK749K30NU2mHFa7lyst8g2WLPV+ZTJDT4SYLPYFrX2cXFrJPjVH4U 4bs/pgVq jWQhgeKZys/xDheENkG0S93E5OBoVOcMQjtqHBhX99HRy3FbW1kGC99Dp9nmqpzAs8qywbTKLeqBIPhr0vWTwYkDfl9jHRvu9du+58H4EeQ5LG/uvgton3VjVli1iba8lTJWWQnE5Swvytc0xU5daW/EnWYxhjIqAZHtlDkn6gwR2FRdXflSLfpGkZfw1ePqXC79UeBymvzzmt9kzpQ5hruJ3nTjAggystkjfbc8HoHYPjRql28b0ZGZkfr8Xm4mo3J7iw7JkJR5+vPRNSSuphQoELZwUP0hLeIepmx9vuTRB3kV1jpjR3AB3m1gAYv57k2TSnhv68IvRfVjj/cKwQe9qh3RzXTrQ6ZiMlvYEQrKKNwt+mjYx+DtdBzKkallcKJQr1z/H2aOH779/Bsov6SJo4ZpwqgSRdHKzdEwLUYTKX/X+aBID966bjHmNRhZDunyH9BSkgn63jRI= 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 Thu, Oct 10, 2024 at 4:32=E2=80=AFAM Yunsheng Lin wrote: > > On 2024/10/10 7:50, Alexander Duyck wrote: > > ... > > >> + > >> +#define PAGE_FRAG_CACHE_PFMEMALLOC_BIT (PAGE_FRAG_CACHE_ORDER= _MASK + 1) > >> + > >> +static inline bool page_frag_encoded_page_pfmemalloc(unsigned long en= coded_page) > >> +{ > >> + return !!(encoded_page & PAGE_FRAG_CACHE_PFMEMALLOC_BIT); > >> +} > >> + > > > > Rather than calling this encoded_page_pfmemalloc you might just go > > with decode_pfmemalloc. Also rather than passing the unsigned long we > > might just want to pass the page_frag_cache pointer. > As the page_frag_encoded_page_pfmemalloc() is also called in > __page_frag_alloc_align(), and __page_frag_alloc_align() uses a > local variable for 'nc->encoded_page' to avoid fetching from > page_frag_cache pointer multi-times, so passing an 'unsigned long' > is perferred here? > > I am not sure if decode_pfmemalloc() is simple enough that it > might be conflicted with naming from other subsystem in the > future. I thought about adding a '__' prefix to it, but the naming > seems long enough that some inline helper' naming is over 80 characters. What you might do is look at having a page_frag version of the function and a encoded_page version as I called out below with the naming. It would make sense to call the two out separately as this is operating on an encoded page, not a page frag. With that we can avoid any sort of naming confusion. > > > >> static inline void page_frag_cache_init(struct page_frag_cache *nc) > >> { > >> - nc->va =3D NULL; > >> + nc->encoded_page =3D 0; > >> } > >> > >> static inline bool page_frag_cache_is_pfmemalloc(struct page_frag_cac= he *nc) > >> { > >> - return !!nc->pfmemalloc; > >> + return page_frag_encoded_page_pfmemalloc(nc->encoded_page); > >> } > >> > >> void page_frag_cache_drain(struct page_frag_cache *nc); > >> diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c > >> index 4c8e04379cb3..4bff4de58808 100644 > >> --- a/mm/page_frag_cache.c > >> +++ b/mm/page_frag_cache.c > >> @@ -12,6 +12,7 @@ > >> * be used in the "frags" portion of skb_shared_info. > >> */ > >> > >> +#include > >> #include > >> #include > >> #include > >> @@ -19,9 +20,41 @@ > >> #include > >> #include "internal.h" > >> > >> +static unsigned long page_frag_encode_page(struct page *page, unsigne= d int order, > >> + bool pfmemalloc) > >> +{ > >> + BUILD_BUG_ON(PAGE_FRAG_CACHE_MAX_ORDER > PAGE_FRAG_CACHE_ORDER= _MASK); > >> + BUILD_BUG_ON(PAGE_FRAG_CACHE_PFMEMALLOC_BIT >=3D PAGE_SIZE); > >> + > >> + return (unsigned long)page_address(page) | > >> + (order & PAGE_FRAG_CACHE_ORDER_MASK) | > >> + ((unsigned long)pfmemalloc * PAGE_FRAG_CACHE_PFMEMALLO= C_BIT); > >> +} > >> + > >> +static unsigned long page_frag_encoded_page_order(unsigned long encod= ed_page) > >> +{ > >> + return encoded_page & PAGE_FRAG_CACHE_ORDER_MASK; > >> +} > >> + > >> +static void *page_frag_encoded_page_address(unsigned long encoded_pag= e) > >> +{ > >> + return (void *)(encoded_page & PAGE_MASK); > >> +} > >> + > >> +static struct page *page_frag_encoded_page_ptr(unsigned long encoded_= page) > >> +{ > >> + return virt_to_page((void *)encoded_page); > >> +} > >> + > > > > Same with these. Instead of calling it encoded_page_XXX we could > > probably just go with decode_page, decode_order, and decode_address. > > Also instead of passing an unsigned long it would make more sense to > > be passing the page_frag_cache pointer, especially once you start > > pulling these out of this block. > > For the not passing the page_frag_cache pointer part, it is the same > as above, it is mainly to avoid fetching from pointer multi-times. > > > > > If you are wanting to just work with the raw unsigned long value in > > the file it might make more sense to drop the "page_frag_" prefix from > > it and just have functions for handling your "encoded_page_" value. In > > that case you might rename page_frag_encode_page to > > "encoded_page_encode" or something like that. > > It am supposing you meant 'encoded_page_decode' here instead of > "encoded_page_encode"? > Something like below? > encoded_page_decode_pfmemalloc() > encoded_page_decode_order() > encoded_page_decode_page() > encoded_page_decode_virt() For the decodes yes. I was referring to page_frag_encode_page. Basically the output from that isn't anything page frag, it is your encoded page type so you could probably just call it encoded_page_encode, or encoded_page_create or something like that.