DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] eal/memory: fix unused SIGBUS handler
@ 2021-10-29  9:53 Olivier Matz
  2021-10-29 10:09 ` Maxime Coquelin
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Olivier Matz @ 2021-10-29  9:53 UTC (permalink / raw)
  To: dev; +Cc: Anatoly Burakov, Maxime Coquelin, David Marchand, stable

Since its introduction in 2018, the SIGBUS handler was never registered,
and all related functions were unused.

A SIGBUS can be received by the application when accessing to hugepages
even if mmap() was successful, This happens especially when running
inside containers when there is not enough hugepages. In this case, we
need to recover. A similar scheme can be found in eal_memory.c.

Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
Cc: stable@dpdk.org

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/eal/linux/eal_memalloc.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
index 0ec8542283..337f2bc739 100644
--- a/lib/eal/linux/eal_memalloc.c
+++ b/lib/eal/linux/eal_memalloc.c
@@ -107,7 +107,7 @@ static struct rte_memseg_list local_memsegs[RTE_MAX_MEMSEG_LISTS];
 
 static sigjmp_buf huge_jmpenv;
 
-static void __rte_unused huge_sigbus_handler(int signo __rte_unused)
+static void huge_sigbus_handler(int signo __rte_unused)
 {
 	siglongjmp(huge_jmpenv, 1);
 }
@@ -116,7 +116,7 @@ static void __rte_unused huge_sigbus_handler(int signo __rte_unused)
  * non-static local variable in the stack frame calling sigsetjmp might be
  * clobbered by a call to longjmp.
  */
-static int __rte_unused huge_wrap_sigsetjmp(void)
+static int huge_wrap_sigsetjmp(void)
 {
 	return sigsetjmp(huge_jmpenv, 1);
 }
@@ -124,7 +124,7 @@ static int __rte_unused huge_wrap_sigsetjmp(void)
 static struct sigaction huge_action_old;
 static int huge_need_recover;
 
-static void __rte_unused
+static void
 huge_register_sigbus(void)
 {
 	sigset_t mask;
@@ -139,7 +139,7 @@ huge_register_sigbus(void)
 	huge_need_recover = !sigaction(SIGBUS, &action, &huge_action_old);
 }
 
-static void __rte_unused
+static void
 huge_recover_sigbus(void)
 {
 	if (huge_need_recover) {
@@ -576,6 +576,8 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
 		mmap_flags = MAP_SHARED | MAP_POPULATE | MAP_FIXED;
 	}
 
+	huge_register_sigbus();
+
 	/*
 	 * map the segment, and populate page tables, the kernel fills
 	 * this segment with zeros if it's a new page.
@@ -651,6 +653,8 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
 				__func__);
 #endif
 
+	huge_recover_sigbus();
+
 	ms->addr = addr;
 	ms->hugepage_sz = alloc_sz;
 	ms->len = alloc_sz;
@@ -664,6 +668,7 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
 mapped:
 	munmap(addr, alloc_sz);
 unmapped:
+	huge_recover_sigbus();
 	flags = EAL_RESERVE_FORCE_ADDRESS;
 	new_addr = eal_get_virtual_area(addr, &alloc_sz, alloc_sz, 0, flags);
 	if (new_addr != addr) {
-- 
2.30.2


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

* Re: [dpdk-dev] [PATCH] eal/memory: fix unused SIGBUS handler
  2021-10-29  9:53 [dpdk-dev] [PATCH] eal/memory: fix unused SIGBUS handler Olivier Matz
@ 2021-10-29 10:09 ` Maxime Coquelin
  2021-11-03 20:03 ` David Marchand
  2021-11-05 14:27 ` David Marchand
  2 siblings, 0 replies; 5+ messages in thread
From: Maxime Coquelin @ 2021-10-29 10:09 UTC (permalink / raw)
  To: Olivier Matz, dev; +Cc: Anatoly Burakov, David Marchand, stable

