Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels
@ 2026-11-06  3:17 Harry Yoo (Meta)
  2026-11-06 10:09 ` Breno Leitao
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Harry Yoo (Meta) @ 2026-11-06  3:17 UTC (permalink / raw)
  To: Arthur Kiyanovski, David Arinzon, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Breno Leitao
  Cc: kernel-team, netdev, linux-kernel, Harry Yoo (Meta)

Since commit ef5b0771d247 ("net: ena: fix driver when PAGE_SIZE ==
64kB"), the RX buffer given to the device is limited to ENA_PAGE_SIZE
(16kB) because the buffer length field in the RX descriptor is 16 bits.
However, the driver still allocates a full page for each RX buffer, so
on a 64kB page kernel only the first 16kB of the page is handed to the
device and the remaining 48kB is never used.

The dynamic RX buffer recycling added by commit f7d625adeb7b ("net: ena:
Add dynamic recycling mechanism for rx buffers") does not fully utilize
the page because ena_try_rx_buf_page_reuse() only reuses what is left
in the current buffer and ignores unused memory in the page.

This shows up when RX data stays in socket receive queues, as each
queued skb keeps the whole 64kB page alive while at most 16kB of it
holds data. On an EC2 t4g.large instance (64kB pages, 8GB memory) with
519MB left unread across 64 TCP sockets, RX pages pinned 2869MB of
memory while the socket layer accounted only 687MB.

Fix this by mapping the whole page and letting
ena_try_rx_buf_page_reuse() size the next buffer from the room left in
the page instead of from what is left of ena_buf->len. ena_buf->len is
then only the length written to the RX descriptor and stays capped at
ENA_PAGE_SIZE. With this fixed, a comparable workload pins only 497MB.

When PAGE_SIZE equals ENA_PAGE_SIZE, the room left in the page equals
ena_buf->len, so nothing changes.

Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 7eb6456ed0d5..4a35cb6a7785 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -492,7 +492,7 @@ static struct page *ena_alloc_map_page(struct ena_ring *rx_ring,
 	/* To enable NIC-side port-mirroring, AKA SPAN port,
 	 * we make the buffer readable from the nic as well
 	 */
-	*dma = dma_map_page(rx_ring->dev, page, 0, ENA_PAGE_SIZE,
+	*dma = dma_map_page(rx_ring->dev, page, 0, PAGE_SIZE,
 			    DMA_BIDIRECTIONAL);
 	if (unlikely(dma_mapping_error(rx_ring->dev, *dma))) {
 		ena_increase_stat(&rx_ring->rx_stats.dma_mapping_err, 1,
@@ -544,7 +544,7 @@ static void ena_unmap_rx_buff_attrs(struct ena_ring *rx_ring,
 				    struct ena_rx_buffer *rx_info,
 				    unsigned long attrs)
 {
-	dma_unmap_page_attrs(rx_ring->dev, rx_info->dma_addr, ENA_PAGE_SIZE, DMA_BIDIRECTIONAL,
+	dma_unmap_page_attrs(rx_ring->dev, rx_info->dma_addr, PAGE_SIZE, DMA_BIDIRECTIONAL,
 			     attrs);
 }
 
@@ -926,19 +926,27 @@ static struct sk_buff *ena_alloc_skb(struct ena_ring *rx_ring, void *first_frag,
 	return skb;
 }
 
-static bool ena_try_rx_buf_page_reuse(struct ena_rx_buffer *rx_info, u16 buf_len,
+static bool ena_try_rx_buf_page_reuse(struct ena_ring *rx_ring,
+				      struct ena_rx_buffer *rx_info, u16 buf_len,
 				      u16 len, int pkt_offset)
 {
+	int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 	struct ena_com_buf *ena_buf = &rx_info->ena_buf;
+	unsigned long room;
+
+	/* Room left up to the end of the page. On 64kB page kernels this
+	 * can be more than ena_buf->len, which is capped at ENA_PAGE_SIZE.
+	 */
+	room = PAGE_SIZE - rx_info->page_offset - rx_ring->rx_headroom - tailroom;
 
 	/* More than ENA_MIN_RX_BUF_SIZE left in the reused buffer
 	 * for data + headroom + tailroom.
 	 */
-	if (SKB_DATA_ALIGN(len + pkt_offset) + ENA_MIN_RX_BUF_SIZE <= ena_buf->len) {
+	if (SKB_DATA_ALIGN(len + pkt_offset) + ENA_MIN_RX_BUF_SIZE <= room) {
 		page_ref_inc(rx_info->page);
 		rx_info->page_offset += buf_len;
 		ena_buf->paddr += buf_len;
-		ena_buf->len -= buf_len;
+		ena_buf->len = min(room - buf_len, ENA_PAGE_SIZE);
 		return true;
 	}
 
@@ -1011,7 +1019,7 @@ static struct sk_buff *ena_rx_skb(struct ena_ring *rx_ring,
 
 	/* If XDP isn't loaded try to reuse part of the RX buffer */
 	reuse_rx_buf_page = !is_xdp_loaded &&
-			    ena_try_rx_buf_page_reuse(rx_info, buf_len, len, pkt_offset);
+			    ena_try_rx_buf_page_reuse(rx_ring, rx_info, buf_len, len, pkt_offset);
 
 	if (!reuse_rx_buf_page)
 		ena_unmap_rx_buff_attrs(rx_ring, rx_info, DMA_ATTR_SKIP_CPU_SYNC);
@@ -1055,7 +1063,8 @@ static struct sk_buff *ena_rx_skb(struct ena_ring *rx_ring,
 		pre_reuse_paddr = dma_unmap_addr(&rx_info->ena_buf, paddr);
 
 		reuse_rx_buf_page = !is_xdp_loaded &&
-				    ena_try_rx_buf_page_reuse(rx_info, buf_len, len, pkt_offset);
+				    ena_try_rx_buf_page_reuse(rx_ring, rx_info,
+							      buf_len, len, pkt_offset);
 
 		dma_sync_single_for_cpu(rx_ring->dev,
 					pre_reuse_paddr + pkt_offset,

---
base-commit: 0822d2fe38262e8b55c5ea55c970022bca8dd2a9
change-id: 20261106-ena-rx-reuse-64k-page-3769849b3e0e

Best regards,
--  
Cheers,
Harry / Hyeonggon


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels
  2026-11-06  3:17 [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels Harry Yoo (Meta)
@ 2026-11-06 10:09 ` Breno Leitao
  2026-11-06 11:10   ` Harry Yoo
  2026-11-12  0:47 ` Jakub Kicinski
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Breno Leitao @ 2026-11-06 10:09 UTC (permalink / raw)
  To: Harry Yoo (Meta)
  Cc: Arthur Kiyanovski, David Arinzon, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, kernel-team, netdev,
	linux-kernel

