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 519DED3DEA4 for ; Fri, 18 Oct 2024 18:03:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CDA246B00A6; Fri, 18 Oct 2024 14:03:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C8A916B00AD; Fri, 18 Oct 2024 14:03:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B03E86B00AE; Fri, 18 Oct 2024 14:03:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 91CCC6B00A6 for ; Fri, 18 Oct 2024 14:03:44 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 08068C080A for ; Fri, 18 Oct 2024 18:03:31 +0000 (UTC) X-FDA: 82687495212.17.FF01FC5 Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) by imf01.hostedemail.com (Postfix) with ESMTP id E75D140019 for ; Fri, 18 Oct 2024 18:03:32 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=k0E638+y; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf01.hostedemail.com: domain of alexander.duyck@gmail.com designates 209.85.221.51 as permitted sender) smtp.mailfrom=alexander.duyck@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729274514; a=rsa-sha256; cv=none; b=eP12KCOsUBsPrtSfG71kBQHRErmte0cBK90ih3zF7cLqUeokwOPTOZ2Rw5icsLvfDHyG8b MHn51OqzoALNo+zyHqxucdm9mIUdddpPnVUvAT91gNKXWNUNysxS98uQrYfDwH3ItoDSbh 9gbDMtKiWWMs9afnrwsPyuefMu7uCVo= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=k0E638+y; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf01.hostedemail.com: domain of alexander.duyck@gmail.com designates 209.85.221.51 as permitted sender) smtp.mailfrom=alexander.duyck@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729274514; 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=vTl/hUSj/UfTQkYKvku3PzPI4RSkWnNGUZziFInNPIM=; b=e48q2sWUYizvb664pz0XEKoCJKXhcvFvdNykXwNlmkkapm8tkNo3re6qMxbdqbx0b2Hx8+ nFW9AS2+vxN6s2GcHsE0UAnXR3A9jTtv0Vu3+uVpODDQOIo4QuRcsWrggD3Zn2m0/gyb8z Xbc1d3EhabW/cXpc1C0jiDe67SGPDOY= Received: by mail-wr1-f51.google.com with SMTP id ffacd0b85a97d-37ec4e349f4so741797f8f.0 for ; Fri, 18 Oct 2024 11:03:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729274621; x=1729879421; 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=vTl/hUSj/UfTQkYKvku3PzPI4RSkWnNGUZziFInNPIM=; b=k0E638+y+8l5r+m6/ElCnUGi6MIiIXDZn+JwNfST2aU3S9Q1sPWVYka0nLpRpxmvmp fco/4hQcPwgcIr66CnTOzRjajjo0+0ns4i4k2yHylxzjbyVDDoXYT9SczAOn4iS1sKo7 PM/q+io4XHNNxRd61J/M0rLyB/EV5GsX+4rDD6Qc1ajHBETKlXO2HYxYmUqHB5lgyQPU +XFbiLQEghrdzIVzom3LKW5f2FxXkJpT1mZeLW1g4CnWzZE1fWPIMnLLCLv0TIZttAgB o0AW74A+0p+c/zyq1MmuAuSERJhgprDKA+96lqvpP28QpdBc4SV2XEFw9MKlIQ3jcKDt Q2tA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729274621; x=1729879421; 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=vTl/hUSj/UfTQkYKvku3PzPI4RSkWnNGUZziFInNPIM=; b=re1Gz2kbZsRdHloFAiwNT4/2pfP71ihqjClP2AJwnhayf2WB0i2eolPOb50+yXFDz6 RekqHi4DP7DrK6SdUGLVbBxxfmVssRRBqc9v1rxS/HFJrsof0ICcEVCwptWTHfKaEMkz MZIazvkIvDUqdUG5korGzYwoEAhDrp7aPOjAHtnu6yACzA1BuukwpFMtFe+dEiqXEGvG 5KpZBVu2Vp+ReZRNzQAo3Gas1NRd79fFVrnIv908pgkcdg/e4G/2e9NqVntjhr702FH4 QJ5ftfAXG74uZrGQBZYbu4G97lSRiu8EmodZ9PM0YdwSOMPSbA3fvIYbBXW+0eeWwnSB U54w== X-Forwarded-Encrypted: i=1; AJvYcCWbSwHnmYq83/+umdugPLYv96ZRbnArK6jLBBYkCK5zdineIqBlgwvttHdm2NfOVadDjRPMgI+npw==@kvack.org X-Gm-Message-State: AOJu0YwyeNj3g48KmfJLwIEgWtIEOQWj5/Oh+1T1C02yjoaa4kgMs7ad Na4hJvXJFTvw08N+/+1nX8sNeMU8VtKSVvcjlDBkJvzw0Squ6qaoebsliPsV6sx52Maa3dRRbff gEmjxBCE5BkRP6XeX9+gIdxiJt8Q= X-Google-Smtp-Source: AGHT+IE53tP/trlIJhSThzo1jS5+hYd/nfh5lSF33/o7kCnAusq4BXbw0lmSqvYd8tM3bVJv0VSKJ2Rfe3i2z4xiiiI= X-Received: by 2002:a5d:6892:0:b0:37d:890c:f485 with SMTP id ffacd0b85a97d-37ea21949e5mr2865340f8f.25.1729274620408; Fri, 18 Oct 2024 11:03:40 -0700 (PDT) MIME-Version: 1.0 References: <20241018105351.1960345-1-linyunsheng@huawei.com> <20241018105351.1960345-11-linyunsheng@huawei.com> In-Reply-To: <20241018105351.1960345-11-linyunsheng@huawei.com> From: Alexander Duyck Date: Fri, 18 Oct 2024 11:03:03 -0700 Message-ID: Subject: Re: [PATCH net-next v22 10/14] mm: page_frag: introduce prepare/probe/commit API 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-Rspamd-Queue-Id: E75D140019 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: qbde4puobg54efbrqahzit4ttgxqzpcf X-HE-Tag: 1729274612-248289 X-HE-Meta: U2FsdGVkX18UVfz30i/dVo+adJlxb4rsJ0iD+mCiAElganWYpDFp4zsnrZHExJnqkqeyuBoNzSvUfNIzqAZeNelxwW2Iu6URRzP1snMmm/ZT5lG2Vh4fzw36FyuZqtdUr6yniwvoqI4sVFXAOPzsb0o8RgVb5kE9Kx2qQ1WFF4UP90JqwnLf81GODGEBU+LeutFqXt7UiGzi9J+8dfa/ZjLsxdcYNrmqm7j0EagAYj9gXX9QjZbNIdBYD5IYQBAajvqOJyq58GBHmK/Uo2u10ouw2772sR8l/yelCcwXItN6SP09RBDdvGqF7kUZxVwFPQ/HGqnkDu0paT/1t2Q0LvOZzeTAV4qLPff8+WR9SfFlG9qxenQ/53+84pRWfXkFy8fX1BTr/kle+1U761SwjKQ+TZJBglUMGvxOU3/qnamDOw8/ymo7eNMoFptI4uFGK9iM7vMSeeviP2I3NaMhLCxGjvkn3ST4Zyzn6fxhDNowpx8bvPPv9ZVd0K+voukPOpuGzKIDNImTj4zYYws76tp90S/+oJZeMGI1wVeFmtSzIKv5VasQc+UANRPWvXAQ3cECel+b3VF1L0YSRTyZeKJH4zpv3a1+axro59xpUdsG0rA8Xs/sTs93+71r835SNAmqnjWXgLVAgYZMuNOB4zM0SaGKOo2qW1Sxxvxoe9OjnYmyVvUdp8ApzvQtjoqnutrJJliEoSG+sQwdjRrPPTxB5yHJPQHPvmFyfgoICImk2ll1lJBCs47/KNpXG2BFa4pcQixZVxxZZJADXkEHLOYdCaqo/Dao2jZcOtZpI9A84rROfK2bd/POPSQnqme81dCTKdWlDQwj3AFiA+PqDi/+9eGPMGzG5R8bFc4hd5Xbx8q3rx1c3MpFZbkD9azIw3Z5YznIGTRZ54fSweg0tY+QMDG8xCZKh/Qbq28/j3ksrDYEuDkTMrEFaAP4Xd3Eq+IddSqELw2yTlBoaCr JlO1PlSx QGW+VAaMGm5Wg93SGXSFXzJPTWKLZzKpgMS6kSG0QRzNLgXAUzrMHHnAdCYwUBNpxF6D4b+8nR2C0tRXpKLMCEap7aCWANOAP8isNigMfEYU9MjOKCqTHe5OnnvBxf1pjEt8vcEJGUZbW0OD+liYm/DnM95hWyhKK8n/qyu0RKQ84R2x7PLC/WTsnAuM9X0Fxim6qN160Q8bt1Hwdf8C6Aw+X5h89R5lqd/zoY18iUcFN6temiBzZVKoELXL069pUPUXvh1w3UntTbx/ZW8SG3oVZnf2FfLSigRG4cGLPI/FeZb0OQ2V/pcmKQJLDH3p/pqq0wKZ+mpasxcfQBjWc1QBzFYCma21LRoYH1bOBq+f8pG+/rdR8btYfiz16tHyz9cj8GpcCkBmGfii+YvsXmbn/CA== 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 Fri, Oct 18, 2024 at 4:00=E2=80=AFAM Yunsheng Lin wrote: > > There are many use cases that need minimum memory in order > for forward progress, but more performant if more memory is > available or need to probe the cache info to use any memory > available for frag caoleasing reason. > > Currently skb_page_frag_refill() API is used to solve the > above use cases, but caller needs to know about the internal > detail and access the data field of 'struct page_frag' to > meet the requirement of the above use cases and its > implementation is similar to the one in mm subsystem. > > To unify those two page_frag implementations, introduce a > prepare API to ensure minimum memory is satisfied and return > how much the actual memory is available to the caller and a > probe API to report the current available memory to caller > without doing cache refilling. The caller needs to either call > the commit API to report how much memory it actually uses, or > not do so if deciding to not use any memory. > > CC: Alexander Duyck > Signed-off-by: Yunsheng Lin > --- > include/linux/page_frag_cache.h | 130 ++++++++++++++++++++++++++++++++ > mm/page_frag_cache.c | 21 ++++++ > 2 files changed, 151 insertions(+) > > diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_ca= che.h > index feed99d0cddb..1c0c11250b66 100644 > --- a/include/linux/page_frag_cache.h > +++ b/include/linux/page_frag_cache.h > @@ -46,6 +46,10 @@ void *__page_frag_cache_prepare(struct page_frag_cache= *nc, unsigned int fragsz, > unsigned int __page_frag_cache_commit_noref(struct page_frag_cache *nc, > struct page_frag *pfrag, > unsigned int used_sz); > +void *__page_frag_alloc_refill_probe_align(struct page_frag_cache *nc, > + unsigned int fragsz, > + struct page_frag *pfrag, > + unsigned int align_mask); > > static inline unsigned int __page_frag_cache_commit(struct page_frag_cac= he *nc, > struct page_frag *pfr= ag, > @@ -88,6 +92,132 @@ static inline void *page_frag_alloc(struct page_frag_= cache *nc, > return __page_frag_alloc_align(nc, fragsz, gfp_mask, ~0u); > } > > +static inline bool __page_frag_refill_align(struct page_frag_cache *nc, > + unsigned int fragsz, > + struct page_frag *pfrag, > + gfp_t gfp_mask, > + unsigned int align_mask) > +{ > + if (unlikely(!__page_frag_cache_prepare(nc, fragsz, pfrag, gfp_ma= sk, > + align_mask))) > + return false; > + > + __page_frag_cache_commit(nc, pfrag, fragsz); > + return true; > +} > + > +static inline bool page_frag_refill_align(struct page_frag_cache *nc, > + unsigned int fragsz, > + struct page_frag *pfrag, > + gfp_t gfp_mask, unsigned int al= ign) > +{ > + WARN_ON_ONCE(!is_power_of_2(align)); > + return __page_frag_refill_align(nc, fragsz, pfrag, gfp_mask, -ali= gn); > +} > + > +static inline bool page_frag_refill(struct page_frag_cache *nc, > + unsigned int fragsz, > + struct page_frag *pfrag, gfp_t gfp_ma= sk) > +{ > + return __page_frag_refill_align(nc, fragsz, pfrag, gfp_mask, ~0u)= ; > +} > + > +static inline bool __page_frag_refill_prepare_align(struct page_frag_cac= he *nc, > + unsigned int fragsz, > + struct page_frag *pfr= ag, > + gfp_t gfp_mask, > + unsigned int align_ma= sk) > +{ > + return !!__page_frag_cache_prepare(nc, fragsz, pfrag, gfp_mask, > + align_mask); > +} > + > +static inline bool page_frag_refill_prepare_align(struct page_frag_cache= *nc, > + unsigned int fragsz, > + struct page_frag *pfrag= , > + gfp_t gfp_mask, > + unsigned int align) > +{ > + WARN_ON_ONCE(!is_power_of_2(align)); > + return __page_frag_refill_prepare_align(nc, fragsz, pfrag, gfp_ma= sk, > + -align); > +} > + > +static inline bool page_frag_refill_prepare(struct page_frag_cache *nc, > + unsigned int fragsz, > + struct page_frag *pfrag, > + gfp_t gfp_mask) > +{ > + return __page_frag_refill_prepare_align(nc, fragsz, pfrag, gfp_ma= sk, > + ~0u); > +} > + > +static inline void *__page_frag_alloc_refill_prepare_align(struct page_f= rag_cache *nc, > + unsigned int f= ragsz, > + struct page_fr= ag *pfrag, > + gfp_t gfp_mask= , > + unsigned int a= lign_mask) > +{ > + return __page_frag_cache_prepare(nc, fragsz, pfrag, gfp_mask, ali= gn_mask); > +} > + > +static inline void *page_frag_alloc_refill_prepare_align(struct page_fra= g_cache *nc, > + unsigned int fra= gsz, > + struct page_frag= *pfrag, > + gfp_t gfp_mask, > + unsigned int ali= gn) > +{ > + WARN_ON_ONCE(!is_power_of_2(align)); > + return __page_frag_alloc_refill_prepare_align(nc, fragsz, pfrag, > + gfp_mask, -align); > +} > + > +static inline void *page_frag_alloc_refill_prepare(struct page_frag_cach= e *nc, > + unsigned int fragsz, > + struct page_frag *pfra= g, > + gfp_t gfp_mask) > +{ > + return __page_frag_alloc_refill_prepare_align(nc, fragsz, pfrag, > + gfp_mask, ~0u); > +} > + > +static inline void *page_frag_alloc_refill_probe(struct page_frag_cache = *nc, > + unsigned int fragsz, > + struct page_frag *pfrag) > +{ > + return __page_frag_alloc_refill_probe_align(nc, fragsz, pfrag, ~0= u); > +} > + > +static inline bool page_frag_refill_probe(struct page_frag_cache *nc, > + unsigned int fragsz, > + struct page_frag *pfrag) > +{ > + return !!page_frag_alloc_refill_probe(nc, fragsz, pfrag); > +} > + > +static inline void page_frag_commit(struct page_frag_cache *nc, > + struct page_frag *pfrag, > + unsigned int used_sz) > +{ > + __page_frag_cache_commit(nc, pfrag, used_sz); > +} > + > +static inline void page_frag_commit_noref(struct page_frag_cache *nc, > + struct page_frag *pfrag, > + unsigned int used_sz) > +{ > + __page_frag_cache_commit_noref(nc, pfrag, used_sz); > +} > + Not a huge fan of introducing a ton of new API calls and then having to have them all applied at once in the follow-on patches. Ideally the functions and the header documentation for them would be introduced in the same patch as well as examples on how it would be used. I really think we should break these up as some are used in one case, and others in another and it is a pain to have a pile of abstractions that are all using these functions in different ways. > +static inline void page_frag_alloc_abort(struct page_frag_cache *nc, > + unsigned int fragsz) > +{ > + VM_BUG_ON(fragsz > nc->offset); > + > + nc->pagecnt_bias++; > + nc->offset -=3D fragsz; > +} > + We should probably have the same checks here you had on the earlier commit. We should not be allowing blind changes. If we are using the commit or abort interfaces we should be verifying a page frag with them to verify that the request to modify this is legitimate. > void page_frag_free(void *addr); > > #endif > diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c > index f55d34cf7d43..5ea4b663ab8e 100644 > --- a/mm/page_frag_cache.c > +++ b/mm/page_frag_cache.c > @@ -112,6 +112,27 @@ unsigned int __page_frag_cache_commit_noref(struct p= age_frag_cache *nc, > } > EXPORT_SYMBOL(__page_frag_cache_commit_noref); > > +void *__page_frag_alloc_refill_probe_align(struct page_frag_cache *nc, > + unsigned int fragsz, > + struct page_frag *pfrag, > + unsigned int align_mask) > +{ > + unsigned long encoded_page =3D nc->encoded_page; > + unsigned int size, offset; > + > + size =3D PAGE_SIZE << encoded_page_decode_order(encoded_page); > + offset =3D __ALIGN_KERNEL_MASK(nc->offset, ~align_mask); > + if (unlikely(!encoded_page || offset + fragsz > size)) > + return NULL; > + > + pfrag->page =3D encoded_page_decode_page(encoded_page); > + pfrag->size =3D size - offset; > + pfrag->offset =3D offset; > + > + return encoded_page_decode_virt(encoded_page) + offset; > +} > +EXPORT_SYMBOL(__page_frag_alloc_refill_probe_align); > + If I am not mistaken this would be the equivalent of allocating a size 0 fragment right? The only difference is that you are copying out the "remaining" size, but we could get that from the offset if we knew the size couldn't we? Would it maybe make sense to look at limiting this to PAGE_SIZE instead of passing the size of the actual fragment? > void *__page_frag_cache_prepare(struct page_frag_cache *nc, unsigned int= fragsz, > struct page_frag *pfrag, gfp_t gfp_mask, > unsigned int align_mask) > -- > 2.33.0 >