All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xsk: clear page contiguity bit when unmapping pool
@ 2022-06-27 19:01 Ivan Malov
  2022-06-28  0:17 ` [PATCH v2 1/1] " Ivan Malov
  2022-06-28  9:18 ` [PATCH net v3 " Ivan Malov
  0 siblings, 2 replies; 6+ messages in thread
From: Ivan Malov @ 2022-06-27 19:01 UTC (permalink / raw)
  To: Magnus Karlsson, Björn Töpel, netdev
  Cc: Andrew Rybchenko, bpf, linux-kernel, Björn Töpel,
	Maciej Fijalkowski, Jonathan Lemon, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
	Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
	Maxim Mikityanskiy

When a XSK pool gets mapped, xp_check_dma_contiguity() adds bit 0x1
to pages' DMA addresses that go in ascending order and at 4K stride.
The problem is that the bit does not get cleared before doing unmap.

As a result, a lot of warnings from iommu_dma_unmap_page() are seen
suggesting mapping lookup failures at drivers/iommu/dma-iommu.c:848.

Fixes: 2b43470add8c ("xsk: Introduce AF_XDP buffer allocation API")

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
---
 net/xdp/xsk_buff_pool.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 87bdd71c7bb6..f70112176b7c 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -332,6 +332,7 @@ static void __xp_dma_unmap(struct xsk_dma_map *dma_map, unsigned long attrs)
 	for (i = 0; i < dma_map->dma_pages_cnt; i++) {
 		dma = &dma_map->dma_pages[i];
 		if (*dma) {
+			*dma &= ~XSK_NEXT_PG_CONTIG_MASK;
 			dma_unmap_page_attrs(dma_map->dev, *dma, PAGE_SIZE,
 					     DMA_BIDIRECTIONAL, attrs);
 			*dma = 0;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v2 1/1] xsk: clear page contiguity bit when unmapping pool
  2022-06-27 19:01 [PATCH] xsk: clear page contiguity bit when unmapping pool Ivan Malov
@ 2022-06-28  0:17 ` Ivan Malov
  2022-06-28  7:47   ` Magnus Karlsson
  2022-06-28  9:18 ` [PATCH net v3 " Ivan Malov
  1 sibling, 1 reply; 6+ messages in thread
From: Ivan Malov @ 2022-06-28  0:17 UTC (permalink / raw)
  To: Magnus Karlsson, Björn Töpel, netdev
  Cc: Andrew Rybchenko, bpf, linux-kernel, Björn Töpel,
	Maciej Fijalkowski, Jonathan Lemon, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
	Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
	Maxim Mikityanskiy

When a XSK pool gets mapped, xp_check_dma_contiguity() adds bit 0x1
to pages' DMA addresses that go in ascending order and at 4K stride.
The problem is that the bit does not get cleared before doing unmap.
As a result, a lot of warnings from iommu_dma_unmap_page() are seen
suggesting mapping lookup failures at drivers/iommu/dma-iommu.c:848.

Fixes: 2b43470add8c ("xsk: Introduce AF_XDP buffer allocation API")
Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
---
 v1 -> v2: minor adjustments to dispose of the "Fixes:" tag warning

 net/xdp/xsk_buff_pool.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 87bdd71c7bb6..f70112176b7c 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -332,6 +332,7 @@ static void __xp_dma_unmap(struct xsk_dma_map *dma_map, unsigned long attrs)
 	for (i = 0; i < dma_map->dma_pages_cnt; i++) {
 		dma = &dma_map->dma_pages[i];
 		if (*dma) {
+			*dma &= ~XSK_NEXT_PG_CONTIG_MASK;
 			dma_unmap_page_attrs(dma_map->dev, *dma, PAGE_SIZE,
 					     DMA_BIDIRECTIONAL, attrs);
 			*dma = 0;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] xsk: clear page contiguity bit when unmapping pool
  2022-06-28  0:17 ` [PATCH v2 1/1] " Ivan Malov
@ 2022-06-28  7:47   ` Magnus Karlsson
  0 siblings, 0 replies; 6+ messages in thread
From: Magnus Karlsson @ 2022-06-28  7:47 UTC (permalink / raw)
  To: Ivan Malov
  Cc: Magnus Karlsson, Björn Töpel, Network Development,
	Andrew Rybchenko, bpf, open list, Björn Töpel,
	Maciej Fijalkowski, Jonathan Lemon, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
	Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
	Maxim Mikityanskiy

On Tue, Jun 28, 2022 at 2:18 AM Ivan Malov <ivan.malov@oktetlabs.ru> wrote:
>
> When a XSK pool gets mapped, xp_check_dma_contiguity() adds bit 0x1
> to pages' DMA addresses that go in ascending order and at 4K stride.
> The problem is that the bit does not get cleared before doing unmap.
> As a result, a lot of warnings from iommu_dma_unmap_page() are seen
> suggesting mapping lookup failures at drivers/iommu/dma-iommu.c:848.

Thanks Ivan for spotting this. Please add if this patch is for bpf or
net in your subject line. E.g., [PATCH net].

Also, I cannot find a warning at drivers/iommu/dma-iommu.c:848. For
net and bpf I have a WARN() at line 679 in __iommu_dma_unmap(). Maybe
it would be better to just refer to __iommu_dma_unmap() and the
warning in that function. Line numbers tend to change.

> Fixes: 2b43470add8c ("xsk: Introduce AF_XDP buffer allocation API")
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> ---
>  v1 -> v2: minor adjustments to dispose of the "Fixes:" tag warning
>
>  net/xdp/xsk_buff_pool.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 87bdd71c7bb6..f70112176b7c 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -332,6 +332,7 @@ static void __xp_dma_unmap(struct xsk_dma_map *dma_map, unsigned long attrs)
>         for (i = 0; i < dma_map->dma_pages_cnt; i++) {
>                 dma = &dma_map->dma_pages[i];
>                 if (*dma) {
> +                       *dma &= ~XSK_NEXT_PG_CONTIG_MASK;
>                         dma_unmap_page_attrs(dma_map->dev, *dma, PAGE_SIZE,
>                                              DMA_BIDIRECTIONAL, attrs);
>                         *dma = 0;
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH net v3 1/1] xsk: clear page contiguity bit when unmapping pool
  2022-06-27 19:01 [PATCH] xsk: clear page contiguity bit when unmapping pool Ivan Malov
  2022-06-28  0:17 ` [PATCH v2 1/1] " Ivan Malov
@ 2022-06-28  9:18 ` Ivan Malov
  2022-06-28  9:29   ` Magnus Karlsson
  2022-06-28 21:00   ` patchwork-bot+netdevbpf
  1 sibling, 2 replies; 6+ messages in thread
From: Ivan Malov @ 2022-06-28  9:18 UTC (permalink / raw)
  To: Magnus Karlsson, Björn Töpel, netdev
  Cc: Andrew Rybchenko, bpf, linux-kernel, Björn Töpel,
	Maciej Fijalkowski, Jonathan Lemon, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
	Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
	Maxim Mikityanskiy

When a XSK pool gets mapped, xp_check_dma_contiguity() adds bit 0x1
to pages' DMA addresses that go in ascending order and at 4K stride.
The problem is that the bit does not get cleared before doing unmap.
As a result, a lot of warnings from iommu_dma_unmap_page() are seen
in dmesg, which indicates that lookups by iommu_iova_to_phys() fail.

Fixes: 2b43470add8c ("xsk: Introduce AF_XDP buffer allocation API")
Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
---
 v1 -> v2: minor adjustments to dispose of the "Fixes:" tag warning
 v2 -> v3: extra refinements to apply notes from Magnus Karlsson

 net/xdp/xsk_buff_pool.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 87bdd71c7bb6..f70112176b7c 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -332,6 +332,7 @@ static void __xp_dma_unmap(struct xsk_dma_map *dma_map, unsigned long attrs)
 	for (i = 0; i < dma_map->dma_pages_cnt; i++) {
 		dma = &dma_map->dma_pages[i];
 		if (*dma) {
+			*dma &= ~XSK_NEXT_PG_CONTIG_MASK;
 			dma_unmap_page_attrs(dma_map->dev, *dma, PAGE_SIZE,
 					     DMA_BIDIRECTIONAL, attrs);
 			*dma = 0;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH net v3 1/1] xsk: clear page contiguity bit when unmapping pool
  2022-06-28  9:18 ` [PATCH net v3 " Ivan Malov
@ 2022-06-28  9:29   ` Magnus Karlsson
  2022-06-28 21:00   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 6+ messages in thread
From: Magnus Karlsson @ 2022-06-28  9:29 UTC (permalink / raw)
  To: Ivan Malov
  Cc: Magnus Karlsson, Björn Töpel, Network Development,
	Andrew Rybchenko, bpf, open list, Björn Töpel,
	Maciej Fijalkowski, Jonathan Lemon, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
	Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
	Maxim Mikityanskiy

On Tue, Jun 28, 2022 at 11:25 AM Ivan Malov <ivan.malov@oktetlabs.ru> wrote:
>
> When a XSK pool gets mapped, xp_check_dma_contiguity() adds bit 0x1
> to pages' DMA addresses that go in ascending order and at 4K stride.
> The problem is that the bit does not get cleared before doing unmap.
> As a result, a lot of warnings from iommu_dma_unmap_page() are seen
> in dmesg, which indicates that lookups by iommu_iova_to_phys() fail.
>
> Fixes: 2b43470add8c ("xsk: Introduce AF_XDP buffer allocation API")
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>

Thanks Ivan.

Acked-by: Magnus Karlsson <magnus.karlsson@intel.com>

> ---
>  v1 -> v2: minor adjustments to dispose of the "Fixes:" tag warning
>  v2 -> v3: extra refinements to apply notes from Magnus Karlsson
>
>  net/xdp/xsk_buff_pool.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 87bdd71c7bb6..f70112176b7c 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -332,6 +332,7 @@ static void __xp_dma_unmap(struct xsk_dma_map *dma_map, unsigned long attrs)
>         for (i = 0; i < dma_map->dma_pages_cnt; i++) {
>                 dma = &dma_map->dma_pages[i];
>                 if (*dma) {
> +                       *dma &= ~XSK_NEXT_PG_CONTIG_MASK;
>                         dma_unmap_page_attrs(dma_map->dev, *dma, PAGE_SIZE,
>                                              DMA_BIDIRECTIONAL, attrs);
>                         *dma = 0;
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH net v3 1/1] xsk: clear page contiguity bit when unmapping pool
  2022-06-28  9:18 ` [PATCH net v3 " Ivan Malov
  2022-06-28  9:29   ` Magnus Karlsson
@ 2022-06-28 21:00   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-06-28 21:00 UTC (permalink / raw)
  To: Ivan Malov
  Cc: magnus.karlsson, bjorn.topel, netdev, andrew.rybchenko, bpf,
	linux-kernel, bjorn, maciej.fijalkowski, jonathan.lemon, davem,
	edumazet, kuba, pabeni, ast, daniel, hawk, john.fastabend,
	maximmi

Hello:

This patch was applied to bpf/bpf.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Tue, 28 Jun 2022 12:18:48 +0300 you wrote:
> When a XSK pool gets mapped, xp_check_dma_contiguity() adds bit 0x1
> to pages' DMA addresses that go in ascending order and at 4K stride.
> The problem is that the bit does not get cleared before doing unmap.
> As a result, a lot of warnings from iommu_dma_unmap_page() are seen
> in dmesg, which indicates that lookups by iommu_iova_to_phys() fail.
> 
> Fixes: 2b43470add8c ("xsk: Introduce AF_XDP buffer allocation API")
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> 
> [...]

Here is the summary with links:
  - [net,v3,1/1] xsk: clear page contiguity bit when unmapping pool
    https://git.kernel.org/bpf/bpf/c/512d1999b8e9

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



^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-06-28 21:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-27 19:01 [PATCH] xsk: clear page contiguity bit when unmapping pool Ivan Malov
2022-06-28  0:17 ` [PATCH v2 1/1] " Ivan Malov
2022-06-28  7:47   ` Magnus Karlsson
2022-06-28  9:18 ` [PATCH net v3 " Ivan Malov
2022-06-28  9:29   ` Magnus Karlsson
2022-06-28 21:00   ` patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.