On Fri, Nov 06, 2026 at 03:17:11AM +0000, Harry Yoo (Meta) wrote:
> Since commit ef5b0771d247 ("net: ena: fix driver when PAGE_SIZE ==
> 64kB"), the RX buffer given to the device is limited to ENA_PAGE_SIZE
> (16kB) because the buffer length field in the RX descriptor is 16 bits.
> However, the driver still allocates a full page for each RX buffer, so
> on a 64kB page kernel only the first 16kB of the page is handed to the
> device and the remaining 48kB is never used.
> 
> The dynamic RX buffer recycling added by commit f7d625adeb7b ("net: ena:
> Add dynamic recycling mechanism for rx buffers") does not fully utilize
> the page because ena_try_rx_buf_page_reuse() only reuses what is left
> in the current buffer and ignores unused memory in the page.
> 
> This shows up when RX data stays in socket receive queues, as each
> queued skb keeps the whole 64kB page alive while at most 16kB of it
> holds data. On an EC2 t4g.large instance (64kB pages, 8GB memory) with
> 519MB left unread across 64 TCP sockets, RX pages pinned 2869MB of
> memory while the socket layer accounted only 687MB.
> 
> Fix this by mapping the whole page and letting
> ena_try_rx_buf_page_reuse() size the next buffer from the room left in
> the page instead of from what is left of ena_buf->len. ena_buf->len is
> then only the length written to the RX descriptor and stays capped at
> ENA_PAGE_SIZE. With this fixed, a comparable workload pins only 497MB.
> 
> When PAGE_SIZE equals ENA_PAGE_SIZE, the room left in the page equals
> ena_buf->len, so nothing changes.
> 
> Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>

Nice finding, this seems like a bug, no? Which means 'net' tree and
a Fix: tag.

Reviewed-by: Breno Leitao <leitao@debian.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels
  2026-11-06 10:09 ` Breno Leitao
@ 2026-11-06 11:10   ` Harry Yoo
  0 siblings, 0 replies; 9+ messages in thread
From: Harry Yoo @ 2026-11-06 11:10 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Arthur Kiyanovski, David Arinzon, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, kernel-team, netdev,
	linux-kernel

On Fri, Nov 06, 2026 at 02:09:34AM -0800, Breno Leitao wrote:
> On Fri, Nov 06, 2026 at 03:17:11AM +0000, Harry Yoo (Meta) wrote:
> > Since commit ef5b0771d247 ("net: ena: fix driver when PAGE_SIZE ==
> > 64kB"), the RX buffer given to the device is limited to ENA_PAGE_SIZE
> > (16kB) because the buffer length field in the RX descriptor is 16 bits.
> > However, the driver still allocates a full page for each RX buffer, so
> > on a 64kB page kernel only the first 16kB of the page is handed to the
> > device and the remaining 48kB is never used.
> > 
> > The dynamic RX buffer recycling added by commit f7d625adeb7b ("net: ena:
> > Add dynamic recycling mechanism for rx buffers") does not fully utilize
> > the page because ena_try_rx_buf_page_reuse() only reuses what is left
> > in the current buffer and ignores unused memory in the page.
> > 
> > This shows up when RX data stays in socket receive queues, as each
> > queued skb keeps the whole 64kB page alive while at most 16kB of it
> > holds data. On an EC2 t4g.large instance (64kB pages, 8GB memory) with
> > 519MB left unread across 64 TCP sockets, RX pages pinned 2869MB of
> > memory while the socket layer accounted only 687MB.
> > 
> > Fix this by mapping the whole page and letting
> > ena_try_rx_buf_page_reuse() size the next buffer from the room left in
> > the page instead of from what is left of ena_buf->len. ena_buf->len is
> > then only the length written to the RX descriptor and stays capped at
> > ENA_PAGE_SIZE. With this fixed, a comparable workload pins only 497MB.
> > 
> > When PAGE_SIZE equals ENA_PAGE_SIZE, the room left in the page equals
> > ena_buf->len, so nothing changes.
> > 
> > Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>
> 
> Nice finding,

Thanks for reviewing, Breno!

> this seems like a bug, no? Which means 'net' tree and a Fix: tag.

Good point, I think Fixes: f7d625adeb7b (6.6.y) would be fairly
reasonable as this is the first commit that reuses the buffer,
it is quite huge unaccounted memory waste on kernels with 64kB pages,
and the change is quite minimal (applies cleanly up to 6.6.y).

I'll respin it targeting net tree with Fixes: tag after waiting for
a few days.

> Reviewed-by: Breno Leitao <leitao@debian.org>

Thanks!

-- 
Cheers,
Harry / Hyeonggon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels
  2026-11-06  3:17 [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels Harry Yoo (Meta)
  2026-11-06 10:09 ` Breno Leitao
@ 2026-11-12  0:47 ` Jakub Kicinski
  2026-11-13 21:02   ` Arthur Kiyanovski
  2026-11-13 21:35   ` Arthur Kiyanovski
  2026-11-13 21:10 ` Arthur Kiyanovski
  2026-11-13 21:48 ` Arthur Kiyanovski
  3 siblings, 2 replies; 9+ messages in thread
From: Jakub Kicinski @ 2026-11-12  0:47 UTC (permalink / raw)
  To: Harry Yoo (Meta)
  Cc: Arthur Kiyanovski, David Arinzon, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, Breno Leitao, kernel-team, netdev,
	linux-kernel

On Fri, 06 Nov 2026 03:17:11 +0000 Harry Yoo (Meta) wrote:
> This shows up when RX data stays in socket receive queues, as each
> queued skb keeps the whole 64kB page alive while at most 16kB of it
> holds data. On an EC2 t4g.large instance (64kB pages, 8GB memory) with
> 519MB left unread across 64 TCP sockets, RX pages pinned 2869MB of
> memory while the socket layer accounted only 687MB.

I'm not sure you're actually fixing the problem you're describing?
You optimize the memory use but the (unused) tail of the buffer is
still not charged to any skb, is it? Or does it now fit perfectly?

Normally when the page can no longer be chunked up we should charge
the tail to whoever happened to use it last (see the page pool code
which does that).

Either way, you have to respin this on top of Breno's fix which
was applied earlier today.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels
  2026-11-12  0:47 ` Jakub Kicinski
@ 2026-11-13 21:02   ` Arthur Kiyanovski
  2026-11-13 21:35   ` Arthur Kiyanovski
  1 sibling, 0 replies; 9+ messages in thread
From: Arthur Kiyanovski @ 2026-11-13 21:02 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Harry Yoo (Meta), Arthur Kiyanovski, David Arinzon, Andrew Lunn,
	David S. Miller, Eric Dumazet, Paolo Abeni, Breno Leitao,
	kernel-team, netdev, linux-kernel

On 2026-11-11 16:47:51-08:00, Jakub Kicinski wrote:
> On Fri, 06 Nov 2026 03:17:11 +0000 Harry Yoo (Meta) wrote:
> 
> > This shows up when RX data stays in socket receive queues, as each
> > queued skb keeps the whole 64kB page alive while at most 16kB of it
> > holds data. On an EC2 t4g.large instance (64kB pages, 8GB memory) with
> > 519MB left unread across 64 TCP sockets, RX pages pinned 2869MB of
> > memory while the socket layer accounted only 687MB.
> 
> I'm not sure you're actually fixing the problem you're describing?
> You optimize the memory use but the (unused) tail of the buffer is
> still not charged to any skb, is it? Or does it now fit perfectly?
> 
> Normally when the page can no longer be chunked up we should charge
> the tail to whoever happened to use it last (see the page pool code
> which does that).
> 
> Either way, you have to respin this on top of Breno's fix which
> was applied earlier today.

Jakub,

If I understand correctly what you are saying, you are correct.
skb->truesize is still not reported correctly.

The focus in this patch is different. It is about using the whole
64KB page instead of only the first 16KB.

Harry's own figures show most of it is recovered — 497MB pinned for
519MB unread, against 2869MB before — so the remaining gap is the
final tail of each page, not the 48kB.

So I agree the commit message needs to be more precise.

The skb->truesize fix can come as a separate fix after this one.

Thanks,
Arthur


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels
  2026-11-06  3:17 [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels Harry Yoo (Meta)
  2026-11-06 10:09 ` Breno Leitao
  2026-11-12  0:47 ` Jakub Kicinski
@ 2026-11-13 21:10 ` Arthur Kiyanovski
  2026-11-13 21:48 ` Arthur Kiyanovski
  3 siblings, 0 replies; 9+ messages in thread
From: Arthur Kiyanovski @ 2026-11-13 21:10 UTC (permalink / raw)
  To: Arthur Kiyanovski, David Arinzon, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Breno Leitao
  Cc: kernel-team, netdev, linux-kernel, Harry Yoo (Meta), Shai Brandes

On Fri, 06 Nov 2026 03:17:11 +0000, Harry Yoo (Meta) <harry@kernel.org> wrote:
> [...]
> ENA_PAGE_SIZE. With this fixed, a comparable workload pins only 497MB.
> 
> When PAGE_SIZE equals ENA_PAGE_SIZE, the room left in the page equals
> ena_buf->len, so nothing changes.
> 
> Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>

Hi Harry,
Thank you for the patch. The issue you are fixing is real, and the patch
is very welcome.

I want to suggest a few changes to make sure that the performance of the
datapath of 4KB pages is changed as little as possible, as this will be
the common case.

First:
DRB (Dynamic RX Buffers) - is what we call the reuse of the pages as
long as the remaining part is "large enough" instead of allocating a
new page for every new packet.

The ENA driver supports DRB only in the non XDP case.

Therefore some of the calculations you are doing per call to
ena_try_rx_buf_page_reuse() can be saved as compile time constants.

For that I suggest defining the following in ena_netdev.h:

+ #define ENA_RX_PAGE_ROOM    (PAGE_SIZE - NET_SKB_PAD -          \
+                   SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
+ #define ENA_RX_MAX_BUF_LEN  (ENA_PAGE_SIZE - NET_SKB_PAD -          \
+                   SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))

Next I would like to use the old calculations for the simpler 4KB pages
case. See these suggestions inline.

Could you fold these into the respin?

Thanks,
Arthur

>
>
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 7eb6456ed0d5..4a35cb6a7785 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -926,19 +926,27 @@ static struct sk_buff *ena_alloc_skb(struct ena_ring *rx_ring, void *first_frag,
>  	return skb;
>  }
>  
> -static bool ena_try_rx_buf_page_reuse(struct ena_rx_buffer *rx_info, u16 buf_len,
> +static bool ena_try_rx_buf_page_reuse(struct ena_ring *rx_ring,

Becomes unused: struct ena_ring *rx_ring parameter and in call sites.

> +				      struct ena_rx_buffer *rx_info, u16 buf_len,
>  				      u16 len, int pkt_offset)
>  {
> +	int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));

Becomes unused:
-	int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));

>  	struct ena_com_buf *ena_buf = &rx_info->ena_buf;
> +	unsigned long room;

-	unsigned long room;
+	u32 room;

> +
> +	/* Room left up to the end of the page. On 64kB page kernels this
> +	 * can be more than ena_buf->len, which is capped at ENA_PAGE_SIZE.
> +	 */
> +	room = PAGE_SIZE - rx_info->page_offset - rx_ring->rx_headroom - tailroom;
>  

-       room = PAGE_SIZE - rx_info->page_offset - rx_ring->rx_headroom - tailroom;

In runtime there is no branch here because the result is known at
compile time (same with the next suggestion below):

+       if (PAGE_SIZE > ENA_PAGE_SIZE)
+               room = ENA_RX_PAGE_ROOM - rx_info->page_offset;
+       else
+               room = ena_buf->len;

>  	/* More than ENA_MIN_RX_BUF_SIZE left in the reused buffer
>  	 * for data + headroom + tailroom.
>  	 */
> -	if (SKB_DATA_ALIGN(len + pkt_offset) + ENA_MIN_RX_BUF_SIZE <= ena_buf->len) {
> +	if (SKB_DATA_ALIGN(len + pkt_offset) + ENA_MIN_RX_BUF_SIZE <= room) {
>  		page_ref_inc(rx_info->page);
>  		rx_info->page_offset += buf_len;
>  		ena_buf->paddr += buf_len;
> -		ena_buf->len -= buf_len;
> +		ena_buf->len = min(room - buf_len, ENA_PAGE_SIZE);

-               ena_buf->len = min(room - buf_len, ENA_PAGE_SIZE);
+               if (PAGE_SIZE > ENA_PAGE_SIZE)
+                       ena_buf->len = min_t(u32, room - buf_len,
+                                            ENA_RX_MAX_BUF_LEN);
+               else
+                       ena_buf->len -= buf_len;

Use ENA_RX_MAX_BUF_LEN here too, so that headroom + len + tailroom
stays within ENA_PAGE_SIZE as it does elsewhere in the driver.

-- 
Arthur Kiyanovski <akiyano@amazon.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels
  2026-11-12  0:47 ` Jakub Kicinski
  2026-11-13 21:02   ` Arthur Kiyanovski
@ 2026-11-13 21:35   ` Arthur Kiyanovski
  2026-11-13 23:34     ` Jakub Kicinski
  1 sibling, 1 reply; 9+ messages in thread
From: Arthur Kiyanovski @ 2026-11-13 21:35 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Harry Yoo (Meta), Arthur Kiyanovski, David Arinzon, Andrew Lunn,
	David S. Miller, Eric Dumazet, Paolo Abeni, Breno Leitao,
	kernel-team, netdev, linux-kernel

On 2026-11-11 16:47:51-08:00, Jakub Kicinski wrote:
> On Fri, 06 Nov 2026 03:17:11 +0000 Harry Yoo (Meta) wrote:
> 
> > This shows up when RX data stays in socket receive queues, as each
> > queued skb keeps the whole 64kB page alive while at most 16kB of it
> > holds data. On an EC2 t4g.large instance (64kB pages, 8GB memory) with
> > 519MB left unread across 64 TCP sockets, RX pages pinned 2869MB of
> > memory while the socket layer accounted only 687MB.
> 
> I'm not sure you're actually fixing the problem you're describing?
> You optimize the memory use but the (unused) tail of the buffer is
> still not charged to any skb, is it? Or does it now fit perfectly?
> 
> Normally when the page can no longer be chunked up we should charge
> the tail to whoever happened to use it last (see the page pool code
> which does that).
> 
> Either way, you have to respin this on top of Breno's fix which
> was applied earlier today.


Jakub,

If I understand correctly what you are saying, you are correct.
skb->truesize is still not reported correctly.

The focus in this patch is different. It is about using the whole
64KB page instead of only the first 16KB.

Harry's own figures show most of it is recovered — 497MB pinned for
519MB unread, against 2869MB before — so the remaining gap is the
final tail of each page, not the 48kB.

So I agree the commit message needs to be more precise.

The skb->truesize fix can come as a separate fix after this one.

Thanks,
Arthur


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels
  2026-11-06  3:17 [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels Harry Yoo (Meta)
                   ` (2 preceding siblings ...)
  2026-11-13 21:10 ` Arthur Kiyanovski
@ 2026-11-13 21:48 ` Arthur Kiyanovski
  3 siblings, 0 replies; 9+ messages in thread
From: Arthur Kiyanovski @ 2026-11-13 21:48 UTC (permalink / raw)
  To: Harry Yoo (Meta)
  Cc: Arthur Kiyanovski, David Arinzon, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Breno Leitao,
	kernel-team, netdev, linux-kernel

On Fri, 06 Nov 2026 03:17:11 +0000, Harry Yoo (Meta) <harry@kernel.org> wrote:
> [...]
> ENA_PAGE_SIZE. With this fixed, a comparable workload pins only 497MB.
> 
> When PAGE_SIZE equals ENA_PAGE_SIZE, the room left in the page equals
> ena_buf->len, so nothing changes.
> 
> Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>

Hi Harry,
Thank you for the patch. The issue you are fixing is real, and the patch
is very welcome.

I want to suggest a few changes to make sure that the performance of the
datapath of 4KB pages is changed as little as possible, as this will be
the common case.

First:
DRB (Dynamic RX Buffers) - is what we call the reuse of the pages as
long as the remaining part is "large enough" instead of allocating a
new page for every new packet.

The ENA driver supports DRB only in the non XDP case.

Therefore some of the calculations you are doing per call to
ena_try_rx_buf_page_reuse() can be saved as compile time constants.

For that I suggest defining the following in ena_netdev.h:

+ #define ENA_RX_PAGE_ROOM    (PAGE_SIZE - NET_SKB_PAD -          \
+                   SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
+ #define ENA_RX_MAX_BUF_LEN  (ENA_PAGE_SIZE - NET_SKB_PAD -          \
+                   SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))

Next I would like to use the old calculations for the simpler 4KB pages
case. See these suggestions inline.

Could you fold these into the respin?

Thanks,
Arthur

>
>
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 7eb6456ed0d5..4a35cb6a7785 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -926,19 +926,27 @@ static struct sk_buff *ena_alloc_skb(struct ena_ring *rx_ring, void *first_frag,
>  	return skb;
>  }
>  
> -static bool ena_try_rx_buf_page_reuse(struct ena_rx_buffer *rx_info, u16 buf_len,
> +static bool ena_try_rx_buf_page_reuse(struct ena_ring *rx_ring,

Becomes unused: struct ena_ring *rx_ring parameter and in call sites.

> +				      struct ena_rx_buffer *rx_info, u16 buf_len,
>  				      u16 len, int pkt_offset)
>  {
> +	int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));

Becomes unused:
-       int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));

>  	struct ena_com_buf *ena_buf = &rx_info->ena_buf;
> +	unsigned long room;

-       unsigned long room;
+       u32 room;

> +
> +	/* Room left up to the end of the page. On 64kB page kernels this
> +	 * can be more than ena_buf->len, which is capped at ENA_PAGE_SIZE.
> +	 */
> +	room = PAGE_SIZE - rx_info->page_offset - rx_ring->rx_headroom - tailroom;
>  

-       room = PAGE_SIZE - rx_info->page_offset - rx_ring->rx_headroom - tailroom;

In runtime there is no branch here because the result is known at
compile time (same with the next suggestion below):

+       if (PAGE_SIZE > ENA_PAGE_SIZE)
+               room = ENA_RX_PAGE_ROOM - rx_info->page_offset;
+       else
+               room = ena_buf->len;

>  	/* More than ENA_MIN_RX_BUF_SIZE left in the reused buffer
>  	 * for data + headroom + tailroom.
>  	 */
> -	if (SKB_DATA_ALIGN(len + pkt_offset) + ENA_MIN_RX_BUF_SIZE <= ena_buf->len) {
> +	if (SKB_DATA_ALIGN(len + pkt_offset) + ENA_MIN_RX_BUF_SIZE <= room) {
>  		page_ref_inc(rx_info->page);
>  		rx_info->page_offset += buf_len;
>  		ena_buf->paddr += buf_len;
> -		ena_buf->len -= buf_len;
> +		ena_buf->len = min(room - buf_len, ENA_PAGE_SIZE);

-               ena_buf->len = min(room - buf_len, ENA_PAGE_SIZE);
+               if (PAGE_SIZE > ENA_PAGE_SIZE)
+                       ena_buf->len = min_t(u32, room - buf_len,
+                                            ENA_RX_MAX_BUF_LEN);
+               else
+                       ena_buf->len -= buf_len;

Use ENA_RX_MAX_BUF_LEN here too, so that headroom + len + tailroom
stays within ENA_PAGE_SIZE as it does elsewhere in the driver.

-- 
Arthur Kiyanovski <akiyano@amazon.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels
  2026-11-13 21:35   ` Arthur Kiyanovski
@ 2026-11-13 23:34     ` Jakub Kicinski
  0 siblings, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2026-11-13 23:34 UTC (permalink / raw)
  To: Arthur Kiyanovski
  Cc: Harry Yoo (Meta), David Arinzon, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, Breno Leitao, kernel-team, netdev,
	linux-kernel

On Fri, 13 Nov 2026 21:35:17 +0000 Arthur Kiyanovski wrote:
> On 2026-11-11 16:47:51-08:00, Jakub Kicinski wrote:
> > On Fri, 06 Nov 2026 03:17:11 +0000 Harry Yoo (Meta) wrote:
> >   
> > > This shows up when RX data stays in socket receive queues, as each
> > > queued skb keeps the whole 64kB page alive while at most 16kB of it
> > > holds data. On an EC2 t4g.large instance (64kB pages, 8GB memory) with
> > > 519MB left unread across 64 TCP sockets, RX pages pinned 2869MB of
> > > memory while the socket layer accounted only 687MB.  
> > 
> > I'm not sure you're actually fixing the problem you're describing?
> > You optimize the memory use but the (unused) tail of the buffer is
> > still not charged to any skb, is it? Or does it now fit perfectly?
> > 
> > Normally when the page can no longer be chunked up we should charge
> > the tail to whoever happened to use it last (see the page pool code
> > which does that).
> > 
> > Either way, you have to respin this on top of Breno's fix which
> > was applied earlier today.  
> 
> 
> Jakub,
> 
> If I understand correctly what you are saying, you are correct.
> skb->truesize is still not reported correctly.
> 
> The focus in this patch is different. It is about using the whole
> 64KB page instead of only the first 16KB.
> 
> Harry's own figures show most of it is recovered — 497MB pinned for
> 519MB unread, against 2869MB before — so the remaining gap is the
> final tail of each page, not the 48kB.
> 
> So I agree the commit message needs to be more precise.
> 
> The skb->truesize fix can come as a separate fix after this one.

You sent it twice? Some LLM automation gone wrong judging by the
m-dashes in your reply? :/

Making more efficient use of allocated pages is not a fix, and
will not be accepted to net or stable. Not accounting truesize 
correctly can be considered a fix. IOW the truesize accounting 
change has to come _first_. Then we can optimize the memory use 
w/ 64kB pages. They can both go to net-next, but the cannot both
go to net.

Hope spelling out practical implications of what I said earlier
makes it clear to everyone..

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-11-13 23:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-06  3:17 [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels Harry Yoo (Meta)
2026-11-06 10:09 ` Breno Leitao
2026-11-06 11:10   ` Harry Yoo
2026-11-12  0:47 ` Jakub Kicinski
2026-11-13 21:02   ` Arthur Kiyanovski
2026-11-13 21:35   ` Arthur Kiyanovski
2026-11-13 23:34     ` Jakub Kicinski
2026-11-13 21:10 ` Arthur Kiyanovski
2026-11-13 21:48 ` Arthur Kiyanovski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox