netdev - Re: [PATCH net-next v3 2/3] gve: adopt page pool for DQ RDA mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CA+f9V1N2H5_uWQ4eVa7r3Gv1JA=AW_VZu4vpC-Th8rujgpKTQA@mail.gmail.com>
Date: Thu, 17 Oct 2024 13:20:14 -0700
From: Praveen Kaligineedi <pkaligineedi@...gle.com>
To: Yunsheng Lin <linyunsheng@...wei.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com, 
	kuba@...nel.org, pabeni@...hat.com, willemb@...gle.com, jeroendb@...gle.com, 
	shailend@...gle.com, hramamurthy@...gle.com, ziweixiao@...gle.com, 
	shannon.nelson@....com, jacob.e.keller@...el.com
Subject: Re: [PATCH net-next v3 2/3] gve: adopt page pool for DQ RDA mode

Thanks Yunsheng for the clarification. It makes sense to me now. We
will send a patch calling page_pool_put_full_page instead of
page_pool_put_page.

--Praveen

On Thu, Oct 17, 2024 at 2:40 AM Yunsheng Lin <linyunsheng@...wei.com> wrote:
>
> On 2024/10/17 3:43, Praveen Kaligineedi wrote:
> > Thanks Yunsheng. One thing that's not clear to me - the GVE driver
> > does not call page_pool_put_page with dma_sync_size of 0 anywhere. Is
> > this still an issue in that case?
>
> It depends on what's value of 'dma_sync_size', as the value of the
> below 'page_info.buf_size' seems to be the size of one fragment, so
> it might end up only doing the dma_sync operation for the first fragment,
> and what we want might be to dma sync all the fragments in the same page.
>
> The doc about that in Documentation/networking/page_pool.rst seems a
> little outdated, but what it meant is still true as my understanding:
>
> https://elixir.bootlin.com/linux/v6.11.3/source/Documentation/networking/page_pool.rst#L101
>
> >
> > Thanks,
> > Praveen
> >
> >
> > On Wed, Oct 16, 2024 at 2:21 AM Yunsheng Lin <linyunsheng@...wei.com> wrote:
> >>
> >> On 2024/10/15 4:21, Praveen Kaligineedi wrote:
> >>
> >> ...
> >>
> >>> +void gve_free_to_page_pool(struct gve_rx_ring *rx,
> >>> +                        struct gve_rx_buf_state_dqo *buf_state,
> >>> +                        bool allow_direct)
> >>> +{
> >>> +     struct page *page = buf_state->page_info.page;
> >>> +
> >>> +     if (!page)
> >>> +             return;
> >>> +
> >>> +     page_pool_put_page(page->pp, page, buf_state->page_info.buf_size,
> >>> +                        allow_direct);
> >>
> >> page_pool_put_full_page() might be a better option here for now when
> >> page_pool is created with PP_FLAG_DMA_SYNC_DEV flag and frag API like
> >> page_pool_alloc() is used in gve_alloc_from_page_pool(), as explained
> >> in below:
> >>
> >> https://lore.kernel.org/netdev/20241014143542.000028dc@gmail.com/T/#mdaba23284a37affc2c46ef846674ae6aa49f8f04
> >>
> >>
> >>> +     buf_state->page_info.page = NULL;
> >>> +}
> >>> +
> >>> +static int gve_alloc_from_page_pool(struct gve_rx_ring *rx,
> >>> +                                 struct gve_rx_buf_state_dqo *buf_state)
> >>> +{
> >>> +     struct gve_priv *priv = rx->gve;
> >>> +     struct page *page;
> >>> +
> >>> +     buf_state->page_info.buf_size = priv->data_buffer_size_dqo;
> >>> +     page = page_pool_alloc(rx->dqo.page_pool,
> >>> +                            &buf_state->page_info.page_offset,
> >>> +                            &buf_state->page_info.buf_size, GFP_ATOMIC);
> >>> +
> >>> +     if (!page)
> >>> +             return -ENOMEM;
> >>> +
> >>> +     buf_state->page_info.page = page;
> >>> +     buf_state->page_info.page_address = page_address(page);
> >>> +     buf_state->addr = page_pool_get_dma_addr(page);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +struct page_pool *gve_rx_create_page_pool(struct gve_priv *priv,
> >>> +                                       struct gve_rx_ring *rx)
> >>> +{
> >>> +     u32 ntfy_id = gve_rx_idx_to_ntfy(priv, rx->q_num);
> >>> +     struct page_pool_params pp = {
> >>> +             .flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
> >>> +             .order = 0,
> >>> +             .pool_size = GVE_PAGE_POOL_SIZE_MULTIPLIER * priv->rx_desc_cnt,
> >>> +             .dev = &priv->pdev->dev,
> >>> +             .netdev = priv->dev,
> >>> +             .napi = &priv->ntfy_blocks[ntfy_id].napi,
> >>> +             .max_len = PAGE_SIZE,
> >>> +             .dma_dir = DMA_FROM_DEVICE,
> >>> +     };
> >>> +
> >>> +     return page_pool_create(&pp);
> >>> +}
> >>> +