Re: [linux-yocto][linux-yocto v5.10][PATCH] eventfd: Enlarge recursion limit to allow vhost to work

2021-07-03 Thread Bruce Ashfield
On Thu, Jul 1, 2021 at 9:38 PM Zhang, Qiang  wrote:
>
>
>
> 
> From: Bruce Ashfield 
> Sent: Thursday, 1 July 2021 08:25
> To: Zhang, Qiang
> Cc: linux-yocto@lists.yoctoproject.org
> Subject: Re: [linux-yocto][linux-yocto v5.10][PATCH] eventfd: Enlarge 
> recursion limit to allow vhost to work
>
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> In message: [linux-yocto][linux-yocto v5.10][PATCH] eventfd: Enlarge 
> recursion limit to allow vhost to work
> on 28/06/2021 qiang.zh...@windriver.com wrote:
>
> > From: Zqiang 
> >
> > 1/1 [
> > Author: He Zhe
> > Email: zhe...@windriver.com
> > Subject: eventfd: Enlarge recursion limit to allow vhost to work
> > Date: Fri, 5 Jun 2020 16:32:18 +0800
> >
> > commit 85f0a97f3aac6bb2c9549af607843644dd2ef5c7 upstream [linux-yocto]
> >
> > Upstream link: 
> > https://lore.kernel.org/lkml/20200410114720.24838-1-zhe...@windriver.com/
>
> >I was reading the thread on lore, and it isn't clear to me what is
> >the latest status.
> >
> >Does this problem show more easily under preempt-rt, but is not a
> >preempt-rt only fix ? I'm just trying to confirm that you are
> >targeting this at v5.10/standard/base.
>
> Hello Bruce
>
> Sorry is my mistake
> this patch  not only for v5.10, but also for v5.13
>
> This problem occurs not only in preempt-rt kernel, but also in non preempt-rt 
> kernel.
> in preempt-rt kernel, due to  spinlock is replaced by rt-mutex, the spinlock 
> critical area becomes preemptive, If preemption occurs after increasing the 
> current reference count of CPU, the new task accesses the current CPU again 
> will trigger warning,
> in non preempt-rt kernel, if the eventfd_signal() is called recursively also 
> trigger warning

Thanks for the update!

I've merged it to the v5.10 and v5.13 standard/* branches.

Bruce

>
> Thanks
> Qiang
> >
> >You first submitted this a year ago, and it still isn't applied
> >upstream. And Juri was seeing the same warning. There was a
> >recent series posted that had a similar change to your v1, but
> >you've resent this patch (v2?) as part of that effort .. but still
> >no answer on the change.
> >
> >Also note, I just pushed 5.13 as the start of the new release
> >kernel, since this is in mainline still, I assume I can apply this
> >to those branches as well.
> >
> >Bruce
>
> >
> > commit b5e683d5cab8 ("eventfd: track eventfd_signal() recursion depth")
> > introduces a percpu counter that tracks the percpu recursion depth and
> > warn if it greater than zero, to avoid potential deadlock and stack
> > overflow.
> >
> > However sometimes different eventfds may be used in parallel. Specifically,
> > when heavy network load goes through kvm and vhost, working as below, it
> > would trigger the following call trace.
> >
> > -  100.00%
> >- 66.51%
> > ret_from_fork
> > kthread
> >   - vhost_worker
> >  - 33.47% handle_tx_kick
> >   handle_tx
> >   handle_tx_copy
> >   vhost_tx_batch.isra.0
> >   vhost_add_used_and_signal_n
> >   eventfd_signal
> >  - 33.05% handle_rx_net
> >   handle_rx
> >   vhost_add_used_and_signal_n
> >   eventfd_signal
> >- 33.49%
> > ioctl
> > entry_SYSCALL_64_after_hwframe
> > do_syscall_64
> > __x64_sys_ioctl
> > ksys_ioctl
> > do_vfs_ioctl
> > kvm_vcpu_ioctl
> > kvm_arch_vcpu_ioctl_run
> > vmx_handle_exit
> > handle_ept_misconfig
> > kvm_io_bus_write
> > __kvm_io_bus_write
> > eventfd_signal
> > 001: WARNING: CPU: 1 PID: 1503 at fs/eventfd.c:73 eventfd_signal+0x85/0xa0
> >  snip 
> > 001: Call Trace:
> > 001:  vhost_signal+0x15e/0x1b0 [vhost]
> > 001:  vhost_add_used_and_signal_n+0x2b/0x40 [vhost]
> > 001:  handle_rx+0xb9/0x900 [vhost_net]
> > 001:  handle_rx_net+0x15/0x20 [vhost_net]
> > 001:  vhost_worker+0xbe/0x120 [vhost]
> > 001:  kthread+0x106/0x140
> > 001:  ? log_used.part.0+0x20/0x20 [vhost]
> > 001:  ? kthread_park+0x90/0x90
> > 001:  ret_from_fork+0x35/0x40
> > 001: ---[ end trace 0003 ]---
> >
> > This patch enlarges the limit to 1 which is the maximum recursion depth we
> > have found so far.
> >
> > Signed-off-

Re: [linux-yocto][linux-yocto v5.10][PATCH] eventfd: Enlarge recursion limit to allow vhost to work

2021-06-30 Thread Bruce Ashfield

In message: [linux-yocto][linux-yocto v5.10][PATCH] eventfd: Enlarge recursion 
limit to allow vhost to work
on 28/06/2021 qiang.zh...@windriver.com wrote:

> From: Zqiang 
> 
> 1/1 [
> Author: He Zhe
> Email: zhe...@windriver.com
> Subject: eventfd: Enlarge recursion limit to allow vhost to work
> Date: Fri, 5 Jun 2020 16:32:18 +0800
> 
> commit 85f0a97f3aac6bb2c9549af607843644dd2ef5c7 upstream [linux-yocto]
> 
> Upstream link: 
> https://lore.kernel.org/lkml/20200410114720.24838-1-zhe...@windriver.com/

I was reading the thread on lore, and it isn't clear to me what is
the latest status.

Does this problem show more easily under preempt-rt, but is not a
preempt-rt only fix ? I'm just trying to confirm that you are
targeting this at v5.10/standard/base.

You first submitted this a year ago, and it still isn't applied
upstream. And Juri was seeing the same warning. There was a
recent series posted that had a similar change to your v1, but
you've resent this patch (v2?) as part of that effort .. but still
no answer on the change.

Also note, I just pushed 5.13 as the start of the new release
kernel, since this is in mainline still, I assume I can apply this
to those branches as well.

Bruce

> 
> commit b5e683d5cab8 ("eventfd: track eventfd_signal() recursion depth")
> introduces a percpu counter that tracks the percpu recursion depth and
> warn if it greater than zero, to avoid potential deadlock and stack
> overflow.
> 
> However sometimes different eventfds may be used in parallel. Specifically,
> when heavy network load goes through kvm and vhost, working as below, it
> would trigger the following call trace.
> 
> -  100.00%
>- 66.51%
> ret_from_fork
> kthread
>   - vhost_worker
>  - 33.47% handle_tx_kick
>   handle_tx
>   handle_tx_copy
>   vhost_tx_batch.isra.0
>   vhost_add_used_and_signal_n
>   eventfd_signal
>  - 33.05% handle_rx_net
>   handle_rx
>   vhost_add_used_and_signal_n
>   eventfd_signal
>- 33.49%
> ioctl
> entry_SYSCALL_64_after_hwframe
> do_syscall_64
> __x64_sys_ioctl
> ksys_ioctl
> do_vfs_ioctl
> kvm_vcpu_ioctl
> kvm_arch_vcpu_ioctl_run
> vmx_handle_exit
> handle_ept_misconfig
> kvm_io_bus_write
> __kvm_io_bus_write
> eventfd_signal
> 001: WARNING: CPU: 1 PID: 1503 at fs/eventfd.c:73 eventfd_signal+0x85/0xa0
>  snip 
> 001: Call Trace:
> 001:  vhost_signal+0x15e/0x1b0 [vhost]
> 001:  vhost_add_used_and_signal_n+0x2b/0x40 [vhost]
> 001:  handle_rx+0xb9/0x900 [vhost_net]
> 001:  handle_rx_net+0x15/0x20 [vhost_net]
> 001:  vhost_worker+0xbe/0x120 [vhost]
> 001:  kthread+0x106/0x140
> 001:  ? log_used.part.0+0x20/0x20 [vhost]
> 001:  ? kthread_park+0x90/0x90
> 001:  ret_from_fork+0x35/0x40
> 001: ---[ end trace 0003 ]---
> 
> This patch enlarges the limit to 1 which is the maximum recursion depth we
> have found so far.
> 
> Signed-off-by: He Zhe 
> Signed-off-by: Bruce Ashfield 
> [CE: backport of 85f0a97f3aac from v5.4 branch of linux-yocto]
> Signed-off-by: Catalin Enache 
> ]
> 
> Signed-off-by: Zqiang 
> ---
>  fs/eventfd.c| 2 +-
>  include/linux/eventfd.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/eventfd.c b/fs/eventfd.c
> index df466ef81ddd..7a5cbec9e843 100644
> --- a/fs/eventfd.c
> +++ b/fs/eventfd.c
> @@ -71,7 +71,7 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 n)
>* it returns true, the eventfd_signal() call should be deferred to a
>* safe context.
>*/
> - if (WARN_ON_ONCE(this_cpu_read(eventfd_wake_count)))
> + if (WARN_ON_ONCE(this_cpu_read(eventfd_wake_count) > 
> EFD_WAKE_COUNT_MAX))
>   return 0;
>  
>   spin_lock_irqsave(>wqh.lock, flags);
> diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
> index dc4fd8a6644d..db050fb99e0b 100644
> --- a/include/linux/eventfd.h
> +++ b/include/linux/eventfd.h
> @@ -29,6 +29,8 @@
>  #define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
>  #define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
>  
> +/* This is the maximum recursion depth we find so far */
> +#define EFD_WAKE_COUNT_MAX  1
>  struct eventfd_ctx;
>  struct file;
>  
> -- 
> 2.29.2
> 

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#10018): 
https://lists.yoctoproject.org/g/linux-yocto/message/10018
Mute This Topic: https://lists.yoctoproject.org/mt/83840069/21656
Group Owner: linux-yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[linux-yocto][linux-yocto v5.10][PATCH] eventfd: Enlarge recursion limit to allow vhost to work

