[PATCH] eventfd: Enlarge recursion limit to allow vhost to work

2021-06-18 Thread He Zhe
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.

The credit of modification for eventfd_signal_count goes to
Xie Yongji 

Signed-off-by: He Zhe 
---
 fs/eventfd.c| 3 ++-
 include/linux/eventfd.h | 5 -
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/eventfd.c b/fs/eventfd.c
index e265b6dd4f34..add6af91cacf 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -71,7 +71,8 @@ __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 fa0a524baed0..74be152ebe87 100644
--- a/include/linux/eventfd.h
+++ b/include/linux/eventfd.h
@@ -29,6 +29,9 @@
 #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;
 
@@ -47,7 +50,7 @@ DECLARE_PER_CPU(int, eventfd_wake_count);
 
 static inline bool eventfd_signal_count(void)
 {
-   return this_cpu_read(eventfd_wake_count);
+   return this_cpu_read(eventfd_wake_count) > EFD_WAKE_COUNT_MAX;
 }
 
 #else /* CONFIG_EVENTFD */
-- 
2.17.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v8 03/10] eventfd: Increase the recursion depth of eventfd_signal()

2021-06-18 Thread He Zhe



On 6/18/21 11:29 AM, Yongji Xie wrote:
> On Thu, Jun 17, 2021 at 4:34 PM He Zhe  wrote:
>>
>>
>> On 6/15/21 10:13 PM, Xie Yongji wrote:
>>> Increase the recursion depth of eventfd_signal() to 1. This
>>> is the maximum recursion depth we have found so far, which
>>> can be triggered with the following call chain:
>>>
>>> kvm_io_bus_write[kvm]
>>>   --> ioeventfd_write   [kvm]
>>> --> eventfd_signal  [eventfd]
>>>   --> vhost_poll_wakeup [vhost]
>>> --> vduse_vdpa_kick_vq  [vduse]
>>>   --> eventfd_signal[eventfd]
>>>
>>> Signed-off-by: Xie Yongji 
>>> Acked-by: Jason Wang 
>> The fix had been posted one year ago.
>>
>> https://lore.kernel.org/lkml/20200410114720.24838-1-zhe...@windriver.com/
>>
> OK, so it seems to be a fix for the RT system if my understanding is
> correct? Any reason why it's not merged? I'm happy to rebase my series
> on your patch if you'd like to repost it.

It works for both mainline and RT kernel. The folks just reproduced in their RT
environments.

This patch somehow hasn't got maintainer's reply, so not merged yet.

And OK, I'll resend the patch.

>
> BTW, I also notice another thread for this issue:
>
> https://lore.kernel.org/linux-fsdevel/dm6pr11mb420291b550a10853403c7592ff...@dm6pr11mb4202.namprd11.prod.outlook.com/T/

This is the same way as my v1

https://lore.kernel.org/lkml/3b4aa4cb-0e76-89c2-c48a-cf24e1a36...@kernel.dk/

which was not what the maintainer wanted.


>
>>> ---
>>>  fs/eventfd.c| 2 +-
>>>  include/linux/eventfd.h | 5 -
>>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/eventfd.c b/fs/eventfd.c
>>> index e265b6dd4f34..cc7cd1dbedd3 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_DEPTH))
>>>   return 0;
>>>
>>>   spin_lock_irqsave(>wqh.lock, flags);
>>> diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
>>> index fa0a524baed0..886d99cd38ef 100644
>>> --- a/include/linux/eventfd.h
>>> +++ b/include/linux/eventfd.h
>>> @@ -29,6 +29,9 @@
>>>  #define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
>>>  #define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
>>>
>>> +/* Maximum recursion depth */
>>> +#define EFD_WAKE_DEPTH 1
>>> +
>>>  struct eventfd_ctx;
>>>  struct file;
>>>
>>> @@ -47,7 +50,7 @@ DECLARE_PER_CPU(int, eventfd_wake_count);
>>>
>>>  static inline bool eventfd_signal_count(void)
>>>  {
>>> - return this_cpu_read(eventfd_wake_count);
>>> + return this_cpu_read(eventfd_wake_count) > EFD_WAKE_DEPTH;
>> count is just count. How deep is acceptable should be put
>> where eventfd_signal_count is called.
>>
> The return value of this function is boolean rather than integer.
> Please see the comments in eventfd_signal():
>
> "then it should check eventfd_signal_count() before calling this
> function. If it returns true, the eventfd_signal() call should be
> deferred to a safe context."

OK. Now that the maintainer comments as such we can use it accordingly,
though I still got the feeling that the function name and the type of the return
value don't match.


Thanks,
Zhe

>
> Thanks,
> Yongji

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v8 03/10] eventfd: Increase the recursion depth of eventfd_signal()

