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 9F866EB64D7 for ; Fri, 23 Jun 2023 09:37:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2D1B38D0002; Fri, 23 Jun 2023 05:37:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 282798D0001; Fri, 23 Jun 2023 05:37:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0FE4C8D0002; Fri, 23 Jun 2023 05:37:23 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id F0FBB8D0001 for ; Fri, 23 Jun 2023 05:37:22 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id A7366C0DF8 for ; Fri, 23 Jun 2023 09:37:22 +0000 (UTC) X-FDA: 80933509524.19.22E1500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf18.hostedemail.com (Postfix) with ESMTP id 647BF1C0009 for ; Fri, 23 Jun 2023 09:37:20 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=McYUpW5I; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf18.hostedemail.com: domain of pabeni@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=pabeni@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1687513040; 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=oIT+7voMSF6N77PJ50xcBRfe35k1Tr0FMG0fG9uFkhw=; b=NYTMHwfgBp5x1v+F7aEZIbdJj2zptUUZ7THXjx16zSD+b41gzCo65Fa7ELgFlzlngwcO0V m3E2KOMGYKRbQlJBXvJZuaZXD2PjF62FEznMAKVcKUZThxeodN2dAH1juknLp/0SzHsOTx 4krGxtc0Jw1AEWKyXgrNsrdDD369Bsk= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=McYUpW5I; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf18.hostedemail.com: domain of pabeni@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=pabeni@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687513040; a=rsa-sha256; cv=none; b=DAwBs+n8fbHrqO6MhwzAQpXi/q+tXzZjMd9jmLLjkjARV4Jz82lSeEMA2TVmAOAd7rIFEU Gve7lcrA69eYogGCKEEuzv+y47EwZ4NRNZoRGYNCEV+WMrhDw+pcevmAQ/nUBl9T8RqDCm wKtr2KHaEK0jpBZIVYcDF+HERBMu2a8= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1687513039; h=from:from: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=oIT+7voMSF6N77PJ50xcBRfe35k1Tr0FMG0fG9uFkhw=; b=McYUpW5IvdgIhvIyDrxHPU4npemDxT0fe9tw10qqLZgnzBSW7KrGVhetyI8megPiDIbOmd LlRZQyf4h/RGifW/EyUDZhg+3MecNrzQuhf6XIcI2so+HEsal1GeGNlPWhACvjNOY2QPbA g8MP/203WjvIaxdYHwCZb74ruYwYqu0= Received: from mail-qv1-f72.google.com (mail-qv1-f72.google.com [209.85.219.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-612--z1D7i3COwW4qqutVB7AWA-1; Fri, 23 Jun 2023 05:37:17 -0400 X-MC-Unique: -z1D7i3COwW4qqutVB7AWA-1 Received: by mail-qv1-f72.google.com with SMTP id 6a1803df08f44-632134ad1bbso1093286d6.1 for ; Fri, 23 Jun 2023 02:37:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687513037; x=1690105037; 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=Fxg71GwonJ2lLdj/TmXZbBE8jlv4yQXpkQ99x3iYdD8=; b=k7wZ5lF/YZxjAs1sk9vQg8KQYlD5KgWvgslHbGuK5aEb6dxDufmYCxKC2sKBSuyLvi x3LKqUUTQR3TeUjzlk4ZYIltd9b1d6R99di91BEuN+Vzt3NVBij4iqJjFORZB9YWVNxo 53aNd8Ao+KftzlSURuylYmZpxD66eCYjsSgh1/wNImnNCAGa477vqv4JB6kyw478nSSh VH+6o7SjPN8ubYCJj0qRACgTsewCKixxh5NmiormJmWGTZzcNcLWqCGDztVillu4q6gN RDleVGt9gJCoZiwNTHBwfRmwMjHqzmwYRxdkpB681zdGnf/Fc4IVPrSQdeuZuueTZCfR tA/w== X-Gm-Message-State: AC+VfDzeVmCmQdKSkWY8iC74/DmDzDuzCyKaujbshS5pD8KbrgyI2hrw Up/fPiUwEq8VGYUuYLj/Ga8KB0uWuqILI+JB0q0OxNEb4filpnpTDMJN8fsfS9jl9cQVtpY7PFv ZszYeWaDr7oM= X-Received: by 2002:ad4:5c83:0:b0:62e:ffc3:a9cb with SMTP id o3-20020ad45c83000000b0062effc3a9cbmr26280155qvh.2.1687513036728; Fri, 23 Jun 2023 02:37:16 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4emjXvyu0vwC95mriQSkKhIcdBJ/xzwdwdd4nlnQZQIG6kdrN0Z5ZQRmuqh25MRMSlZcnC0A== X-Received: by 2002:ad4:5c83:0:b0:62e:ffc3:a9cb with SMTP id o3-20020ad45c83000000b0062effc3a9cbmr26280128qvh.2.1687513036400; Fri, 23 Jun 2023 02:37:16 -0700 (PDT) Received: from gerbillo.redhat.com (146-241-231-243.dyn.eolo.it. [146.241.231.243]) by smtp.gmail.com with ESMTPSA id u10-20020a0c8dca000000b0060530c942f4sm4883219qvb.46.2023.06.23.02.37.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 23 Jun 2023 02:37:15 -0700 (PDT) Message-ID: <6cf2ea121c4fdbd04682224c5acf6c73cc47f2f7.camel@redhat.com> Subject: Re: [PATCH net-next v3 01/18] net: Copy slab data for sendmsg(MSG_SPLICE_PAGES) From: Paolo Abeni To: David Howells Cc: netdev@vger.kernel.org, Alexander Duyck , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Willem de Bruijn , David Ahern , Matthew Wilcox , Jens Axboe , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Menglong Dong Date: Fri, 23 Jun 2023 11:37:12 +0200 In-Reply-To: <1969720.1687511219@warthog.procyon.org.uk> References: <634c885ccfb2e49e284aedc60e157bb12e5f3530.camel@redhat.com> <20230620145338.1300897-1-dhowells@redhat.com> <20230620145338.1300897-2-dhowells@redhat.com> <1969720.1687511219@warthog.procyon.org.uk> User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 647BF1C0009 X-Stat-Signature: seuujk8pedkw5ppg3t1pjqmarbw3onrn X-Rspam-User: X-HE-Tag: 1687513040-203463 X-HE-Meta: U2FsdGVkX19ghY1kMfwJSkb6gegOwrC9DMiRc6l1WmCeJEnBNgbLTg18gJ48OaugzRWTPFWOiSDsFjOzLYojokOpl3+7MLKVflHiAk++KX82BuT3rHHUrj90oBXLs/EjmmQ7tsoJQnAH/Pv1rAiy7bHtNNRWl0KSzKFTtSxw0PJO8FNB31LnSKx1WkgFX/azcMqo6Hc7219dO3C64uIcbq80hS8dVeHBOVKSeUqEwWIi8BErEHGQDhww3H5TZa17dkmyzlA84xlL18DBugxGRAoLwUhGKYwKlHTnfdXr9LDbLTcB+J0SbGhspVsdk4NnvfnzAtI18rLyqzrJmKGwtlxULQXpWPN7e+I5MleeWWGBQZzkMO5AbvaUiNokVxndSu0QLFOfH64Ndfdt9tMZ8kmHFtafwNZKe2gmLbVghCNpK9quqr0BymTVyRIufVtKDBSBRENY84aj9b1SB+/SPK7pIwAxYCKbFmGVpn+/aXSsJCyJhV1MBImhTI6dGlBhGaLutqakDkDxqelKobt/7R2TGIpji4SeoRXCntA4+PDWJP+CrJAqirfYMIJ7ZxSQfxcW1vTrapflPm6bivWYHrRuHNdinnanOMw0wo9K2elAlu0kL0WZsbFZdhcnN4Xb+TFuFrb98S4f8UF7bJZATkG6M0EbSZdhI1dNpwKRg79WviuKG/lV8hG6wLlRHC3AE+GawF+1dhUJ7HuDoFc9TlZjzOJkEwRnG/9GN+Fw4eS6FzI+3vlXsKQ28vyoEjW0B2OWh8j7vL5aoNP2sK97z7a+haNqthRkIE4UnJiW9iyWoMnLAoZPG6FTFCRRK37hl8z4sv8xd+v3zAV/9VypJxBSJmSNZSZgYaw+tJHUHnaSb4RfimCATNVQgA4HQCK4SW3/gWmGzrJyqpm+QbIklgtkHFYs5o8JlS3n98F2gKfg/RRujBa2ihTo2GKjzyrR0GrA1Ylk7pEWd+MN1pR wUceryVA no9WacWe3W9QPCMqakCgXuobHp9I31+lOZxzt6/MzmMhcWQddn3brzJ/GdXAbLd32eQ148y8JbxxAM+GhlZi8xtkJXcjz/8EfV0rH/oAXgd02HRtF2vO6PddLx3s6pGSNG4txlK7fkJ7vVR9oEOAQxjTJttcJWM/PuEirixTs4b3jacZ/RKseXxUgkOhQ60/xhDq8C8QHA8lM8gWElZA5LRO26mpnMLFgG9v4Lz7+SrF0qfKjHZsMzc0TYReSxhccnyGIiGL1PaZNFlUKSeNRbnmM1qrRKz1QohmCCryK2u6abD29M2kxyF/kYTppLx9tHw4KQ7pEcr1tolz8ajTioZLV0loLYdljUUk8T9fLTY1rVyGJgs+X9NnkiTcGxUPeTe/kKxmF2aul8ebZm+f8ADezllpZs7quOhbIVtl5pgr3ccsahv5Wy6kNcQk8HiBut7JAo8BX2OxDYOvOEG/mjYj6tdE4+Sd65Vr3soDqgZVQJVW4WxGX5AFpSJ8ZkOv4WsJF 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: On Fri, 2023-06-23 at 10:06 +0100, David Howells wrote: > Paolo Abeni wrote: >=20 > > IMHO this function uses a bit too much labels and would be more easy to > > read, e.g. moving the above chunk of code in conditional branch. >=20 > Maybe. I was trying to put the fast path up at the top without the slow = path > bits in it, but I can put the "insufficient_space" bit there. I *think* you could move the insufficient_space in a separate helped, that should achieve your goal with fewer labels and hopefully no additional complexity. >=20 > > Even without such change, I think the above 'goto try_again;' > > introduces an unneeded conditional, as at this point we know 'fragsz <= =3D > > fsize'. >=20 > Good point. >=20 > > > +=09=09cache->pfmemalloc =3D folio_is_pfmemalloc(spare); > > > +=09=09if (cache->folio) > > > +=09=09=09goto reload; > >=20 > > I think there is some problem with the above. > >=20 > > If cache->folio is !=3D NULL, and cache->folio was not pfmemalloc-ed > > while the spare one is, it looks like the wrong policy will be used. > > And should be even worse if folio was pfmemalloc-ed while spare is not. > >=20 > > I think moving 'cache->pfmemalloc' initialization... > >=20 > > > +=09} > > > + > >=20 > > ... here should fix the above. >=20 > Yeah. We might have raced with someone else or been moved to another cpu= and > there might now be a folio we can allocate from. >=20 > > > +=09/* Reset page count bias and offset to start of new frag */ > > > +=09cache->pagecnt_bias =3D PAGE_FRAG_CACHE_MAX_SIZE + 1; > > > +=09offset =3D folio_size(folio); > > > +=09goto try_again; > >=20 > > What if fragsz > PAGE_SIZE, we are consistently unable to allocate an > > high order page, but order-0, pfmemalloc-ed page allocation is > > successful? It looks like this could become an unbounded loop? >=20 > It shouldn't. It should go: >=20 > =09try_again: > =09=09if (fragsz > offset) > =09=09=09goto insufficient_space; > =09insufficient_space: > =09=09/* See if we can refurbish the current folio. */ > =09=09... I think the critical path is with pfmemalloc-ed pages: =09=09if (unlikely(cache->pfmemalloc)) { =09=09=09__folio_put(folio); =09=09=09goto get_new_folio; =09=09} just before the following. > =09=09fsize =3D folio_size(folio); > =09=09if (unlikely(fragsz > fsize)) > =09=09=09goto frag_too_big; > =09frag_too_big: > =09=09... > =09=09return NULL; >=20 > Though for safety's sake, it would make sense to put in a size check in t= he > case we fail to allocate a larger-order folio. >=20 > > > =09=09do { > > > =09=09=09struct page *page =3D pages[i++]; > > > =09=09=09size_t part =3D min_t(size_t, PAGE_SIZE - off, len); > > > - > > > -=09=09=09ret =3D -EIO; > > > -=09=09=09if (WARN_ON_ONCE(!sendpage_ok(page))) > > > +=09=09=09bool put =3D false; > > > + > > > +=09=09=09if (PageSlab(page)) { > >=20 > > I'm a bit concerned from the above. If I read correctly, tcp 0-copy >=20 > Well, splice()-to-tcp will; MSG_ZEROCOPY is unaffected. Ah right! I got lost in some 'if' branch. > > will go through that for every page, even if the expected use-case is > > always !PageSlub(page). compound_head() could be costly if the head > > page is not hot on cache and I'm not sure if that could be the case for > > tcp 0-copy. The bottom line is that I fear a possible regression here. >=20 > I can put the PageSlab() check inside the sendpage_ok() so the page flag = is > only checked once. =C2=A0 Perhaps I'm lost again, but AFAICS: __PAGEFLAG(Slab, slab, PF_NO_TAIL) // ... #define __PAGEFLAG(uname, lname, policy)=09=09=09\ =09TESTPAGEFLAG(uname, lname, policy)=09=09=09\ // ... #define TESTPAGEFLAG(uname, lname, policy)=09=09=09=09\ static __always_inline bool folio_test_##lname(struct folio *folio)=09\ { return test_bit(PG_##lname, folio_flags(folio, FOLIO_##policy));}=09\ static __always_inline int Page##uname(struct page *page) \ { return test_bit(PG_##lname, &policy(page, 0)->flags); } // ... 'policy' is PF_NO_TAIL here #define PF_NO_TAIL(page, enforce) ({ \ VM_BUG_ON_PGFLAGS(enforce && PageTail(page), page);=09\ PF_POISONED_CHECK(compound_head(page)); }) It looks at compound_head in the end ?!? > But PageSlab() doesn't check the headpage, only the page > it is given. sendpage_ok() is more the problem as it also calls > page_count(). I could drop the check. Once the head page is hot on cache due to the previous check, it should be cheap? Cheers, Paolo