On 2/7/2019 11:42 PM, Ilias Apalodimas wrote: > Hi Matthew, > > On Thu, Feb 07, 2019 at 01:34:00PM -0800, Matthew Wilcox wrote: >> On Thu, Feb 07, 2019 at 01:25:19PM -0800, David Miller wrote: >>> From: Ilias Apalodimas <ilias.apalodi...@linaro.org> >>> Date: Thu, 7 Feb 2019 17:20:34 +0200 >>> >>>> Well updating struct page is the final goal, hence the comment. I am mostly >>>> looking for opinions here since we are trying to store dma addresses which >>>> are >>>> irrelevant to pages. Having dma_addr_t definitions in mm-related headers >>>> is a >>>> bit controversial isn't it ? If we can add that, then yes the code would >>>> look >>>> better >>> >>> I fundamentally disagree. >>> >>> One of the core operations performed on a page is mapping it so that a >>> device >>> and use it. >>> >>> Why have ancillary data structure support for this all over the place, >>> rather >>> than in the common spot which is the page. >>> >>> A page really is not just a 'mm' structure, it is a system structure. >> >> +1 >> >> The fundamental point of computing is to do I/O. > Ok, great that should sort it out then. > I'll use your proposal and base the patch on that. > > Thanks for taking the time with this > > /Ilias >
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). 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. Regards, Tariq