Re: [RFC, PATCH] net: page_pool: Don't use page->private to store dma_addr_t

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Matthew,

On Thu, Feb 07, 2019 at 07:07:45AM -0800, Matthew Wilcox wrote:
> On Thu, Feb 07, 2019 at 04:36:36PM +0200, Ilias Apalodimas wrote:
> > +/* Until we can update struct-page, have a shadow struct-page, that
> > + * include our use-case
> > + * Used to store retrieve dma addresses from network drivers.
> > + * Never access this directly, use helper functions provided
> > + * page_pool_get_dma_addr()
> > + */
> 
> Huh?  Why not simply:
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 2c471a2c43fa..2495a93ad90c 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -28,6 +28,10 @@ struct address_space;
>  struct mem_cgroup;
>  struct hmm;
>  
> +struct page_pool {
> +       dma_addr_t dma_addr;
> +};
> +
>  /*
>   * Each physical page in the system has a struct page associated with
>   * it to keep track of whatever it is we are using the page for at the
> @@ -77,6 +81,7 @@ struct page {
>          * avoid collision and false-positive PageTail().
>          */
>         union {
> +               struct page_pool pool;
>                 struct {        /* Page cache and anonymous pages */
>                         /**
>                          * @lru: Pageout list, eg. active_list protected by
> 

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

Thanks
/Ilias