Hi Olivier,

On 10/29/21 11:53, Olivier Matz wrote:
> Since its introduction in 2018, the SIGBUS handler was never registered,
> and all related functions were unused.
> 
> A SIGBUS can be received by the application when accessing to hugepages
> even if mmap() was successful, This happens especially when running
> inside containers when there is not enough hugepages. In this case, we
> need to recover. A similar scheme can be found in eal_memory.c.
> 
> Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>   lib/eal/linux/eal_memalloc.c | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
> index 0ec8542283..337f2bc739 100644
> --- a/lib/eal/linux/eal_memalloc.c
> +++ b/lib/eal/linux/eal_memalloc.c
> @@ -107,7 +107,7 @@ static struct rte_memseg_list local_memsegs[RTE_MAX_MEMSEG_LISTS];
>   
>   static sigjmp_buf huge_jmpenv;
>   
> -static void __rte_unused huge_sigbus_handler(int signo __rte_unused)
> +static void huge_sigbus_handler(int signo __rte_unused)
>   {
>   	siglongjmp(huge_jmpenv, 1);
>   }
> @@ -116,7 +116,7 @@ static void __rte_unused huge_sigbus_handler(int signo __rte_unused)
>    * non-static local variable in the stack frame calling sigsetjmp might be
>    * clobbered by a call to longjmp.
>    */
> -static int __rte_unused huge_wrap_sigsetjmp(void)
> +static int huge_wrap_sigsetjmp(void)
>   {
>   	return sigsetjmp(huge_jmpenv, 1);
>   }
> @@ -124,7 +124,7 @@ static int __rte_unused huge_wrap_sigsetjmp(void)
>   static struct sigaction huge_action_old;
>   static int huge_need_recover;
>   
> -static void __rte_unused
> +static void
>   huge_register_sigbus(void)
>   {
>   	sigset_t mask;
> @@ -139,7 +139,7 @@ huge_register_sigbus(void)
>   	huge_need_recover = !sigaction(SIGBUS, &action, &huge_action_old);
>   }
>   
> -static void __rte_unused
> +static void
>   huge_recover_sigbus(void)
>   {
>   	if (huge_need_recover) {
> @@ -576,6 +576,8 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
>   		mmap_flags = MAP_SHARED | MAP_POPULATE | MAP_FIXED;
>   	}
>   
> +	huge_register_sigbus();
> +
>   	/*
>   	 * map the segment, and populate page tables, the kernel fills
>   	 * this segment with zeros if it's a new page.
> @@ -651,6 +653,8 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
>   				__func__);
>   #endif
>   
> +	huge_recover_sigbus();
> +
>   	ms->addr = addr;
>   	ms->hugepage_sz = alloc_sz;
>   	ms->len = alloc_sz;
> @@ -664,6 +668,7 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
>   mapped:
>   	munmap(addr, alloc_sz);
>   unmapped:
> +	huge_recover_sigbus();
>   	flags = EAL_RESERVE_FORCE_ADDRESS;
>   	new_addr = eal_get_virtual_area(addr, &alloc_sz, alloc_sz, 0, flags);
>   	if (new_addr != addr) {
>

I had almost the same series ready, instead that I installed/removed the
handler before/after the loop of alloc_seg(). I don't think this is
necessary though, so I'm fine with your patch, which is simpler:

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime




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

* Re: [dpdk-dev] [PATCH] eal/memory: fix unused SIGBUS handler
  2021-10-29  9:53 [dpdk-dev] [PATCH] eal/memory: fix unused SIGBUS handler Olivier Matz
  2021-10-29 10:09 ` Maxime Coquelin
@ 2021-11-03 20:03 ` David Marchand
  2021-11-04  8:39   ` Olivier Matz
  2021-11-05 14:27 ` David Marchand
  2 siblings, 1 reply; 5+ messages in thread
From: David Marchand @ 2021-11-03 20:03 UTC (permalink / raw)
  To: Olivier Matz, Anatoly Burakov; +Cc: dev, Maxime Coquelin, dpdk stable

On Fri, Oct 29, 2021 at 11:53 AM Olivier Matz <olivier.matz@6wind.com> wrote:
>
> Since its introduction in 2018, the SIGBUS handler was never registered,
> and all related functions were unused.
>
> A SIGBUS can be received by the application when accessing to hugepages
> even if mmap() was successful, This happens especially when running
> inside containers when there is not enough hugepages. In this case, we
> need to recover. A similar scheme can be found in eal_memory.c.
>
> Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
> Cc: stable@dpdk.org
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

This patch lgtm.

The key point here is that in the "container context" (well, cgroups)
mmap succeeds regardless of availability of hugepages.
I would put an emphasis about this in the title.

What do you think of:
mem: fix dynamic hugepage mapping in container


-- 
David Marchand


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

* Re: [dpdk-dev] [PATCH] eal/memory: fix unused SIGBUS handler
  2021-11-03 20:03 ` David Marchand
@ 2021-11-04  8:39   ` Olivier Matz
  0 siblings, 0 replies; 5+ messages in thread
From: Olivier Matz @ 2021-11-04  8:39 UTC (permalink / raw)
  To: David Marchand; +Cc: Anatoly Burakov, dev, Maxime Coquelin, dpdk stable

On Wed, Nov 03, 2021 at 09:03:19PM +0100, David Marchand wrote:
> On Fri, Oct 29, 2021 at 11:53 AM Olivier Matz <olivier.matz@6wind.com> wrote:
> >
> > Since its introduction in 2018, the SIGBUS handler was never registered,
> > and all related functions were unused.
> >
> > A SIGBUS can be received by the application when accessing to hugepages
> > even if mmap() was successful, This happens especially when running
> > inside containers when there is not enough hugepages. In this case, we
> > need to recover. A similar scheme can be found in eal_memory.c.
> >
> > Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> 
> This patch lgtm.
> 
> The key point here is that in the "container context" (well, cgroups)
> mmap succeeds regardless of availability of hugepages.
> I would put an emphasis about this in the title.
> 
> What do you think of:
> mem: fix dynamic hugepage mapping in container

Yes it's a better title.

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

* Re: [dpdk-dev] [PATCH] eal/memory: fix unused SIGBUS handler
  2021-10-29  9:53 [dpdk-dev] [PATCH] eal/memory: fix unused SIGBUS handler Olivier Matz
  2021-10-29 10:09 ` Maxime Coquelin
  2021-11-03 20:03 ` David Marchand
@ 2021-11-05 14:27 ` David Marchand
  2 siblings, 0 replies; 5+ messages in thread
From: David Marchand @ 2021-11-05 14:27 UTC (permalink / raw)
  To: Olivier Matz; +Cc: dev, Anatoly Burakov, Maxime Coquelin, dpdk stable

On Fri, Oct 29, 2021 at 11:53 AM Olivier Matz <olivier.matz@6wind.com> wrote:
>
> Since its introduction in 2018, the SIGBUS handler was never registered,
> and all related functions were unused.
>
> A SIGBUS can be received by the application when accessing to hugepages
> even if mmap() was successful, This happens especially when running
> inside containers when there is not enough hugepages. In this case, we
> need to recover. A similar scheme can be found in eal_memory.c.
>
> Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
> Cc: stable@dpdk.org
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Acked-by: David Marchand <david.marchand@redhat.com>

Applied, thanks.


-- 
David Marchand


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

end of thread, other threads:[~2021-11-05 14:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-29  9:53 [dpdk-dev] [PATCH] eal/memory: fix unused SIGBUS handler Olivier Matz
2021-10-29 10:09 ` Maxime Coquelin
2021-11-03 20:03 ` David Marchand
2021-11-04  8:39   ` Olivier Matz
2021-11-05 14:27 ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).