Messages in this thread | | | Date | Wed, 29 Nov 2023 14:17:26 +0100 | Subject | Re: [Intel-wired-lan] [PATCH net-next v5 03/14] page_pool: avoid calling no-op externals when possible | From | Alexander Lobakin <> |
| |
From: Yunsheng Lin <linyunsheng@huawei.com> Date: Wed, 29 Nov 2023 11:17:50 +0800
> On 2023/11/27 22:32, Alexander Lobakin wrote: >> >> Chris, any thoughts on a global flag for skipping DMA syncs ladder? > > It seems there was one already in the past: > > https://lore.kernel.org/netdev/7c55a4d7-b4aa-25d4-1917-f6f355bd722e@arm.com/T/
It addresses a different problem, meaningless indirect calls, while this one addresses meaningless direct calls :> When the above gets merged, we could combine these two into one global, but Eric wasn't active with his patch and I remember there were some problems, so I wouldn't count on that it will arrive soon.
> >> >>> >>> > >>>> +static inline bool page_pool_set_dma_addr(const struct page_pool *pool, >>>> + struct page *page, >>>> + dma_addr_t addr) >>>> { >>>> + unsigned long val = addr; >>>> + >>>> + if (unlikely(!addr)) { >>>> + page->dma_addr = 0; >>>> + return true; >>>> + } >>> >>> The above seems unrelated change? >> >> Related. We use page_put_set_dma_addr() to clear ::dma_addr as well >> (grep for it in page_pool.c). In this case, we don't want >> dma_need_sync() to be called as we explicitly pass zero. This check >> zeroes the field and exits as quickly as possible. > > The question seems to be about if we need to ensure the LSB of > page->dma_addr is not set when page_pool releases a page back to page > allocator?
But why do we need to call dma_need_sync(0) when freeing a page wasting CPU cycles on relatively hot path?
> >> In case with the call mentioned above, zero is a compile-time constant >> there, so that this little branch will be inlined with the rest dropped.
Thanks, Olek
| |