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. However, what bothers me more is another issue. The optimization doesn't cleanly combine with the new page_pool direction for maintaining a queue for "available" pages, as the put_page flow would need to read pageref_bias, asynchronously, and act accordingly. The suggested hook in put_page (to catch the 2 -> 1 "biased refcnt" transition) causes a problem to the traditional pageref_bias idea, as it implies a new point in which the pageref_bias field is read *asynchronously*. This would risk missing the this critical 2 -> 1 transition! Unless pageref_bias is atomic... > pageref_bias means the producer only have to read the "struct page" and not > dirty it > in the case the page can be recycled. > > > >> I would replace this dma field with a pointer to an extensible struct, >> that would contain the dma mapping (and other stuff in the near future). >> This pointer fits perfectly with the existing unsigned long private; >> they can share the memory, for both 32- and 64-bits systems. >> >> The only downside is one more pointer de-reference. This should be perf >> tested. >> However, when introducing the page refcnt bias optimization into >> page_pool, I believe the perf gain would be guaranteed. > > Only in some cases perhaps (when the cache line can be dirtied without > performance hit) >
