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 7E4C1C4332F for ; Mon, 11 Dec 2023 20:14:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0A8786B0200; Mon, 11 Dec 2023 15:14:17 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 058126B0202; Mon, 11 Dec 2023 15:14:17 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E63F76B0203; Mon, 11 Dec 2023 15:14:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id CAFEA6B0200 for ; Mon, 11 Dec 2023 15:14:16 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id A1B9F1A07CE for ; Mon, 11 Dec 2023 20:14:16 +0000 (UTC) X-FDA: 81555639312.26.5BEB9C9 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf06.hostedemail.com (Postfix) with ESMTP id 9629A180017 for ; Mon, 11 Dec 2023 20:14:14 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=FL0fmRjE; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf06.hostedemail.com: domain of kuba@kernel.org designates 145.40.73.55 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=1702325655; 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=4ZB6Z0hUwc4j5m6d3egu91kgFjHk9Hit/WNRxypoM4s=; b=goaDKEoK5lYg+pGkNib49eXy8Kzsh9VMPBlBEVlZbgVnxm8nteqbm8VXaXSZnZWipv+Wo8 d/0ukaGF/V0ChD6943PyPv2st5rB7dfCQPb6YG+8X/GIocwr4kgSPN0t0VWmg7iYOGoye7 21fgCMJuPNgaruHZMXkSjW5kzk+Ga1g= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=FL0fmRjE; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf06.hostedemail.com: domain of kuba@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=kuba@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702325655; a=rsa-sha256; cv=none; b=F0rkwT0mkbnJECQfBHFh+zUlc8cQ/kJAnvc62sJrkQN0ibqQhIehgWGFTDFZUM1c+UWI0V 3xVHJCHyUAKIU1tWbNsEvIo3qiALQ5r40EwGj8JwrFKwzUqYYStENr/GLmbVftfbLKh5kh V4EqzKflQ0q4tmK6KnOjjauaCroek0U= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 49B7ACE140D; Mon, 11 Dec 2023 20:14:11 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2A780C433C8; Mon, 11 Dec 2023 20:14:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1702325650; bh=1Xtip+7vpslDMt+EAlOYykStn1lMrjnsm0PG/aFT3lc=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=FL0fmRjEtdFc6xX6XIthj3lBq4lv0BgbGAUj8ztWy+gxPy4ndQUS7sAu1ADc1TN/O ME1ZTTSjRU980syPqJFHPi9bqDGcH8Xn3C00RvFfytaj8f0Q6MlX2S9CRB4eJsQBHv 2GCAQRmRmspQQ1KJNQMq1zjkEksjWIgiTvBjWHu4mtPUaymmSWcjLMtV/IWNqtnghY mp7FCXXxukF9UCo88CRuGMjj7tVLIFSiqbi2U3ZzuZTQGafQiTRusVUhzIKeYVYlfj nuU85A/6OY6enMRJDty/w8eoMIrJuL5Xb7YQnDOW/fVOYLNMIe+KThLzWHAqqm4mNU ECjfuEu1UmyvQ== Date: Mon, 11 Dec 2023 12:14:09 -0800 From: Jakub Kicinski To: Ilias Apalodimas Cc: Liang Chen , davem@davemloft.net, edumazet@google.com, pabeni@redhat.com, hawk@kernel.org, linyunsheng@huawei.com, netdev@vger.kernel.org, linux-mm@kvack.org, jasowang@redhat.com, almasrymina@google.com Subject: Re: [PATCH net-next v8 4/4] skbuff: Optimization of SKB coalescing for page pool Message-ID: <20231211121409.5cfaebd5@kernel.org> In-Reply-To: References: <20231211035243.15774-1-liangchen.linux@gmail.com> <20231211035243.15774-5-liangchen.linux@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 9629A180017 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: fb6bi7snr6nzwssjz1e68dg5uuoej6d5 X-HE-Tag: 1702325654-165012 X-HE-Meta: U2FsdGVkX1/JxpI63rmaHo303f0jjitX67ROk35TXyITAJMpeHuaHhee4ziT+d67UmoC14q4dFbsaR5w675yL1ZTQxPpSqldYQ4WozXxiOOEljWdXjCEMKGMn8lby0f7XHMvWEv+okNo75VhOjoQlCdyOHQ2JNNZGuEqBLtXHm0/t/oxsvbg4KIjUeWuqfCYOYwKI8i9YFI/UmVHtFt0lFptA2V2YMJ7wwCLYUyLAbjVSa+fRojkj9sX2Esxf8xfX9W4OoIcmlmVJPZAuu4LnMzNUiAxLzEAAILfND5WNg2heY79HDTx4QNx1ztCRUaBGOPkn3WJpz4RLEG35z8aRP5fPTKBMrk1lSiYydH7Mbxdwjqji9QOOsSahqhBrAn35Xi3QQtnm+14nsinmcCfmQkgvfRuBKb1jh1NJ+NzC2wfsruy3n89wfv/2igL3BtkH3PRuQxlSb3zZTXgfwo/b/i6J95ClYnWFeootdFrWumgZgtvDEkJAMmcfuQoODsIkkPiBgVzNx93C6UYSsFi2MpsImi0xe+uFtyD0/zXWXEPssT5bP2AGNJaNMqJyOmUoe+/urr+XdrMB2BHvVDtLDYzoGArPCG8NSqC/7vo6/nBkswHoC9QxO4bcbsMANm/wHq8u1LKXE1cUbQ78jO2bL5BhRAvPgrI9qzoCZL70zYPUBt37Lg+YOBBpbQMl1oXdmSVXadVg7fvmOnzETa9IFmuqVU3Uw+12oRisIbRMADSDevFOs+acwCJpQdaMBW5cvY5XUl+z3mDmgztWPZCEf/o7FPiAGH3jkFbBMLJf8ORFSPAbESDwft2iL5HzZwVO9MGirPfPA/pVPlgX1RUpzVQurP/JvG1dHXk2bR792twxeVzX0vYGV9FtZWpkduhkEpm47QJ67aQy4rSrHPMqrbpvuT8Qx91r1WtUokHqno/0WOMSgfwQ/v5FJ0O6r8UqxwY1IiEh50Wfk5SqID zLmVhI+j /vzPGb7FThZ3xxdrDCLLfqLEGa+drIuWAh8XPX9kKi9A8l1Z0km8IpXa6oVdVDLTPpMmemdZqT4hQ3ft9yEbFJ/+bC+G7u1kFkbD26G7dlOqGXCurGXxu/cb/VAkWPvMyk0vTCpWoYOB7cQCx9Cm4L2vKQ3mUZUublV9FAfNMcmWx7tLH8xEbwXYQVPzeN0fomUFPIqxM8l+BKxoT/+tESu4iSZocaQwxN4KAzbWnScn2lPivSCZJtuEFa015Aavf2wsI 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, 11 Dec 2023 09:46:55 +0200 Ilias Apalodimas wrote: > As I said in the past the patch look correct. I don't like the fact > that more pp internals creep into the default network stack, but > perhaps this is fine with the bigger adoption? > Jakub any thoughts/objections? Now that you asked... the helper does seem to be in sort of a in-between state of being skb specific. What worries me is that this: +/** + * skb_pp_frag_ref() - Increase fragment reference count of a page + * @page: page of the fragment on which to increase a reference + * + * Increase fragment reference count (pp_ref_count) on a page, but if it is + * not a page pool page, fallback to increase a reference(_refcount) on a + * normal page. + */ +static void skb_pp_frag_ref(struct page *page) +{ + struct page *head_page = compound_head(page); + + if (likely(is_pp_page(head_page))) + page_pool_ref_page(head_page); + else + page_ref_inc(head_page); +} doesn't even document that the caller must make sure that the skb which owns this page is marked for pp recycling. The caller added by this patch does that, but we should indicate somewhere that doing skb_pp_frag_ref() for frag in a non-pp-recycling skb is not correct. We can either lean in the direction of making it less skb specific, put the code in page_pool.c / helpers.h and make it clear that the caller has to be careful. Or we make it more skb specific, take a skb pointer as arg, and also look at its recycling marking.. or just improve the kdoc.