[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e9c9b58d-4b78-fe2d-4511-b0ffa744a994@huawei.com>
Date: Wed, 8 Nov 2023 19:25:16 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Mina Almasry <almasrymina@...gle.com>
CC: <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-arch@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
<linux-media@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
<linaro-mm-sig@...ts.linaro.org>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Jesper Dangaard Brouer <hawk@...nel.org>,
Ilias Apalodimas <ilias.apalodimas@...aro.org>,
Arnd Bergmann <arnd@...db.de>,
David Ahern <dsahern@...nel.org>,
Willem de Bruijn <willemdebruijn.kernel@...il.com>,
Shuah Khan <shuah@...nel.org>,
Sumit Semwal <sumit.semwal@...aro.org>,
Christian König <christian.koenig@....com>,
Shakeel Butt <shakeelb@...gle.com>,
Jeroen de Borst <jeroendb@...gle.com>,
Praveen Kaligineedi <pkaligineedi@...gle.com>
Subject: Re: [RFC PATCH v3 08/12] net: support non paged skb frags
On 2023/11/8 5:19, Mina Almasry wrote:
>>
>>
>
> My personal immediate reaction is that this may just introduce code
> churn without significant benefit. If an unsuspecting caller call
> skb_frag_page() on devmem frag and doesn't correctly handle NULL
> return, it will crash or error out anyway, and likely in some obvious
> way, so maybe the BUG_ON() isn't so useful that it's worth changing
If it will always crash or error out, then I agree that BUG_ON() is
unnecessary.
> all the call sites. But if there is consensus on adding a change like
> you propose, I have no problem adding it.
One obvious benefit I forget to mention is that, it provides a better
semantic that if a caller need to do the return checking:
1. For the old helper, the semantic is not to do the checking if the caller
has ensure that it has passed a readable frag to skb_frag_page(), which
avoid adding some overhead for non-devmen supported drivers.
2. For the new helper, the semantic is to do the checking and we may provide
a compiler '__must_check' function-attribute to ensure the caller to do
the checking.
>
Powered by blists - more mailing lists