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 8F74DC4332F for ; Mon, 11 Dec 2023 20:07:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D9A6A6B01FC; Mon, 11 Dec 2023 15:07:20 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D49436B01FE; Mon, 11 Dec 2023 15:07:20 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C38656B01FF; Mon, 11 Dec 2023 15:07:20 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id B483D6B01FC for ; Mon, 11 Dec 2023 15:07:20 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 7A43740843 for ; Mon, 11 Dec 2023 20:07:20 +0000 (UTC) X-FDA: 81555621840.11.A4D865C Received: from mail-vk1-f171.google.com (mail-vk1-f171.google.com [209.85.221.171]) by imf08.hostedemail.com (Postfix) with ESMTP id AFE61160020 for ; Mon, 11 Dec 2023 20:07:18 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=GStUPvF9; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of almasrymina@google.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=almasrymina@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702325238; 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=TliIQAhu/pCdffbMcTGtCdKTnFLxSs38Z+sDzdIobAQ=; b=TJPe1tsl0XbFFEzDQGuCjD1Tem26e8sc2znSx0qS/m85e4jk/xrI1iUtzbiwLh9GTZHMPc oVvj0UsE0Vv2sCWLSAtNJTB3TuT9jtBX/6dV2vNNDdgjhtqCKScVTMeXqWMj4wmbSJcgvy KJ2MbcRRvaLLwwsIfbXBTED29KTpM8o= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=GStUPvF9; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of almasrymina@google.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=almasrymina@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702325238; a=rsa-sha256; cv=none; b=jwlyWScWZt75vhrtziDRMhRKzNCxTMsJ1dgDp+W23yc+4FkeHFs1/+hv4jAWbFNOi5mT9L KmD4C0GLyDBC3UYvdSfpEc0en1itHTMbE4l2hKgGhPQjnQgsY6D4Mx87kL8iOh9Scys2qH 2tQxhYygQaC6XdRTfEtPckDP2V6IjPk= Received: by mail-vk1-f171.google.com with SMTP id 71dfb90a1353d-4b2f3539089so2719258e0c.0 for ; Mon, 11 Dec 2023 12:07:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702325237; x=1702930037; 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=TliIQAhu/pCdffbMcTGtCdKTnFLxSs38Z+sDzdIobAQ=; b=GStUPvF9ioO8+9w7YA1v8HT/lK4estPq7XJJIo7KjUDS4J3Cqgj+h40WD2BtsPjBSd jSPCmczsbf0Ry8TOf2K8fgC2w0JaRCKH4eDbhwyd+1RgCYo2iSYTO1tT1J2dVWXevo+W 4ourN83sQxzpjC626yrmOWCh+sL/2lTMwi/6vGUHinGDlLMtxPrPTmzrK9OhchIKdeiM VJyLkaNPwAq6Q2xBBMZfcuN53nqHgVI8c2KuwDVuo8bIJUjZVLGL8UZOwJyDqiQozA3I 545ivwhau8e5N7A69sm1BMmldW4bly0r6gojChdYzc0tk8wZ2WBi4xD4LRVMFzOMH1c5 exPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702325237; x=1702930037; 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=TliIQAhu/pCdffbMcTGtCdKTnFLxSs38Z+sDzdIobAQ=; b=LwtB14HdwscnVqCrtOPatUtgIJcXBaKsVbtJuKuw68kd1Kqlwkk8kHrj1aRrri02Q2 Lni3A08G3Etg2IIcryUxIcd1lz6Ac+u6An9rtGI/Uh4MNNfcc8xP72x4kicpUgnEQKr7 Z37bUljcjvVFMjTVNXk1OhbvkCzUDvQgkbVkVS7ml1rvoVBeis1PXpBABVKigyENVyC1 dnlXqbhocPhef8vjCd8ofJlgPvFo9LiuCfUX14elCshCMBSFtxcGsPUwaPVAJAmvzNe6 YKGE/C4/QSWBEy7f1LnCakUL3/BerJUk9LZZ9Iv4r+MWoyxGrhvFGDrqUMj6drjp5Gxx Kvsg== X-Gm-Message-State: AOJu0YyvxK9gAe8MTYcwi86IrAWR9IAqfQuiweqxkxfIN49O1ZQo+gS2 2mjLEwKeXlcKYJVbU5xoVgBAIe1umg7QO8UReZOBpA== X-Google-Smtp-Source: AGHT+IE2XNyXB9LN+CcM/yDOZ9FuUhcOtPVL3OAz+8cA46emfRDRZ3nlWO9zNWR6/KXXjiLP5hM7skukVbE5bgggYB4= X-Received: by 2002:a05:6122:4a11:b0:4ac:2054:6a27 with SMTP id ez17-20020a0561224a1100b004ac20546a27mr4568671vkb.2.1702325237431; Mon, 11 Dec 2023 12:07:17 -0800 (PST) MIME-Version: 1.0 References: <20231206105419.27952-1-liangchen.linux@gmail.com> <20231206105419.27952-5-liangchen.linux@gmail.com> <20231211113203.2ae8bccf@kernel.org> In-Reply-To: <20231211113203.2ae8bccf@kernel.org> From: Mina Almasry Date: Mon, 11 Dec 2023 12:07:06 -0800 Message-ID: Subject: Re: [PATCH net-next v7 4/4] skbuff: Optimization of SKB coalescing for page pool To: Jakub Kicinski Cc: Liang Chen , davem@davemloft.net, edumazet@google.com, pabeni@redhat.com, hawk@kernel.org, ilias.apalodimas@linaro.org, linyunsheng@huawei.com, netdev@vger.kernel.org, linux-mm@kvack.org, jasowang@redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: AFE61160020 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: 7yn7fko4uj7wsiywzeqhnxf1tb6pr1i1 X-HE-Tag: 1702325238-409460 X-HE-Meta: U2FsdGVkX182L71WqyeKLY2B3OqrCR5LSoQOv17oCxnhquv2c19ZM+i9T9j16jqgk6BsD7ZOa4SVBvhcBMgyt0d94s2RtzuQH18IfTPexcwOIOTRC7Eu3+vaNF2DoTTea/8V7eO2Oy+egePAKiJB91lUaTmy4jXos8Y/r+uW1tIaVoFW6g/Wt58nvncsLnqhWpZfN18IO/BzWq30+gFr6PQHvkqqhn+CVWP0XAtdP0vGPcV/oV2TH/PhyvH9at9S10E60lQuwIRzqP/Tp9w/NmnSRw0+HKq4imJfY0MouSeMcJ6OMSbZnfC4sMjjdzVKvaH6n8RCtGq2csaHkDcFJMXmcwqEFtISepmI6kSuAHyqExTKsNCU9s8UEJC+2VkcBQMrzTEdIOS4mG2X69F1I59ntkr4q7R9k+1PJt+eE1cq1fVbJB3S9ekQ+VejZxChzCduOUtKQA7gJrnmDbZZMotydQMom/bevhi5O0WDcGHWSFDP8boZbbTmE9sIohAqnakJvXPDrIVlN2ORzkiwSbetVBdKrbjh0PdGv1lDGOkojqL7oKRxeZxh4M9DTZZ26x1kzW9y39GDuK1M/7Gkr/+w/1P3TGWbjaLFM10mcAmRT5RrOmysfHQORnr6rmguzzXPeFty/fH2T+OCr/pbcLHK33O4RKFmCuep4qY/+BIfDcrsLjK0eN1nTaWmk9LxUb5EO1mHE3wpAFL9lFzCO4tqyPArMs1rBSd/C2j19TLGNFeii1tFAqowxhVpMS55wMgNfRwZMd5yZ1Wupkf6wwxoGg16UO5k4IvZN2T7NPJewK507+KVhvhZfcicIfC13iQnKQI7Thdz5twvnJtbCIItdYJiYEFa8ZHaKywkrMdMMdMhPPxqzvbj0YgwHZfZX0nZpfeYqYuoqZV4LKWgr235PT2YVL0Fo50VgnkbLgcKpT8Fk7/UggXQLuA3sfJTP/zne3tFs5iDZIzkMEe HQoQtuT3 IUz+REcYEu5duwce1qZg7SoBXIt8tD7PVQsLu54mMbGe5hpi4VteBp4LPEWBY99FjPJSFaErOw6uY6FuxyofCr/cF8dA+X9V2ihJ9Wj9+Npro6aw08UZeGwHYmE8hp71OrgzAs1VuLI6DlddBZSxsPNIj3H9YFEfYvU8IFW/A28dFhY23WKVCR94dq4kprQKvI6mVhNG0AiShHH90BkZPo3+JFJ6rH5FWHtAs8UagXpFuQ73Es7r/xNHLlAiUANLpFBAxha0REk6rHz5DBCIy0kZRv5KuH+Kh4V+u13hlZYtUjLeEhjsum9+5Sep9VXD6tY5yUQFKEXTho4Tw8I1HvUfvBfXGNfgavWQaqvVbUPRVL2E= X-Bogosity: Ham, tests=bogofilter, spamicity=0.143071, 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, Dec 11, 2023 at 11:32=E2=80=AFAM Jakub Kicinski w= rote: > > On Sun, 10 Dec 2023 20:21:21 -0800 Mina Almasry wrote: > > Is it possible/desirable to add a comment to skb_frag_ref() that it > > should not be used with skb->pp_recycle? At least I was tripped by > > this, but maybe it's considered obvious somehow. > > > > But I feel like this maybe needs to be fixed. Why does the page_pool > > need a separate page->pp_ref_count? Why not use page->_refcount like > > the rest of the code? Is there a history here behind this decision > > that you can point me to? It seems to me that > > incrementing/decrementing page->pp_ref_count may be equivalent to > > doing the same on page->_refcount. > > Does reading the contents of the comment I proposed here: > https://lore.kernel.org/all/20231208173816.2f32ad0f@kernel.org/ > elucidate it? The pp_ref_count means the holder is aware that > they can't release the reference by calling put_page(). > Because (a) we may need to clean up the pp state, unmap DMA etc. > and (b) one day it may not even be a real page (your work). > Thank you, that makes sense. > TBH I'm partial to the rename from patch 1, so I wouldn't delay this > work any more :) But you have a point that we should inspect the code > and consider making the semantics of skb_frag_ref() stronger all by > itself, without the need to add a new flavor of the helper.. > Are you okay with leaving that as a follow up or do you reckon it's > easy enough we should push for it now? I think the rename from pp_frag_count -> pp_ref_count is a huge improvement, and I think the fact that the netstack has a way to obtain a reference on a pp frag is also a huge improvement. Please go ahead mearging this if you like, I was asking questions for my own education/follow up work to consider. --=20 Thanks, Mina