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 X-Spam-Level: X-Spam-Status: No, score=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 01652C4743E for ; Fri, 4 Jun 2021 19:42:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DD7F86135D for ; Fri, 4 Jun 2021 19:42:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231158AbhFDToR (ORCPT ); Fri, 4 Jun 2021 15:44:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60400 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230414AbhFDToQ (ORCPT ); Fri, 4 Jun 2021 15:44:16 -0400 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D7D2EC061766; Fri, 4 Jun 2021 12:42:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=bb1jA4KJJMvI5FjBqhzon96/oQku4pcdDpeVipG1x4o=; b=BowMt2rxf6wlCAcaDzFLvRL9vk lu/SlDa8a1E1AUMg39AIg/J6ubklu/VeSWYyra362BJmNu884vvuCzBXgUYLrv6641nMHd9vp/ZuB VgBnT5f4fO9e/LAeEd56w3Os8gbADQ/cJTfPYMXQKTchbkAkOhcDhkykbz99W+EixtD4KR1F5tYW6 3pt6yFc7hZRKPaho4ImmTq6eUTYbAgVr8oJJ6kwfQgkjMtSPhmKf/OWIV3oYdEAmvMVeV3zladMIB V4fka3/qThGS3JCHKyhezmeapa2+QMq52cUIP2WIvCBXZTCInAsqQJ5z2h4HkuPMVolnOyyUCH+CY H7vFInew==; Received: from willy by casper.infradead.org with local (Exim 4.94 #2 (Red Hat Linux)) id 1lpFhY-00DWac-2k; Fri, 04 Jun 2021 19:42:02 +0000 Date: Fri, 4 Jun 2021 20:41:52 +0100 From: Matthew Wilcox To: Matteo Croce Cc: netdev@vger.kernel.org, linux-mm@kvack.org, Ayush Sawal , Vinay Kumar Yadav , Rohit Maheshwari , "David S. Miller" , Jakub Kicinski , Thomas Petazzoni , Marcin Wojtas , Russell King , Mirko Lindner , Stephen Hemminger , Tariq Toukan , Jesper Dangaard Brouer , Ilias Apalodimas , Alexei Starovoitov , Daniel Borkmann , John Fastabend , Boris Pismenny , Arnd Bergmann , Andrew Morton , "Peter Zijlstra (Intel)" , Vlastimil Babka , Yu Zhao , Will Deacon , Fenghua Yu , Roman Gushchin , Hugh Dickins , Peter Xu , Jason Gunthorpe , Jonathan Lemon , Alexander Lobakin , Cong Wang , wenxu , Kevin Hao , Jakub Sitnicki , Marco Elver , Willem de Bruijn , Miaohe Lin , Yunsheng Lin , Guillaume Nault , linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org, bpf@vger.kernel.org, Eric Dumazet , David Ahern , Lorenzo Bianconi , Saeed Mahameed , Andrew Lunn , Paolo Abeni , Sven Auhagen Subject: Re: [PATCH net-next v7 3/5] page_pool: Allow drivers to hint on SKB recycling Message-ID: References: <20210604183349.30040-1-mcroce@linux.microsoft.com> <20210604183349.30040-4-mcroce@linux.microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210604183349.30040-4-mcroce@linux.microsoft.com> Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, Jun 04, 2021 at 08:33:47PM +0200, Matteo Croce wrote: > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index 7fcfea7e7b21..057b40ad29bd 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -40,6 +40,9 @@ > #if IS_ENABLED(CONFIG_NF_CONNTRACK) > #include > #endif > +#ifdef CONFIG_PAGE_POOL > +#include > +#endif I'm not a huge fan of conditional includes ... any reason to not include it always? > @@ -3088,7 +3095,13 @@ static inline void skb_frag_ref(struct sk_buff *skb, int f) > */ > static inline void __skb_frag_unref(skb_frag_t *frag, bool recycle) > { > - put_page(skb_frag_page(frag)); > + struct page *page = skb_frag_page(frag); > + > +#ifdef CONFIG_PAGE_POOL > + if (recycle && page_pool_return_skb_page(page_address(page))) > + return; It feels weird to have a page here, convert it back to an address, then convert it back to a head page in page_pool_return_skb_page(). How about passing 'page' here, calling compound_head() in page_pool_return_skb_page() and calling virt_to_page() in skb_free_head()? > @@ -251,4 +253,11 @@ static inline void page_pool_ring_unlock(struct page_pool *pool) > spin_unlock_bh(&pool->ring.producer_lock); > } > > +/* Store mem_info on struct page and use it while recycling skb frags */ > +static inline > +void page_pool_store_mem_info(struct page *page, struct page_pool *pp) > +{ > + page->pp = pp; I'm not sure this wrapper needs to exist. > +} > + > #endif /* _NET_PAGE_POOL_H */ > diff --git a/net/core/page_pool.c b/net/core/page_pool.c > index e1321bc9d316..a03f48f45696 100644 > --- a/net/core/page_pool.c > +++ b/net/core/page_pool.c > @@ -628,3 +628,26 @@ void page_pool_update_nid(struct page_pool *pool, int new_nid) > } > } > EXPORT_SYMBOL(page_pool_update_nid); > + > +bool page_pool_return_skb_page(void *data) > +{ > + struct page_pool *pp; > + struct page *page; > + > + page = virt_to_head_page(data); > + if (unlikely(page->pp_magic != PP_SIGNATURE)) > + return false; > + > + pp = (struct page_pool *)page->pp; You don't need the cast any more. > + /* Driver set this to memory recycling info. Reset it on recycle. > + * This will *not* work for NIC using a split-page memory model. > + * The page will be returned to the pool here regardless of the > + * 'flipped' fragment being in use or not. > + */ > + page->pp = NULL; > + page_pool_put_full_page(pp, page, false); > + > + return true; > +} > +EXPORT_SYMBOL(page_pool_return_skb_page); > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 12b7e90dd2b5..f769f08e7b32 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -70,6 +70,9 @@ > #include > #include > #include > +#ifdef CONFIG_PAGE_POOL > +#include > +#endif > > #include > #include > @@ -645,10 +648,15 @@ static void skb_free_head(struct sk_buff *skb) > { > unsigned char *head = skb->head; > > - if (skb->head_frag) > + if (skb->head_frag) { > +#ifdef CONFIG_PAGE_POOL > + if (skb->pp_recycle && page_pool_return_skb_page(head)) > + return; > +#endif put this in a header file: static inline bool skb_pp_recycle(struct sk_buff *skb, void *data) { if (!IS_ENABLED(CONFIG_PAGE_POOL) || !skb->pp_recycle) return false; return page_pool_return_skb_page(virt_to_page(data)); } then this becomes: if (skb->head_frag) { if (skb_pp_recycle(skb, head)) return; > skb_free_frag(head); > - else > + } else { > kfree(head); > + } > } > > static void skb_release_data(struct sk_buff *skb)