Re: [PATCH net-next v2 4/4] selftests/xsk: Introduce mmap_size in umem struct

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Wed, Jun 03, 2026 at 11:33:27AM +0530, Tushar Vyavahare wrote:
> UMEM teardown currently recomputes the munmap() length from frame
> geometry, shared-UMEM adjustment, and hugepage rounding. This duplicates
> setup-time logic in cleanup and relies on re-deriving the mapping size
> instead of using the size originally established for the mapping.
> 
> Store the final mapping length in xsk_umem_info as mmap_size when the
> UMEM mapping is created, and use that value during teardown.
> 
> Also join the RX worker thread before cleanup in the single-thread
> path. This establishes synchronization before reading umem->mmap_size
> in teardown and avoids a potential visibility race.
> 
> This removes duplicated size arithmetic in cleanup and makes munmap()
> use the canonical mapping size recorded at setup time.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

> ---
>  .../selftests/bpf/prog_tests/test_xsk.c       | 20 +++++++++----------
>  .../selftests/bpf/prog_tests/test_xsk.h       |  1 +
>  2 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> index f1730466ffd9..e94c38a1faf4 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> @@ -1581,7 +1581,7 @@ static int thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
>  	struct xsk_umem_info *umem = ifobject->xsk->umem;
>  	LIBBPF_OPTS(bpf_xdp_query_opts, opts);
>  	int mmap_flags;
> -	u64 umem_sz;
> +	u64 umem_sz, mmap_sz;
>  	void *bufs;
>  	int ret;
>  	u32 i;
> @@ -1595,10 +1595,15 @@ static int thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
>  	if (ifobject->shared_umem)
>  		umem_sz *= 2;
>  
> -	bufs = mmap(NULL, umem_sz, PROT_READ | PROT_WRITE, mmap_flags, -1, 0);
> +	mmap_sz = umem->unaligned_mode ?
> +		ceil_u64(umem_sz, HUGEPAGE_SIZE) * HUGEPAGE_SIZE : umem_sz;
> +
> +	bufs = mmap(NULL, mmap_sz, PROT_READ | PROT_WRITE, mmap_flags, -1, 0);
>  	if (bufs == MAP_FAILED)
>  		return -errno;
>  
> +	umem->mmap_size = mmap_sz;
> +
>  	ret = xsk_configure_umem(ifobject, umem, bufs, umem_sz);
>  	if (ret)
>  		return ret;
> @@ -1706,15 +1711,9 @@ void *worker_testapp_validate_rx(void *arg)
>  static void testapp_clean_xsk_umem(struct ifobject *ifobj)
>  {
>  	struct xsk_umem_info *umem = ifobj->xsk->umem;
> -	u64 umem_sz = umem_size(umem);
> -
> -	if (ifobj->shared_umem)
> -		umem_sz *= 2;
> -
> -	umem_sz = ceil_u64(umem_sz, HUGEPAGE_SIZE) * HUGEPAGE_SIZE;
>  
>  	xsk_umem__delete(umem->umem);
> -	munmap(umem->buffer, umem_sz);
> +	munmap(umem->buffer, umem->mmap_size);
>  }
>  
>  static void handler(int signum)
> @@ -1857,8 +1856,7 @@ static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *i
>  
>  	if (!ifobj2)
>  		pthread_kill(t0, SIGUSR1);
> -	else
> -		pthread_join(t0, NULL);
> +	pthread_join(t0, NULL);
>  
>  	if (test->total_steps == test->current_step || test->fail) {
>  		clean_sockets(test, ifobj1);
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.h b/tools/testing/selftests/bpf/prog_tests/test_xsk.h
> index 99003995d7c3..4313d0d87235 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.h
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.h
> @@ -103,6 +103,7 @@ struct xsk_umem_info {
>  	struct xsk_ring_cons cq;
>  	struct xsk_umem *umem;
>  	u64 next_buffer;
> +	u64 mmap_size;
>  	u32 num_frames;
>  	u32 frame_headroom;
>  	void *buffer;
> -- 
> 2.43.0
>