Hi Alexander, On Tue, Feb 12, 2019 at 10:13:30AM -0800, Alexander Duyck wrote: > On Tue, Feb 12, 2019 at 7:16 AM Eric Dumazet <eric.duma...@gmail.com> wrote: > > > > > > > > On 02/12/2019 04:39 AM, Tariq Toukan wrote: > > > > > > > > > On 2/11/2019 7:14 PM, Eric Dumazet wrote: > > >> > > >> > > >> On 02/11/2019 12:53 AM, Tariq Toukan wrote: > > >>> > > >> > > >>> Hi, > > >>> > > >>> It's great to use the struct page to store its dma mapping, but I am > > >>> worried about extensibility. > > >>> page_pool is evolving, and it would need several more per-page fields. > > >>> One of them would be pageref_bias, a planned optimization to reduce the > > >>> number of the costly atomic pageref operations (and replace existing > > >>> code in several drivers). > > >>> > > >> > > >> But the point about pageref_bias is to place it in a different cache > > >> line than "struct page" > > >> > > >> The major cost is having a cache line bouncing between producer and > > >> consumer. > > >> > > > > > > pageref_bias is meant to be dirtied only by the page requester, i.e. the > > > NIC driver / page_pool. > > > All other components (basically, SKB release flow / put_page) should > > > continue working with the atomic page_refcnt, and not dirty the > > > pageref_bias. > > > > This is exactly my point. > > > > You suggested to put pageref_bias in struct page, which breaks this > > completely. > > > > pageref_bias is better kept in a driver structure, with appropriate > > prefetching > > since most NIC use a ring buffer for their queues. > > > > The dma address _can_ be put in the struct page, since the driver does not > > dirty it > > and does not even read it when page can be recycled. > > Instead of maintaining the pageref_bias in the page itself it could be > maintained in some sort of separate structure. You could just maintain > a pointer to a slot in an array somewhere. Then you can still access > it if needed, the pointer would be static for as long as it is in the > page pool, and you could invalidate the pointer prior to removing the > bias from the page.
I think that's what Tariq was suggesting in the first place. /Ilias