2021-06-28 Thread Zhang, Qiang
From: Zqiang 

1/1 [
Author: He Zhe
Email: zhe...@windriver.com
Subject: eventfd: Enlarge recursion limit to allow vhost to work
Date: Fri, 5 Jun 2020 16:32:18 +0800

commit 85f0a97f3aac6bb2c9549af607843644dd2ef5c7 upstream [linux-yocto]

Upstream link: 
https://lore.kernel.org/lkml/20200410114720.24838-1-zhe...@windriver.com/

commit b5e683d5cab8 ("eventfd: track eventfd_signal() recursion depth")
introduces a percpu counter that tracks the percpu recursion depth and
warn if it greater than zero, to avoid potential deadlock and stack
overflow.

However sometimes different eventfds may be used in parallel. Specifically,
when heavy network load goes through kvm and vhost, working as below, it
would trigger the following call trace.

-  100.00%
   - 66.51%
ret_from_fork
kthread
  - vhost_worker
 - 33.47% handle_tx_kick
  handle_tx
  handle_tx_copy
  vhost_tx_batch.isra.0
  vhost_add_used_and_signal_n
  eventfd_signal
 - 33.05% handle_rx_net
  handle_rx
  vhost_add_used_and_signal_n
  eventfd_signal
   - 33.49%
ioctl
entry_SYSCALL_64_after_hwframe
do_syscall_64
__x64_sys_ioctl
ksys_ioctl
do_vfs_ioctl
kvm_vcpu_ioctl
kvm_arch_vcpu_ioctl_run
vmx_handle_exit
handle_ept_misconfig
kvm_io_bus_write
__kvm_io_bus_write
eventfd_signal
001: WARNING: CPU: 1 PID: 1503 at fs/eventfd.c:73 eventfd_signal+0x85/0xa0
 snip 
001: Call Trace:
001:  vhost_signal+0x15e/0x1b0 [vhost]
001:  vhost_add_used_and_signal_n+0x2b/0x40 [vhost]
001:  handle_rx+0xb9/0x900 [vhost_net]
001:  handle_rx_net+0x15/0x20 [vhost_net]
001:  vhost_worker+0xbe/0x120 [vhost]
001:  kthread+0x106/0x140
001:  ? log_used.part.0+0x20/0x20 [vhost]
001:  ? kthread_park+0x90/0x90
001:  ret_from_fork+0x35/0x40
001: ---[ end trace 0003 ]---

This patch enlarges the limit to 1 which is the maximum recursion depth we
have found so far.

Signed-off-by: He Zhe 
Signed-off-by: Bruce Ashfield 
[CE: backport of 85f0a97f3aac from v5.4 branch of linux-yocto]
Signed-off-by: Catalin Enache 
]

Signed-off-by: Zqiang 
---
 fs/eventfd.c| 2 +-
 include/linux/eventfd.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/eventfd.c b/fs/eventfd.c
index df466ef81ddd..7a5cbec9e843 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -71,7 +71,7 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 n)
 * it returns true, the eventfd_signal() call should be deferred to a
 * safe context.
 */
-   if (WARN_ON_ONCE(this_cpu_read(eventfd_wake_count)))
+   if (WARN_ON_ONCE(this_cpu_read(eventfd_wake_count) > 
EFD_WAKE_COUNT_MAX))
return 0;
 
spin_lock_irqsave(>wqh.lock, flags);
diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
index dc4fd8a6644d..db050fb99e0b 100644
--- a/include/linux/eventfd.h
+++ b/include/linux/eventfd.h
@@ -29,6 +29,8 @@
 #define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
 #define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
 
+/* This is the maximum recursion depth we find so far */
+#define EFD_WAKE_COUNT_MAX  1
 struct eventfd_ctx;
 struct file;
 
-- 
2.29.2


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#10009): 
https://lists.yoctoproject.org/g/linux-yocto/message/10009
Mute This Topic: https://lists.yoctoproject.org/mt/83840069/21656
Group Owner: linux-yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-