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=-16.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 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 2DF6DC11F69 for ; Mon, 12 Jul 2021 08:33:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1AE2C61002 for ; Mon, 12 Jul 2021 08:33:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1377052AbhGLIfM (ORCPT ); Mon, 12 Jul 2021 04:35:12 -0400 Received: from szxga08-in.huawei.com ([45.249.212.255]:11256 "EHLO szxga08-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344103AbhGLHrs (ORCPT ); Mon, 12 Jul 2021 03:47:48 -0400 Received: from dggemv703-chm.china.huawei.com (unknown [172.30.72.55]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4GNbHr2mLHz1CJ2P; Mon, 12 Jul 2021 15:39:20 +0800 (CST) Received: from dggpemm500005.china.huawei.com (7.185.36.74) by dggemv703-chm.china.huawei.com (10.3.19.46) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Mon, 12 Jul 2021 15:44:48 +0800 Received: from [10.69.30.204] (10.69.30.204) by dggpemm500005.china.huawei.com (7.185.36.74) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2176.2; Mon, 12 Jul 2021 15:44:48 +0800 Subject: Re: [PATCH rfc v2 2/5] page_pool: add interface for getting and setting pagecnt_bias To: Alexander Duyck CC: David Miller , Jakub Kicinski , Russell King - ARM Linux , Marcin Wojtas , , , "Salil Mehta" , , , Ilias Apalodimas , "Alexei Starovoitov" , Daniel Borkmann , "John Fastabend" , Andrew Morton , Peter Zijlstra , "Will Deacon" , Matthew Wilcox , "Vlastimil Babka" , , , Peter Xu , Feng Tang , Jason Gunthorpe , Matteo Croce , Hugh Dickins , Jonathan Lemon , "Alexander Lobakin" , Willem de Bruijn , , Cong Wang , Kevin Hao , , Marco Elver , Netdev , LKML , bpf References: <1625903002-31619-1-git-send-email-linyunsheng@huawei.com> <1625903002-31619-3-git-send-email-linyunsheng@huawei.com> From: Yunsheng Lin Message-ID: Date: Mon, 12 Jul 2021 15:44:47 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.69.30.204] X-ClientProxiedBy: dggeme706-chm.china.huawei.com (10.1.199.102) To dggpemm500005.china.huawei.com (7.185.36.74) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2021/7/11 0:55, Alexander Duyck wrote: > On Sat, Jul 10, 2021 at 12:44 AM Yunsheng Lin wrote: >> >> As suggested by Alexander, "A DMA mapping should be page >> aligned anyway so the lower 12 bits would be reserved 0", >> so it might make more sense to repurpose the lower 12 bits >> of the dma address to store the pagecnt_bias for elevated >> refcnt case in page pool. >> >> As newly added page_pool_get_pagecnt_bias() may be called >> outside of the softirq context, so annotate the access to >> page->dma_addr[0] with READ_ONCE() and WRITE_ONCE(). >> >> Other three interfaces using page->dma_addr[0] is only called >> in the softirq context during normal rx processing, hopefully >> the barrier in the rx processing will ensure the correct order >> between getting and setting pagecnt_bias. >> >> Signed-off-by: Yunsheng Lin >> --- >> include/net/page_pool.h | 24 ++++++++++++++++++++++-- >> 1 file changed, 22 insertions(+), 2 deletions(-) >> >> diff --git a/include/net/page_pool.h b/include/net/page_pool.h >> index 8d7744d..5746f17 100644 >> --- a/include/net/page_pool.h >> +++ b/include/net/page_pool.h >> @@ -200,7 +200,7 @@ static inline void page_pool_recycle_direct(struct page_pool *pool, >> >> static inline dma_addr_t page_pool_get_dma_addr(struct page *page) >> { >> - dma_addr_t ret = page->dma_addr[0]; >> + dma_addr_t ret = READ_ONCE(page->dma_addr[0]) & PAGE_MASK; >> if (sizeof(dma_addr_t) > sizeof(unsigned long)) >> ret |= (dma_addr_t)page->dma_addr[1] << 16 << 16; >> return ret; >> @@ -208,11 +208,31 @@ static inline dma_addr_t page_pool_get_dma_addr(struct page *page) >> >> static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr) >> { >> - page->dma_addr[0] = addr; >> + unsigned long dma_addr_0 = READ_ONCE(page->dma_addr[0]); >> + >> + dma_addr_0 &= ~PAGE_MASK; >> + dma_addr_0 |= (addr & PAGE_MASK); > > So rather than doing all this testing and clearing it would probably > be better to add a return value to the function and do something like: > > if (WARN_ON(dma_addr_0 & ~PAGE_MASK)) > return -1; > > That way you could have page_pool_dma_map unmap, free the page, and > return false indicating that the DMA mapping failed with a visible > error in the event that our expectionat that the dma_addr is page > aligned is ever violated. I suppose the above is based on that page_pool_set_dma_addr() is called only once before page_pool_set_pagecnt_bias(), right? so we could: static inline bool page_pool_set_dma_addr(struct page *page, dma_addr_t addr) { if (WARN_ON(dma_addr_0 & ~PAGE_MASK)) return false; page->dma_addr[0] = addr; if (sizeof(dma_addr_t) > sizeof(unsigned long)) page->dma_addr[1] = upper_32_bits(addr); return true; } > >> + WRITE_ONCE(page->dma_addr[0], dma_addr_0); >> + >> if (sizeof(dma_addr_t) > sizeof(unsigned long)) >> page->dma_addr[1] = upper_32_bits(addr); >> } >> >> +static inline int page_pool_get_pagecnt_bias(struct page *page) >> +{ >> + return (READ_ONCE(page->dma_addr[0]) & ~PAGE_MASK); > > You don't need the parenthesis around the READ_ONCE and PAGE_MASK. ok. > >> +} >> + >> +static inline void page_pool_set_pagecnt_bias(struct page *page, int bias) >> +{ >> + unsigned long dma_addr_0 = READ_ONCE(page->dma_addr[0]); >> + >> + dma_addr_0 &= PAGE_MASK; >> + dma_addr_0 |= (bias & ~PAGE_MASK); >> + >> + WRITE_ONCE(page->dma_addr[0], dma_addr_0); >> +} >> + >> static inline bool is_page_pool_compiled_in(void) >> { >> #ifdef CONFIG_PAGE_POOL >> -- >> 2.7.4 >> > . >