[PATCH net-next] gve: change to use page_pool_put_full_page when recycling pages

[PATCH net-next] gve: change to use page_pool_put_full_page when recycling pages

[Praveen Kaligineedi]

From: Harshitha Ramamurthy <hramamurthy@google.com>

The driver currently uses page_pool_put_page() to recycle
page pool pages. Since gve uses split pages, if the fragment
being recycled is not the last fragment in the page, there
is no dma sync operation. When the last fragment is recycled,
dma sync is performed by page pool infra according to the
value passed as dma_sync_size which right now is set to the
size of fragment.

But the correct thing to do is to dma sync the entire page when
the last fragment is recycled. Hence change to using
page_pool_put_full_page().

Link: https://lore.kernel.org/netdev/89d7ce83-cc1d-4791-87b5-6f7af29a031d@huawei.com/

Suggested-by: Yunsheng Lin <linyunsheng@huawei.com>
Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
---
 drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c b/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
index 05bf1f80a79c..403f0f335ba6 100644
--- a/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
+++ b/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
@@ -210,8 +210,7 @@ void gve_free_to_page_pool(struct gve_rx_ring *rx,
 	if (!page)
 		return;
 
-	page_pool_put_page(page->pp, page, buf_state->page_info.buf_size,
-			   allow_direct);
+	page_pool_put_full_page(page->pp, page, allow_direct);
 	buf_state->page_info.page = NULL;
 }
 
-- 
2.47.0.105.g07ac214952-goog

Re: [PATCH net-next] gve: change to use page_pool_put_full_page when recycling pages

[Yunsheng Lin]

On 2024/10/24 6:11, Praveen Kaligineedi wrote:
> From: Harshitha Ramamurthy <hramamurthy@google.com>
> 
> The driver currently uses page_pool_put_page() to recycle
> page pool pages. Since gve uses split pages, if the fragment
> being recycled is not the last fragment in the page, there
> is no dma sync operation. When the last fragment is recycled,
> dma sync is performed by page pool infra according to the
> value passed as dma_sync_size which right now is set to the
> size of fragment.
> 
> But the correct thing to do is to dma sync the entire page when
> the last fragment is recycled. Hence change to using
> page_pool_put_full_page().

I am not sure if Fixes tag is needed if the blamed commit is only
in the net-next tree. Otherwise, LGTM.

Reviewed-by: Yunsheng Lin <linyunsheng@huawei.com>

> 
> Link: https://lore.kernel.org/netdev/89d7ce83-cc1d-4791-87b5-6f7af29a031d@huawei.com/
> 
> Suggested-by: Yunsheng Lin <linyunsheng@huawei.com>
> Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
> Reviewed-by: Willem de Bruijn <willemb@google.com>
> Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
> ---
>  drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c b/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
> index 05bf1f80a79c..403f0f335ba6 100644
> --- a/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
> +++ b/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
> @@ -210,8 +210,7 @@ void gve_free_to_page_pool(struct gve_rx_ring *rx,
>  	if (!page)
>  		return;
>  
> -	page_pool_put_page(page->pp, page, buf_state->page_info.buf_size,
> -			   allow_direct);
> +	page_pool_put_full_page(page->pp, page, allow_direct);
>  	buf_state->page_info.page = NULL;
>  }
>  

Re: [PATCH net-next] gve: change to use page_pool_put_full_page when recycling pages

[Simon Horman]

On Thu, Oct 24, 2024 at 10:36:02AM +0800, Yunsheng Lin wrote:
> On 2024/10/24 6:11, Praveen Kaligineedi wrote:
> > From: Harshitha Ramamurthy <hramamurthy@google.com>
> > 
> > The driver currently uses page_pool_put_page() to recycle
> > page pool pages. Since gve uses split pages, if the fragment
> > being recycled is not the last fragment in the page, there
> > is no dma sync operation. When the last fragment is recycled,
> > dma sync is performed by page pool infra according to the
> > value passed as dma_sync_size which right now is set to the
> > size of fragment.
> > 
> > But the correct thing to do is to dma sync the entire page when
> > the last fragment is recycled. Hence change to using
> > page_pool_put_full_page().
> 
> I am not sure if Fixes tag is needed if the blamed commit is only
> in the net-next tree. Otherwise, LGTM.

I think it would be best to provide a fixes tag in this case.
It can be done by supplying it in a response to this email thread.
(I think it needs to start at the beginning of a line.)

> Reviewed-by: Yunsheng Lin <linyunsheng@huawei.com>
> 
> > 
> > Link: https://lore.kernel.org/netdev/89d7ce83-cc1d-4791-87b5-6f7af29a031d@huawei.com/
> > 
> > Suggested-by: Yunsheng Lin <linyunsheng@huawei.com>
> > Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
> > Reviewed-by: Willem de Bruijn <willemb@google.com>
> > Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>

...

Re: [PATCH net-next] gve: change to use page_pool_put_full_page when recycling pages

