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 65844C4345F for ; Mon, 15 Apr 2024 23:55:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AB5036B0082; Mon, 15 Apr 2024 19:55:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A647D6B0085; Mon, 15 Apr 2024 19:55:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 92C5A6B0087; Mon, 15 Apr 2024 19:55:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 7654E6B0082 for ; Mon, 15 Apr 2024 19:55:50 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 39DB8A0813 for ; Mon, 15 Apr 2024 23:55:50 +0000 (UTC) X-FDA: 82013426460.09.E08B8F5 Received: from mail-ot1-f50.google.com (mail-ot1-f50.google.com [209.85.210.50]) by imf23.hostedemail.com (Postfix) with ESMTP id 5BDD1140008 for ; Mon, 15 Apr 2024 23:55:48 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="RDll0/Yi"; spf=pass (imf23.hostedemail.com: domain of alexander.duyck@gmail.com designates 209.85.210.50 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=1713225348; 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=sq974b3zN56bT5SWz19uBa19EDT0KaB/lKfvgwpSeWA=; b=NGJE7KI7GICP8NKTsOXA9gJRrdAAERareFrEzkzUkdoEZ56ybJU4aG6if3XLlgZLFDvhVA PwF8aPseYCoUh7KlB3PniaIuKfiozJexl6ZnMuWJd8lw8gPDXAKea1yCEflitgZ2NWKXbG ivltFFyEoflrFRNDjsqv6CEJBF5YKMQ= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713225348; a=rsa-sha256; cv=none; b=h0qNKroGp16BFqaPORLHvbydqvNCG09Gt6eUSwNG4+sJpnRVlEoJYvMovpONp5aDsqP6DN vvyAJX8Hmz3TYGl93/y0WFUpXERY22kq01yAJBpTaU79gzqueiBshKOigViD5A+IpMd4vz gDwllpiuCl58USTnL+DGMCfWTkvsrww= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="RDll0/Yi"; spf=pass (imf23.hostedemail.com: domain of alexander.duyck@gmail.com designates 209.85.210.50 as permitted sender) smtp.mailfrom=alexander.duyck@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-ot1-f50.google.com with SMTP id 46e09a7af769-6e0f43074edso2706705a34.1 for ; Mon, 15 Apr 2024 16:55:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713225347; x=1713830147; darn=kvack.org; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject :date:message-id:reply-to; bh=sq974b3zN56bT5SWz19uBa19EDT0KaB/lKfvgwpSeWA=; b=RDll0/YiUZpRnqLA6KGeBg6zDXxTacFzINLmsj/oXIQ3zV2dUb61GIa4u7H1mAJMTz 7ZzYtjZYJ5+oCYZb/CCbsGXJZmtmAX425WM/lf65fGOjIkF5EgIVmaz0n/nmpKBb6tPV 2x+Px1CUteOnHkeFXwNwMNodpseTV20dWyjSPObETJs00Zl52FcSgkNZ7ROqT6a9bNMD hvN0BEWBbzP341ZfJg72bBcTGcchMvSbKvbk8pyrxy2adOCKvgmwzjpqIJ0h/Cg/rrjB sqhapl/7rBiex0d2+b1gmgw80UUhwhjrEEwyFGkG3GTSkUAGOSrWeYiT/JAJTzq4926X apGQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713225347; x=1713830147; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=sq974b3zN56bT5SWz19uBa19EDT0KaB/lKfvgwpSeWA=; b=KckB1al129VxJL4KLSleTthvfv1NxIouk6s6vohUnAk03QYnEzE2uZEOZVIX0M4LDt 2F4lfdfikbhIzFDT7CBzk/LuQCymHufNhiTmR/vZ+aUxHa9xcnzoNfNFEoAbp+PkV8sS C1w+YrOLM1BJ/c1BosbycDwZIRRxz49vLaQBEy39VrbaMdS4slX2ZJoC8cun3eZ4mYih CosIqd55+Ndupu9BJG1tl58pNjkXU1FpgBctS9M/IjvHb97WMDQlzgWA1IYeP+xuUpia 8gPHjd8c8QzJyQLzFbSP6SDwqBuSDZs+fLxfrUqSgXGVmUq3lLI4xMunSfAvx5kL5ryK +tdQ== X-Forwarded-Encrypted: i=1; AJvYcCXWsZ3Gkh99AglyJAFuB+7hBb6AVSMdENnkHrFI6jH0yP9Ep9SaB/q/Ht7177htMlZy0un5KhU+RQ4zS7C/agIDeug= X-Gm-Message-State: AOJu0Yw4IeHJUDVfZ17oDLWOlOJWZF8d3MDIBCujwLY7EyhpwOGA48Ir FyDQH3tldVu7hhyPg6DP0hRWMVZ7CiET1IdPiXHf0paHb70hR2iw X-Google-Smtp-Source: AGHT+IHiucEWzxrRlC1n9ieu3IXs01n8gYsLVmIpRDvJmbOzZKxfnYKgNFMKy4lMB4emzn4mC2HidA== X-Received: by 2002:a9d:7388:0:b0:6eb:8136:bf3e with SMTP id j8-20020a9d7388000000b006eb8136bf3emr3516020otk.21.1713225347283; Mon, 15 Apr 2024 16:55:47 -0700 (PDT) Received: from [192.168.0.128] ([98.97.103.43]) by smtp.googlemail.com with ESMTPSA id o65-20020a634144000000b005dc36761ad1sm7769686pga.33.2024.04.15.16.55.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 15 Apr 2024 16:55:46 -0700 (PDT) Message-ID: Subject: Re: [PATCH net-next v2 05/15] mm: page_frag: use initial zero offset for page_frag_alloc_align() From: Alexander H Duyck To: Yunsheng Lin , davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Andrew Morton , linux-mm@kvack.org Date: Mon, 15 Apr 2024 16:55:45 -0700 In-Reply-To: <20240415131941.51153-6-linyunsheng@huawei.com> References: <20240415131941.51153-1-linyunsheng@huawei.com> <20240415131941.51153-6-linyunsheng@huawei.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) MIME-Version: 1.0 X-Stat-Signature: r93yo5uuotwawdp53schuffs4cdguxt8 X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 5BDD1140008 X-Rspam-User: X-HE-Tag: 1713225348-430411 X-HE-Meta: U2FsdGVkX1+1K610tf3qgubutCJnme/d7KuF/wYrxo1Xh20x8PIP35QckZl7GGWCF8JXd52tcJTV3Kqxh4yNJa6GBI1qbw5A6f1QdxlCbAzc8UJz8hPAZVMijBVapKdo+IIGYCyId3ZxMuwSV0yOm1J4KkTIXuhOzt2vCmcyn8t4Jhm5QcchavmaNmZPfJEhmfFOL592s78UiO+gnxBMA3agWMNazVXPQrP/xCISaAOICNBEgI0QHZpjTG7E2fYnt2YfUB4MlYuVNMBTTws99EzWVgLg2CDb+PHF+UsN9c2405oRqZbw0vBTruBkX8Od9TuiOKQE8XhyuoKdq68GQqpSCF8cXpqeo/oX3useBwXMJWpisdCvk5s+jqDE8KExTidIXTJYxDVl1djHhBWok/t5xnhxTMJ61oHlRyALHEVjMp5Fvj0JZuHmzL8F+U0f446mh08Z43gx7tMeMA2vxUj9ICzAVkK9tEs1nRGy0Ni25RxuuPhJz0bEb7Mp+xb46D6t8z0MAZEuZv7X9o6GKXymF/A+c5lH6X5iKgE3iWgK2xBYoFmU1e00TSHxaNOqLU3p+l/Oo6bhXCxaE59btpF8UIqeX6bhJGrksz0H/Okh+fBv2xR7wtINbjXVP5hq4jm7dpFJ14WSRGdovm58jDRYAhfzvFdl8swGFLcfdxISxwjwWqRViUc1v9gEl5hTN9+dRqsT7pH46634tdPOKtFlyAHMf27b1QNzMVZ11sN9k3cjd6PmrKp5FUs8Sq24tEZ+yRJjK7G8KG7WKSsRex1Tigmy5et1l7k9Tyj2xOleG6l49yfTiggEgWGzVpRafCmwBKeEBrxOPehrkQGmOtnCW0kaSX+NtDxFCAYaiRB6SuaHKJkqRP8IksVK0AsR8kydARqhqUEjvpt0sfJ7bXtwEjFLYFMKNz8UPRxByMfQM2EAsO3Rd/m1ohzfZS5ngnhI+whrL60v0OIfzGH BmdZ0mcZ p/iQncNm5TqCZv15Vu5cva10zf0XeHuoXop3jXybsQOuttYz9JhItrbTV6AP6+etL+KA/1+ZiRmkByQEdhOvp6V5g+cOnpOz8MkTCCl7turOiwOCMc93J95RUGdkN/vkHynWJwKANjuFrxOTF6IN953bvbRGm5qe1Iwz5NxjOPXZ8l4Rhiy4VEu+vdL7p1/ReeSL2jZ1xiH2IpqJLnURo6Te3lhOFNb2vYIWmULeirkYjNGJe4Achgs3FkdZ5rfwSYMGPphboiVezNkeXXc+mCiaN+8LSogfo1YgK6IKQRKDHEmrV8ZKzE8IJGI0lsBsl/N6YtHoS1oFfkTjb8bBHSrj6kUombl+7uCRkkJ374A79YD0kTebJHVbYD9kAWNXidcaxcK3uGljEZEKuYl7XE9D13ajLJlF89BtfXpCC24y31XHo+EjRbnJxZfDXUz4RIzgZhlm4ui0nQ9qnZVDL5Esv5Vkmx+GOyfKwq/CQjTji9A7VPU3BSDNlPXESbfHijJyU2OlrBAmTus4FVFg+DlJ2ph719OU0Wy3huhFHFW3IY+AwB6WXp+vTxkiQ7L5ObLG8ng21rR17geHjarMKNH9eVY4tDDdgQhsddbb6/W1AH+s= 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 Mon, 2024-04-15 at 21:19 +0800, Yunsheng Lin wrote: > We are above to use page_frag_alloc_*() API to not just > allocate memory for skb->data, but also use them to do > the memory allocation for skb frag too. Currently the > implementation of page_frag in mm subsystem is running > the offset as a countdown rather than count-up value, > there may have several advantages to that as mentioned > in [1], but it may have some disadvantages, for example, > it may disable skb frag coaleasing and more correct cache > prefetching >=20 > We have a trade-off to make in order to have a unified > implementation and API for page_frag, so use a initial zero > offset in this patch, and the following patch will try to > make some optimization to aovid the disadvantages as much > as possible. >=20 > 1. https://lore.kernel.org/all/f4abe71b3439b39d17a6fb2d410180f367cadf5c.c= amel@gmail.com/ >=20 > CC: Alexander Duyck > Signed-off-by: Yunsheng Lin > --- > mm/page_frag_cache.c | 31 ++++++++++++++----------------- > 1 file changed, 14 insertions(+), 17 deletions(-) >=20 > diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c > index 64993b5d1243..dc864ee09536 100644 > --- a/mm/page_frag_cache.c > +++ b/mm/page_frag_cache.c > @@ -65,9 +65,8 @@ void *__page_frag_alloc_align(struct page_frag_cache *n= c, > unsigned int fragsz, gfp_t gfp_mask, > unsigned int align_mask) > { > - unsigned int size =3D PAGE_SIZE; > + unsigned int size, offset; > struct page *page; > - int offset; > =20 > if (unlikely(!nc->va)) { > refill: > @@ -75,10 +74,6 @@ void *__page_frag_alloc_align(struct page_frag_cache *= nc, > if (!page) > return NULL; > =20 > -#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) > - /* if size can vary use size else just use PAGE_SIZE */ > - size =3D nc->size; > -#endif > /* Even if we own the page, we do not use atomic_set(). > * This would break get_page_unless_zero() users. > */ > @@ -87,11 +82,18 @@ void *__page_frag_alloc_align(struct page_frag_cache = *nc, > /* reset page count bias and offset to start of new frag */ > nc->pfmemalloc =3D page_is_pfmemalloc(page); > nc->pagecnt_bias =3D PAGE_FRAG_CACHE_MAX_SIZE + 1; > - nc->offset =3D size; > + nc->offset =3D 0; > } > =20 > - offset =3D nc->offset - fragsz; > - if (unlikely(offset < 0)) { > +#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) > + /* if size can vary use size else just use PAGE_SIZE */ > + size =3D nc->size; > +#else > + size =3D PAGE_SIZE; > +#endif > + > + offset =3D ALIGN(nc->offset, -align_mask); I am not sure if using -align_mask here with the ALIGN macro is really to your benefit. I would be curious what the compiler is generating. Again, I think you would be much better off with: offset =3D __ALIGN_KERNEL_MASK(nc->offset, ~align_mask); That will save you a number of conversions as the use of the ALIGN macro gives you: offset =3D (nc->offset + (-align_mask - 1)) & ~(-align_mask - 1); whereas what I am suggesting gives you: offset =3D (nc->offset + ~align_mask) & ~(~align_mask)); My main concern is that I am not sure the compiler will optimize around the combination of bit operations and arithmetic operations. It seems much cleaner to me to stick to the bitwise operations for the alignment than to force this into the vhost approach which requires a power of 2 aligned mask. Also the old code was aligning on the combination of offset AND fragsz. This new logic is aligning on offset only. Do we run the risk of overwriting blocks of neighbouring fragments if two users of napi_alloc_frag_align end up passing arguments that have different alignment values? > + if (unlikely(offset + fragsz > size)) { > page =3D virt_to_page(nc->va); > =20 > if (!page_ref_sub_and_test(page, nc->pagecnt_bias)) > @@ -102,17 +104,13 @@ void *__page_frag_alloc_align(struct page_frag_cach= e *nc, > goto refill; > } > =20 > -#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) > - /* if size can vary use size else just use PAGE_SIZE */ > - size =3D nc->size; > -#endif > /* OK, page count is 0, we can safely set it */ > set_page_count(page, PAGE_FRAG_CACHE_MAX_SIZE + 1); > =20 > /* reset page count bias and offset to start of new frag */ > nc->pagecnt_bias =3D PAGE_FRAG_CACHE_MAX_SIZE + 1; > - offset =3D size - fragsz; > - if (unlikely(offset < 0)) { > + offset =3D 0; > + if (unlikely(fragsz > size)) { This check can probably be moved now. It was placed here to optimize things as a check of offset < 0 was a single jump command based on the signed flag being set as a result of the offset calculation. It might make sense to pull this out of here and instead place it at the start of this block after the initial check with offset + fragsz > size since that would shorten the need to carry the size variable. > /* > * The caller is trying to allocate a fragment > * with fragsz > PAGE_SIZE but the cache isn't big > @@ -127,8 +125,7 @@ void *__page_frag_alloc_align(struct page_frag_cache = *nc, > } > =20 > nc->pagecnt_bias--; > - offset &=3D align_mask; > - nc->offset =3D offset; > + nc->offset =3D offset + fragsz; > =20 > return nc->va + offset; > }