2021-06-17 Thread He Zhe



On 6/15/21 10:13 PM, Xie Yongji wrote:
> Increase the recursion depth of eventfd_signal() to 1. This
> is the maximum recursion depth we have found so far, which
> can be triggered with the following call chain:
>
> kvm_io_bus_write[kvm]
>   --> ioeventfd_write   [kvm]
> --> eventfd_signal  [eventfd]
>   --> vhost_poll_wakeup [vhost]
> --> vduse_vdpa_kick_vq  [vduse]
>   --> eventfd_signal[eventfd]
>
> Signed-off-by: Xie Yongji 
> Acked-by: Jason Wang 

The fix had been posted one year ago.

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


> ---
>  fs/eventfd.c| 2 +-
>  include/linux/eventfd.h | 5 -
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/fs/eventfd.c b/fs/eventfd.c
> index e265b6dd4f34..cc7cd1dbedd3 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_DEPTH))
>   return 0;
>  
>   spin_lock_irqsave(>wqh.lock, flags);
> diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
> index fa0a524baed0..886d99cd38ef 100644
> --- a/include/linux/eventfd.h
> +++ b/include/linux/eventfd.h
> @@ -29,6 +29,9 @@
>  #define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
>  #define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
>  
> +/* Maximum recursion depth */
> +#define EFD_WAKE_DEPTH 1
> +
>  struct eventfd_ctx;
>  struct file;
>  
> @@ -47,7 +50,7 @@ DECLARE_PER_CPU(int, eventfd_wake_count);
>  
>  static inline bool eventfd_signal_count(void)
>  {
> - return this_cpu_read(eventfd_wake_count);
> + return this_cpu_read(eventfd_wake_count) > EFD_WAKE_DEPTH;

count is just count. How deep is acceptable should be put
where eventfd_signal_count is called.


Zhe

>  }
>  
>  #else /* CONFIG_EVENTFD */

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2] kernel/dma: Fix panic caused by passing swiotlb to command line

2019-01-10 Thread He Zhe



On 1/11/19 9:46 AM, He Zhe wrote:
>
> On 1/11/19 1:27 AM, Konrad Rzeszutek Wilk wrote:
>> Let's skip it. There was another patch that would allocate a default 4MB 
>> size if it there was an misue of swiotlb parameters.
> But this patch mainly fixes a crash. Could you please point me to the patch 
> you mentioned?

And the v2 is modified according to your suggestion.

Zhe

>
> Thanks,
> Zhe
>
>>
>>
>> On Mon, Jan 7, 2019, 4:07 AM Christoph Hellwig > <mailto:h...@lst.de> wrote:
>>
>> On Mon, Jan 07, 2019 at 04:46:51PM +0800, He Zhe wrote:
>> > Kindly ping.
>>
>> Konrad, I'll pick this up through the DMA mapping tree unless you
>> protest in the next few days.
>>
>

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2] kernel/dma: Fix panic caused by passing swiotlb to command line

2019-01-10 Thread He Zhe



On 1/11/19 1:27 AM, Konrad Rzeszutek Wilk wrote:
> Let's skip it. There was another patch that would allocate a default 4MB size 
> if it there was an misue of swiotlb parameters.

But this patch mainly fixes a crash. Could you please point me to the patch you 
mentioned?

Thanks,
Zhe

