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 ECA70C4332F for ; Mon, 11 Dec 2023 19:32:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5E1766B01F6; Mon, 11 Dec 2023 14:32:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 56A8B6B01F7; Mon, 11 Dec 2023 14:32:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4320E6B01F8; Mon, 11 Dec 2023 14:32:09 -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 2FA066B01F6 for ; Mon, 11 Dec 2023 14:32:09 -0500 (EST) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id EBF6F807A4 for ; Mon, 11 Dec 2023 19:32:08 +0000 (UTC) X-FDA: 81555533136.30.C952EE3 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf23.hostedemail.com (Postfix) with ESMTP id 22AE3140006 for ; Mon, 11 Dec 2023 19:32:06 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=VwB9x+sO; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf23.hostedemail.com: domain of kuba@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=kuba@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702323127; 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=olfu2FRY8IduEsqoeMU2rkUkSs1QdZGFtTm6R0bmrTs=; b=mLetE17oz4P34I5ggLcE1HV/NgUCwWVVR9rMeoT0ORJoUqvVmq4LjB724ItmDLrb5KVpAk b42nqC6AttIX69yqhpsiJo7lDl9u6D5aov99EBjhKTZOkmC53P3XxP2h6uNb7hQQsBkrt+ tDMf4YAQZ56e0/MoLAg5+90npXOw3fE= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=VwB9x+sO; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf23.hostedemail.com: domain of kuba@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=kuba@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702323127; a=rsa-sha256; cv=none; b=OxkShijB7FKKd53lCgT0WmNIdzcHCCGocQx29e7sFAoTK3hr48wn/gwQ1XIXEEz+cuI8u4 FP3wiooh8OQJ+16gO9eK7OJMsgr3kt4kTCuPoo739mVoAkIY6OMYm/ML0j93rg/bpSanPo +biDtiNd/YKJCwTdmDNwqFRixGD76zE= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by ams.source.kernel.org (Postfix) with ESMTP id 7FC19B80F92; Mon, 11 Dec 2023 19:32:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6D2D7C433C7; Mon, 11 Dec 2023 19:32:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1702323124; bh=LPLfUWNHRirQ2vRL7AGp6B484wCdzz0S+UFQPz/KsoA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=VwB9x+sOUVbTtmBWRUy+VcMJJhSqr4uK6nArQznkfV64srs/Xvp9Ds4x+F+t9zTMh JteyfGFGQNd5t/MoOlYbcgqwDqRwSDzPEL5u+Gsjj4hWd5hp+xGSo2utrDbK0kBher P+qiZ2DsTpKJT74b3N8q5CVqWIoWC4NZljasrOQFhFRY7gjeFSSkdHsXlkIqVwU+z2 cCjYkas+BvKCp4czZu3zoGcaABpP+MqW0SE1q4mVmT5Dpe7P5ZRFFjNE6uveDcnk3+ ihorlEMBdWet1+6NYcy78JuZSIR3dBp6kyumCPsxLREgsHItvGslBNtzb6/fcWoDEA zQDht5IpGuymg== Date: Mon, 11 Dec 2023 11:32:03 -0800 From: Jakub Kicinski To: Mina Almasry 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 Subject: Re: [PATCH net-next v7 4/4] skbuff: Optimization of SKB coalescing for page pool Message-ID: <20231211113203.2ae8bccf@kernel.org> In-Reply-To: References: <20231206105419.27952-1-liangchen.linux@gmail.com> <20231206105419.27952-5-liangchen.linux@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 22AE3140006 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: p63kbrxuswmomqbqnwcrpa7i378jpwtx X-HE-Tag: 1702323126-648979 X-HE-Meta: U2FsdGVkX1+jkOOOcNlTd9GuhaXDf4iUAWYJsxWbd+Nal6qKrXXU/YOKzXAF8JzYimKlqGWviloNYsJABxi3/6rzStpim9DH0mQb8GJHjfe2P2Cej0moLXuMWGd3C+SurxNAlYqcdDeS5KXhDVM/B9JKX13E2Ln+tptw6NpSJKCqZnNCSD2IYgSgSmZ0BH2j8AF/hZ1Ybt12Otsi0Mhar7z3aJ6otDT/AWGkBT5HQDjV3o0r0Ou1x8Fn8e3kYdK4mZs/vZykUvpFP+ng4F3cUAeqZff8O+RtD2k6XBthjBNpMXx+aHNFOZeTebMJmikkaa2kFc4FHpwaG7KPlq0x1vnibzD4MwKOtUiLkeJCwRMLkLjVoijHczUwh1AwDylZRRMYxnk4SPTk/izRxxY0IoeQcq4+lgDZ79toiCU/qStZbK5+JH7MQ9CXkhCw8yleUrI6tXVy/2aprFEGGB4WRrwMHD8mdw1PmjnfEV7HBATK5w2hu+lKh5adzb932O7uL+cH7pExdAxoTtLyYGom9oaew5kbXPh95Cd5WQNB6kjuw/wM7t3bc0e6GfVvLjFMEDMueJ4JtM8dD7JcIeWH0lTtjuAqLy5mUPjACHV5m/fyTMIexJVqClQ9ygtSydbC6th496OHAVu0TjleUGwDkXy2A+Bep4zF7d2ubukod4zYJYvdbK8pjE2Bc+OZXQAHzIIu8xvBvuj/jh6jTSUWXe+OJqZUlNnGHNqE2O0LyaqhVEjTewXrkx0PYWL7ZfAobO5QP6IpOiWK1/FCbvhYxcGxgWoZIaHYqE3DD+/rj6+7cQnOwjDZdgryzKvZeEJ3e4yMd26VNKvLhMNKsPsxV2sgWkiwDr7WgW4XSrWd4lzyThi51eV/aODhCD75QMla70HFtrITHGhNGzLmdRJXOsPlFh1rRmI5owvsuM1gCsQ35wbGzWTJfP7OThoiyMYUn0R+LtqhpG/cAsMSVOw sHhlunAo 1HpPTsvSLL0B9Mgosy9LsbxgUq3yqLLAgdrKMJBqzhkM5u3wOrS/4e0ijZDTlKEgKe4cBL870+hC4t+JQNtj1yazhI4AV/xLrVa13p/3sG/flKIYerYfQvdLcMYCvEKKQ96Z+ZOvAat3X0a89HWPqzU0zL6ifoTitKRoFlRsFgPEVzdSrxUoW0z+Ni8eqnp2I+PLPNQYvw3G/fl2ye3oiaSHHI9NYvdooQ7AasoRILHnIfo+935HnZmlcB9X/H9cuWcXLtzPJmYpf3LNvesGh4g6HMJq1y3vtMhjkV9KDNaz6OraG/udV/QMcvTZbwGNlDJP8 X-Bogosity: Ham, tests=bogofilter, spamicity=0.005463, 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 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). 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?