diff mbox series

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

Message ID 20241023221141.3008011-1-pkaligineedi@google.com (mailing list archive)
State Accepted
Commit 4ddf7ccfdf70364010005b0b695b1a0d92677425
Delegated to: Netdev Maintainers
Headers show
Series [net-next] gve: change to use page_pool_put_full_page when recycling pages | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 5 this patch: 5
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 11 of 11 maintainers
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 4 this patch: 4
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-10-29--06-00 (tests: 777)

Commit Message

Praveen Kaligineedi Oct. 23, 2024, 10:11 p.m. UTC 
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(-)

Comments

Yunsheng Lin Oct. 24, 2024, 2:36 a.m. UTC | #1 
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;
>  }
>
Simon Horman Oct. 24, 2024, 3:45 p.m. UTC | #2 
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>

...
Harshitha Ramamurthy Oct. 24, 2024, 5:58 p.m. UTC | #3 
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>
>
> ...
Simon Horman Oct. 25, 2024, 9:09 a.m. UTC | #4 
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>
> >
> > ...
>
patchwork-bot+netdevbpf@kernel.org Oct. 31, 2024, 1:10 a.m. UTC | #5 
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!
diff mbox series

Patch

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