>
>
>
> On Mon, Jan 7, 2019, 4:07 AM Christoph Hellwig  <mailto:h...@lst.de> wrote:
>
> On Mon, Jan 07, 2019 at 04:46:51PM +0800, He Zhe wrote:
> > Kindly ping.
>
> Konrad, I'll pick this up through the DMA mapping tree unless you
> protest in the next few days.
>

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2] kernel/dma: Fix panic caused by passing swiotlb to command line

2019-01-07 Thread He Zhe
Kindly ping.

Zhe

On 12/3/18 6:22 PM, zhe...@windriver.com wrote:
> From: He Zhe 
>
> setup_io_tlb_npages does not check input argument before passing it
> to isdigit. The argument would be a NULL pointer if "swiotlb", without
> its value, is set in command line and thus causes the following panic.
>
> PANIC: early exception 0xe3 IP 10:bb9b8e9f error 0 cr2 0x0
> [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 
> 4.19.0-rc3-yocto-standard+ #9
> [0.00] RIP: 0010:setup_io_tlb_npages+0xf/0x95
> ...
> [0.00] Call Trace:
> [0.00]  do_early_param+0x57/0x8e
> [0.00]  parse_args+0x208/0x320
> [0.00]  ? rdinit_setup+0x30/0x30
> [0.00]  parse_early_options+0x29/0x2d
> [0.00]  ? rdinit_setup+0x30/0x30
> [0.00]  parse_early_param+0x36/0x4d
> [0.00]  setup_arch+0x336/0x99e
> [0.00]  start_kernel+0x6f/0x4e6
> [0.00]  x86_64_start_reservations+0x24/0x26
> [0.00]  x86_64_start_kernel+0x6f/0x72
> [0.00]  secondary_startup_64+0xa4/0xb0
>
> This patch adds a check to prevent the panic and sets it for 4MB by default.
>
> Signed-off-by: He Zhe 
> Cc: sta...@vger.kernel.org
> Cc: konrad.w...@oracle.com
> Cc: h...@lst.de
> Cc: m.szyprow...@samsung.com
> Cc: robin.mur...@arm.com
>
> Signed-off-by: He Zhe 
> ---
> v2: Set swiotlb for 4MB by default
>
>  kernel/dma/swiotlb.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 045930e..0e18cd4 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -58,6 +58,9 @@
>   */
>  #define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
>  
> +#define IO_TLB_DEFAULT_MB 4
> +#define IO_TLB_DEFAULT_SLABS ((IO_TLB_DEFAULT_MB<<20) >> IO_TLB_SHIFT)
> +
>  enum swiotlb_force swiotlb_force;
>  
>  /*
> @@ -103,6 +106,14 @@ static int late_alloc;
>  static int __init
>  setup_io_tlb_npages(char *str)
>  {
> + if (!str) {
> + pr_err("No value provided for swiotlb, "
> +"set it to %d slabs for %dMB by default.\n",
> +IO_TLB_DEFAULT_SLABS, IO_TLB_DEFAULT_MB);
> + io_tlb_nslabs = IO_TLB_DEFAULT_SLABS;
> + return 0;
> + }
> +
>   if (isdigit(*str)) {
>   io_tlb_nslabs = simple_strtoul(str, , 0);
>   /* avoid tail segment of size < IO_TLB_SEGSIZE */

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] kernel/dma: Fix panic caused by passing swiotlb to command line

2018-12-03 Thread He Zhe


On 2018/12/2 20:30, Konrad Rzeszutek Wilk wrote:
> On Tue, Oct 30, 2018 at 07:06:10PM +0800, He Zhe wrote:
>>
>> On 2018/10/23 19:14, He Zhe wrote:
>>> On 2018/10/23 03:29, Konrad Rzeszutek Wilk wrote:
>>>> On Sat, Sep 22, 2018 at 08:56:58PM +0800, He Zhe wrote:
>>>>> May I have your input?
>>>> Alternatively would it make more sense for it to assume some default
>>>> value?
>>> Maybe, but the original code has no default value and I have no idea
>>> what default value is proper here.
>> Can anyone give some suggestions? Though I'd prefer to do nothing when
>> no option is provided.
> One provided the paramter for a reason. I would just do a small one, say
> 4MB.

