Received-SPF: pass (google.com: domain of linux-kernel+bounces-94858-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 6 Mar 2024 18:16:00 -0800
From: Jakub Kicinski <kuba@kernel.org>
To: Mina Almasry <almasrymina@google.com>
Cc: Liang Chen <liangchen.linux@gmail.com>, Yunsheng Lin
 <linyunsheng@huawei.com>, Dragos Tatulea <dtatulea@nvidia.com>,
 "davem@davemloft.net" <davem@davemloft.net>, "herbert@gondor.apana.org.au"
 <herbert@gondor.apana.org.au>, Gal Pressman <gal@nvidia.com>,
 "dsahern@kernel.org" <dsahern@kernel.org>, "steffen.klassert@secunet.com"
 <steffen.klassert@secunet.com>, "linux-kernel@vger.kernel.org"
 <linux-kernel@vger.kernel.org>, "pabeni@redhat.com" <pabeni@redhat.com>,
 Leon Romanovsky <leonro@nvidia.com>, "edumazet@google.com"
 <edumazet@google.com>, "ian.kumlien@gmail.com" <ian.kumlien@gmail.com>,
 "Anatoli.Chechelnickiy@m.interpipe.biz"
 <Anatoli.Chechelnickiy@m.interpipe.biz>, "netdev@vger.kernel.org"
 <netdev@vger.kernel.org>
Subject: Re: [RFC] net: esp: fix bad handling of pages from page_pool
Message-ID: <20240306181600.5af8ef5f@kernel.org>
In-Reply-To: <CAHS8izOoO-EovwMwAm9tLYetwikNPxC0FKyVGu1TPJWSz4bGoA@mail.gmail.com>
References: <20240304094950.761233-1-dtatulea@nvidia.com>
	<20240305190427.757b92b8@kernel.org>
	<7fc334b847dc4d90af796f84a8663de9f43ede5d.camel@nvidia.com>
	<20240306072225.4a61e57c@kernel.org>
	<320ef2399e48ba0a8a11a3b258b7ad88384f42fb.camel@nvidia.com>
	<20240306080931.2e24101b@kernel.org>
	<CAHS8izMw_hxdoNDoCZs8T7c5kmX=0Lwqw_dboSj7z1LqtS-WKA@mail.gmail.com>
	<9a78b37abdf40daafd9936299ea2c08f936ad3d5.camel@nvidia.com>
	<20240306094133.7075c39f@kernel.org>
	<CAHS8izN436pn3SndrzsCyhmqvJHLyxgCeDpWXA4r1ANt3RCDLQ@mail.gmail.com>
	<CAHS8izOoO-EovwMwAm9tLYetwikNPxC0FKyVGu1TPJWSz4bGoA@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Wed, 6 Mar 2024 10:46:45 -0800 Mina Almasry wrote:
> Gah, nevermind, skb_pp_frag_ref() actually returns -EINVAL if
> !skb->pp_recycle, and in the call site we do a skb_frag_ref() on this
> error, so all in all we end up doing a get_page/put_page pair. Sorry
> for the noise.
> 
> So we're supposed to:
> - !skb->pp_recycle && is_pp_page()
> ref via get_page
> unref via put_page
> 
> Very subtle stuff (for me at least). I'll try to propose some cleanup
> to make this a bit simpler using helpers that handle all these subtle
> details internally so that the call sites don't have to do this
> special handling.

Sure, although complexity is complexity, we can only do so much to hide
it.

For pp_recycle - the problem is when we added page pool pages, hardly
anything in the upper layers of the stack was made pp aware. So we can
end up with someone doing

	get_page(page);
	skb_fill_page_desc(skb, page);

on a PP page.

You probably inspected a lot of those cases for the ZC work, and they
can be fixed up to do a "pp-aware get()", but until then we need
skb->pp_recycle. pp_recycle kinda denotes whether whoever constructed
the skb was PP aware.

So, yes, definitely a good long term goal, but not in a fix :)