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

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 <redacted>
Reviewed-by: Praveen Kaligineedi <redacted>
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

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 <redacted>

On Thu, Oct 24, 2024 at 10:36:02AM +0800, Yunsheng Lin wrote:

On 2024/10/24 6:11, Praveen Kaligineedi wrote:

quoted

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 <redacted>

quoted

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

Suggested-by: Yunsheng Lin <redacted>
Reviewed-by: Praveen Kaligineedi <redacted>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>

...

On Thu, Oct 24, 2024 at 8:45 AM Simon Horman [off-list ref] wrote:

On Thu, Oct 24, 2024 at 10:36:02AM +0800, Yunsheng Lin wrote:

quoted

On 2024/10/24 6:11, Praveen Kaligineedi wrote:

quoted

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.

quoted

Reviewed-by: Yunsheng Lin <redacted>

quoted

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

Fixes: ebdfae0d377b ("gve: adopt page pool for DQ RDA mode")

quoted

quoted

Suggested-by: Yunsheng Lin <redacted>
Reviewed-by: Praveen Kaligineedi <redacted>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>

...

On Thu, Oct 24, 2024 at 10:58:47AM -0700, Harshitha Ramamurthy wrote:

On Thu, Oct 24, 2024 at 8:45 AM Simon Horman [off-list ref] wrote:

quoted

On Thu, Oct 24, 2024 at 10:36:02AM +0800, Yunsheng Lin wrote:

quoted

On 2024/10/24 6:11, Praveen Kaligineedi wrote:

quoted

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.

quoted

quoted

Reviewed-by: Yunsheng Lin <redacted>

quoted

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

Fixes: ebdfae0d377b ("gve: adopt page pool for DQ RDA mode")

quoted

quoted

quoted

Suggested-by: Yunsheng Lin <redacted>
Reviewed-by: Praveen Kaligineedi <redacted>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>

...

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski [off-list ref]:

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