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] [thread-next>] [day] [month] [year] [list]

Message-ID: <e9c92aab-16bc-4814-8902-7796b9d29826@huawei.com>
Date: Thu, 17 Oct 2024 17:40:04 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Praveen Kaligineedi <pkaligineedi@...gle.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

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);
>>> +}
>>> +