[Harshitha Ramamurthy]

On Thu, Oct 24, 2024 at 8:45 AM Simon Horman <horms@kernel.org> wrote:
>
> On Thu, Oct 24, 2024 at 10:36:02AM +0800, Yunsheng Lin wrote:
> > On 2024/10/24 6:11, Praveen Kaligineedi wrote:
> > > From: Harshitha Ramamurthy <hramamurthy@google.com>
> > >
> > > The driver currently uses page_pool_put_page() to recycle
> > > page pool pages. Since gve uses split pages, if the fragment
> > > being recycled is not the last fragment in the page, there
> > > is no dma sync operation. When the last fragment is recycled,
> > > dma sync is performed by page pool infra according to the
> > > value passed as dma_sync_size which right now is set to the
> > > size of fragment.
> > >
> > > But the correct thing to do is to dma sync the entire page when
> > > the last fragment is recycled. Hence change to using
> > > page_pool_put_full_page().
> >
> > I am not sure if Fixes tag is needed if the blamed commit is only
> > in the net-next tree. Otherwise, LGTM.
>
> I think it would be best to provide a fixes tag in this case.
> It can be done by supplying it in a response to this email thread.
> (I think it needs to start at the beginning of a line.)

Thanks Yunsheng and Simon. I wasn't sure since this patch was targeted
for net-next. I have provided a Fixes tag below.
>
> > Reviewed-by: Yunsheng Lin <linyunsheng@huawei.com>
> >
> > >
> > > Link: https://lore.kernel.org/netdev/89d7ce83-cc1d-4791-87b5-6f7af29a031d@huawei.com/
> > >
Fixes: ebdfae0d377b ("gve: adopt page pool for DQ RDA mode")
> > > Suggested-by: Yunsheng Lin <linyunsheng@huawei.com>
> > > Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
> > > Reviewed-by: Willem de Bruijn <willemb@google.com>
> > > Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
>
> ...

Re: [PATCH net-next] gve: change to use page_pool_put_full_page when recycling pages

[Simon Horman]

On Thu, Oct 24, 2024 at 10:58:47AM -0700, Harshitha Ramamurthy wrote:
> On Thu, Oct 24, 2024 at 8:45 AM Simon Horman <horms@kernel.org> wrote:
> >
> > On Thu, Oct 24, 2024 at 10:36:02AM +0800, Yunsheng Lin wrote:
> > > On 2024/10/24 6:11, Praveen Kaligineedi wrote:
> > > > From: Harshitha Ramamurthy <hramamurthy@google.com>
> > > >
> > > > The driver currently uses page_pool_put_page() to recycle
> > > > page pool pages. Since gve uses split pages, if the fragment
> > > > being recycled is not the last fragment in the page, there
> > > > is no dma sync operation. When the last fragment is recycled,
> > > > dma sync is performed by page pool infra according to the
> > > > value passed as dma_sync_size which right now is set to the
> > > > size of fragment.
> > > >
> > > > But the correct thing to do is to dma sync the entire page when
> > > > the last fragment is recycled. Hence change to using
> > > > page_pool_put_full_page().
> > >
> > > I am not sure if Fixes tag is needed if the blamed commit is only
> > > in the net-next tree. Otherwise, LGTM.
> >
> > I think it would be best to provide a fixes tag in this case.
> > It can be done by supplying it in a response to this email thread.
> > (I think it needs to start at the beginning of a line.)
> 
> Thanks Yunsheng and Simon. I wasn't sure since this patch was targeted
> for net-next. I have provided a Fixes tag below.

Thanks. FWIIW, the tag looks correct to me.

> >
> > > Reviewed-by: Yunsheng Lin <linyunsheng@huawei.com>
> > >
> > > >
> > > > Link: https://lore.kernel.org/netdev/89d7ce83-cc1d-4791-87b5-6f7af29a031d@huawei.com/
> > > >
> Fixes: ebdfae0d377b ("gve: adopt page pool for DQ RDA mode")
> > > > Suggested-by: Yunsheng Lin <linyunsheng@huawei.com>
> > > > Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
> > > > Reviewed-by: Willem de Bruijn <willemb@google.com>
> > > > Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
> >
> > ...
> 

Re: [PATCH net-next] gve: change to use page_pool_put_full_page when recycling pages

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 23 Oct 2024 15:11:41 -0700 you wrote:
> From: Harshitha Ramamurthy <hramamurthy@google.com>
> 
> The driver currently uses page_pool_put_page() to recycle
> page pool pages. Since gve uses split pages, if the fragment
> being recycled is not the last fragment in the page, there
> is no dma sync operation. When the last fragment is recycled,
> dma sync is performed by page pool infra according to the
> value passed as dma_sync_size which right now is set to the
> size of fragment.
> 
> [...]

Here is the summary with links:
  - [net-next] gve: change to use page_pool_put_full_page when recycling pages
    https://git.kernel.org/netdev/net-next/c/4ddf7ccfdf70

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html