OK. Thanks, I'll send v2.

Zhe

>> Thanks,
>> Zhe
>>
>>> Zhe
>>>
>>>>> Thanks,
>>>>> Zhe
>>>>>
>>>>> On 2018年09月17日 11:27, zhe...@windriver.com wrote:
>>>>>> From: He Zhe 
>>>>>>
>>>>>> setup_io_tlb_npages does not check input argument before passing it
>>>>>> to isdigit. The argument would be a NULL pointer if "swiotlb", without
>>>>>> its value, is set in command line and thus causes the following panic.
>>>>>>
>>>>>> PANIC: early exception 0xe3 IP 10:bb9b8e9f error 0 cr2 0x0
>>>>>> [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 
>>>>>> 4.19.0-rc3-yocto-standard+ #9
>>>>>> [0.00] RIP: 0010:setup_io_tlb_npages+0xf/0x95
>>>>>> ...
>>>>>> [0.00] Call Trace:
>>>>>> [0.00]  do_early_param+0x57/0x8e
>>>>>> [0.00]  parse_args+0x208/0x320
>>>>>> [0.00]  ? rdinit_setup+0x30/0x30
>>>>>> [0.00]  parse_early_options+0x29/0x2d
>>>>>> [0.00]  ? rdinit_setup+0x30/0x30
>>>>>> [0.00]  parse_early_param+0x36/0x4d
>>>>>> [0.00]  setup_arch+0x336/0x99e
>>>>>> [0.00]  start_kernel+0x6f/0x4e6
>>>>>> [0.00]  x86_64_start_reservations+0x24/0x26
>>>>>> [0.00]  x86_64_start_kernel+0x6f/0x72
>>>>>> [0.00]  secondary_startup_64+0xa4/0xb0
>>>>>>
>>>>>> This patch adds a check to prevent the panic.
>>>>>>
>>>>>> Signed-off-by: He Zhe 
>>>>>> Cc: sta...@vger.kernel.org
>>>>>> Cc: konrad.w...@oracle.com
>>>>>> Cc: h...@lst.de
>>>>>> Cc: m.szyprow...@samsung.com
>>>>>> Cc: robin.mur...@arm.com
>>>>>> ---
>>>>>>  kernel/dma/swiotlb.c | 5 +
>>>>>>  1 file changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>>>>>> index 4f8a6db..46fc34e 100644
>>>>>> --- a/kernel/dma/swiotlb.c
>>>>>> +++ b/kernel/dma/swiotlb.c
>>>>>> @@ -109,6 +109,11 @@ static int late_alloc;
>>>>>>  static int __init
>>>>>>  setup_io_tlb_npages(char *str)
>>>>>>  {
>>>>>> +if (!str) {
>>>>>> +pr_err("Config string not provided\n");
>>>>>> +return -EINVAL;
>>>>>> +}
>>>>>> +
>>>>>>  if (isdigit(*str)) {
>>>>>>  io_tlb_nslabs = simple_strtoul(str, , 0);
>>>>>>  /* avoid tail segment of size < IO_TLB_SEGSIZE */
>>>>> ___
>>>>> iommu mailing list
>>>>> iommu@lists.linux-foundation.org
>>>>> https://lists.linuxfoundation.org/mailman/listinfo/iommu

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] kernel/dma: Fix panic caused by passing swiotlb to command line

2018-10-30 Thread He Zhe


On 2018/10/23 19:14, He Zhe wrote:
>
> On 2018/10/23 03:29, Konrad Rzeszutek Wilk wrote:
>> On Sat, Sep 22, 2018 at 08:56:58PM +0800, He Zhe wrote:
>>> May I have your input?
>> Alternatively would it make more sense for it to assume some default
>> value?
> Maybe, but the original code has no default value and I have no idea
> what default value is proper here.

Can anyone give some suggestions? Though I'd prefer to do nothing when
no option is provided.

Thanks,
Zhe

>
> Zhe
>
>>> Thanks,
>>> Zhe
>>>
>>> On 2018年09月17日 11:27, zhe...@windriver.com wrote:
>>>> From: He Zhe 
>>>>
>>>> setup_io_tlb_npages does not check input argument before passing it
>>>> to isdigit. The argument would be a NULL pointer if "swiotlb", without
>>>> its value, is set in command line and thus causes the following panic.
>>>>
>>>> PANIC: early exception 0xe3 IP 10:bb9b8e9f error 0 cr2 0x0
>>>> [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 
>>>> 4.19.0-rc3-yocto-standard+ #9
>>>> [0.00] RIP: 0010:setup_io_tlb_npages+0xf/0x95
>>>> ...
>>>> [0.00] Call Trace:
>>>> [0.00]  do_early_param+0x57/0x8e
>>>> [0.00]  parse_args+0x208/0x320
>>>> [0.00]  ? rdinit_setup+0x30/0x30
>>>> [0.00]  parse_early_options+0x29/0x2d
>>>> [0.00]  ? rdinit_setup+0x30/0x30
>>>> [0.00]  parse_early_param+0x36/0x4d
>>>> [    0.00]  setup_arch+0x336/0x99e
>>>> [0.00]  start_kernel+0x6f/0x4e6
>>>> [0.00]  x86_64_start_reservations+0x24/0x26
>>>> [0.00]  x86_64_start_kernel+0x6f/0x72
>>>> [0.00]  secondary_startup_64+0xa4/0xb0
>>>>
>>>> This patch adds a check to prevent the panic.
>>>>
>>>> Signed-off-by: He Zhe 
>>>> Cc: sta...@vger.kernel.org
>>>> Cc: konrad.w...@oracle.com
>>>> Cc: h...@lst.de
>>>> Cc: m.szyprow...@samsung.com
>>>> Cc: robin.mur...@arm.com
>>>> ---
>>>>  kernel/dma/swiotlb.c | 5 +
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>>>> index 4f8a6db..46fc34e 100644
>>>> --- a/kernel/dma/swiotlb.c
>>>> +++ b/kernel/dma/swiotlb.c
>>>> @@ -109,6 +109,11 @@ static int late_alloc;
>>>>  static int __init
>>>>  setup_io_tlb_npages(char *str)
>>>>  {
>>>> +  if (!str) {
>>>> +  pr_err("Config string not provided\n");
>>>> +  return -EINVAL;
>>>> +  }
>>>> +
>>>>if (isdigit(*str)) {
>>>>io_tlb_nslabs = simple_strtoul(str, , 0);
>>>>/* avoid tail segment of size < IO_TLB_SEGSIZE */
>>> ___
>>> iommu mailing list
>>> iommu@lists.linux-foundation.org
>>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] kernel/dma: Fix panic caused by passing swiotlb to command line

2018-10-23 Thread He Zhe


On 2018/10/23 03:29, Konrad Rzeszutek Wilk wrote:
> On Sat, Sep 22, 2018 at 08:56:58PM +0800, He Zhe wrote:
>> May I have your input?
> Alternatively would it make more sense for it to assume some default
> value?

Maybe, but the original code has no default value and I have no idea
what default value is proper here.

Zhe

>> Thanks,
>> Zhe
>>
>> On 2018年09月17日 11:27, zhe...@windriver.com wrote:
>>> From: He Zhe 
>>>
>>> setup_io_tlb_npages does not check input argument before passing it
>>> to isdigit. The argument would be a NULL pointer if "swiotlb", without
>>> its value, is set in command line and thus causes the following panic.
>>>
>>> PANIC: early exception 0xe3 IP 10:bb9b8e9f error 0 cr2 0x0
>>> [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 
>>> 4.19.0-rc3-yocto-standard+ #9
>>> [0.00] RIP: 0010:setup_io_tlb_npages+0xf/0x95
>>> ...
>>> [0.00] Call Trace:
>>> [0.00]  do_early_param+0x57/0x8e
>>> [0.00]  parse_args+0x208/0x320
>>> [0.00]  ? rdinit_setup+0x30/0x30
>>> [0.00]  parse_early_options+0x29/0x2d
>>> [0.00]  ? rdinit_setup+0x30/0x30
>>> [0.00]  parse_early_param+0x36/0x4d
>>> [0.00]  setup_arch+0x336/0x99e
>>> [0.00]  start_kernel+0x6f/0x4e6
>>> [0.00]  x86_64_start_reservations+0x24/0x26
>>> [0.00]  x86_64_start_kernel+0x6f/0x72
>>> [0.00]  secondary_startup_64+0xa4/0xb0
>>>
>>> This patch adds a check to prevent the panic.
>>>
>>> Signed-off-by: He Zhe 
>>> Cc: sta...@vger.kernel.org
>>> Cc: konrad.w...@oracle.com
>>> Cc: h...@lst.de
>>> Cc: m.szyprow...@samsung.com
>>> Cc: robin.mur...@arm.com
>>> ---
>>>  kernel/dma/swiotlb.c | 5 +
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>>> index 4f8a6db..46fc34e 100644
>>> --- a/kernel/dma/swiotlb.c
>>> +++ b/kernel/dma/swiotlb.c
>>> @@ -109,6 +109,11 @@ static int late_alloc;
>>>  static int __init
>>>  setup_io_tlb_npages(char *str)
>>>  {
>>> +   if (!str) {
>>> +   pr_err("Config string not provided\n");
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> if (isdigit(*str)) {
>>> io_tlb_nslabs = simple_strtoul(str, , 0);
>>> /* avoid tail segment of size < IO_TLB_SEGSIZE */
>> ___
>> iommu mailing list
>> iommu@lists.linux-foundation.org
>> https://lists.linuxfoundation.org/mailman/listinfo/iommu

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] kernel/dma: Fix panic caused by passing swiotlb to command line

2018-09-22 Thread He Zhe
May I have your input?

Thanks,
Zhe

On 2018年09月17日 11:27, zhe...@windriver.com wrote:
> From: He Zhe 
>
> setup_io_tlb_npages does not check input argument before passing it
> to isdigit. The argument would be a NULL pointer if "swiotlb", without
> its value, is set in command line and thus causes the following panic.
>
> PANIC: early exception 0xe3 IP 10:bb9b8e9f error 0 cr2 0x0
> [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 
> 4.19.0-rc3-yocto-standard+ #9
> [0.00] RIP: 0010:setup_io_tlb_npages+0xf/0x95
> ...
> [0.00] Call Trace:
> [0.00]  do_early_param+0x57/0x8e
> [0.00]  parse_args+0x208/0x320
> [0.00]  ? rdinit_setup+0x30/0x30
> [0.00]  parse_early_options+0x29/0x2d
> [0.00]  ? rdinit_setup+0x30/0x30
> [0.00]  parse_early_param+0x36/0x4d
> [0.00]  setup_arch+0x336/0x99e
> [0.00]  start_kernel+0x6f/0x4e6
> [0.00]  x86_64_start_reservations+0x24/0x26
> [0.00]  x86_64_start_kernel+0x6f/0x72
> [0.00]  secondary_startup_64+0xa4/0xb0
>
> This patch adds a check to prevent the panic.
>
> Signed-off-by: He Zhe 
> Cc: sta...@vger.kernel.org
> Cc: konrad.w...@oracle.com
> Cc: h...@lst.de
> Cc: m.szyprow...@samsung.com
> Cc: robin.mur...@arm.com
> ---
>  kernel/dma/swiotlb.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 4f8a6db..46fc34e 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -109,6 +109,11 @@ static int late_alloc;
>  static int __init
>  setup_io_tlb_npages(char *str)
>  {
> + if (!str) {
> + pr_err("Config string not provided\n");
> + return -EINVAL;
> + }
> +
>   if (isdigit(*str)) {
>   io_tlb_nslabs = simple_strtoul(str, , 0);
>   /* avoid tail segment of size < IO_TLB_SEGSIZE */

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu