[linux-yocto] [PATCH 1/1] dma: xilinx: dpdma: alloc xilinx_dpdma_tx_desc using GFP_ATOMIC

2019-10-22 Thread quanyang.wang 
From: Quanyang Wang 

The function xilinx_dpdma_chan_alloc_tx_desc can be called
from atomic context, hence use GFP_ATOMIC instead of GFP_KERNEL,
or else it will cause the calltrace as below:

BUG: sleeping function called from invalid context at mm/slab.h:418
in_atomic(): 1, irqs_disabled(): 128, pid: 416, name: alsa-sink-(null 
Preemption disabled at:
[] snd_pcm_stream_lock_irq+0x40/0x50
CPU: 2 PID: 416 Comm: alsa-sink-(null Tainted: G O 5.2.17-yocto-standard #1
Hardware name: ZynqMP ZCU102 Rev1.0 (DT) Call trace:
dump_backtrace+0x0/0x138
show_stack+0x24/0x30
dump_stack+0x94/0xbc
___might_sleep+0x14c/0x178
__might_sleep+0x58/0x90
kmem_cache_alloc_trace+0x1a4/0x300
xilinx_dpdma_chan_alloc_tx_desc.isra.0+0x24/0x40
xilinx_dpdma_prep_dma_cyclic+0xc0/0x208
snd_dmaengine_pcm_trigger+0xf8/0x198
soc_pcm_trigger+0xb4/0x128
snd_pcm_do_start+0x48/0x58
snd_pcm_action_single+0x4c/0xa0
snd_pcm_action+0x78/0x90
snd_pcm_action_lock_irq+0x3c/0x60
snd_pcm_common_ioctl+0x4ac/0x10c8
snd_pcm_ioctl+0x48/0x68
do_vfs_ioctl+0x6d4/0x968
ksys_ioctl+0x84/0xb8
__arm64_sys_ioctl+0x28/0x38
el0_svc_common.constprop.0+0x74/0x180
el0_svc_handler+0x70/0x90
el0_svc+0x8/0xc

Signed-off-by: Quanyang Wang 
Reviewed-by: Hyun Kwon 
---
There are two available flags which can be used here: GFP_ATOMIC
and GFP_NOWAIT. The difference is that GFP_ATOMIC may grab memory
from emergency pools but GFP_NOWAIT not. Since struct xilinx_dpdma_tx_desc
is small, use GFP_ATOMIC is better.

 drivers/dma/xilinx/xilinx_dpdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/xilinx/xilinx_dpdma.c 
b/drivers/dma/xilinx/xilinx_dpdma.c
index 1ea509e00742..89ade893f51c 100644
--- a/drivers/dma/xilinx/xilinx_dpdma.c
+++ b/drivers/dma/xilinx/xilinx_dpdma.c
@@ -797,7 +797,7 @@ xilinx_dpdma_chan_alloc_tx_desc(struct xilinx_dpdma_chan 
*chan)
 {
struct xilinx_dpdma_tx_desc *tx_desc;
 
-   tx_desc = kzalloc(sizeof(*tx_desc), GFP_KERNEL);
+   tx_desc = kzalloc(sizeof(*tx_desc), GFP_ATOMIC);
if (!tx_desc)
return NULL;
 
-- 
2.17.1

-- 
___
linux-yocto mailing list
linux-yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/linux-yocto

Re: [linux-yocto] [PATCH 1/1] dma: xilinx: dpdma: alloc xilinx_dpdma_tx_desc using GFP_ATOMIC

2019-10-22 Thread Quanyang Wang 


On 10/18/19 10:42 PM, Radhey Shyam Pandey wrote:

-Original Message-
From: Hyun Kwon 
Sent: Thursday, October 17, 2019 11:01 PM
To: Quanyang Wang 
Cc: Hyun Kwon ; Michal Simek ;
Bruce ; linux-yocto@yoctoproject.org; Radhey
Shyam Pandey 
Subject: Re: [linux-yocto][PATCH 1/1] dma: xilinx: dpdma: alloc
xilinx_dpdma_tx_desc using GFP_ATOMIC

Hi Quanyang,

+ Radhey

On Wed, 2019-10-16 at 20:11:39 -0700, Quanyang Wang wrote:

On 10/17/19 1:01 AM, Hyun Kwon wrote:

Hi Quanyang,

Thanks for the patch.

On Tue, 2019-10-15 at 22:49:41 -0700, Michal Simek wrote:

+Hyun,

On 15. 10. 19 16:56, quanyang.w...@windriver.com wrote:

From: Quanyang Wang 

The function xilinx_dpdma_chan_alloc_tx_desc can be called
from atomic context, hence use GFP_ATOMIC instead of GFP_KERNEL,
or else it will cause the calltrace as below:

BUG: sleeping function called from invalid context at mm/slab.h:418
in_atomic(): 1, irqs_disabled(): 128, pid: 416, name: alsa-sink-(null

Preemption disabled at:

[] snd_pcm_stream_lock_irq+0x40/0x50
CPU: 2 PID: 416 Comm: alsa-sink-(null Tainted: G O 5.2.17-yocto-

standard #1

Hardware name: ZynqMP ZCU102 Rev1.0 (DT) Call trace:
dump_backtrace+0x0/0x138
show_stack+0x24/0x30
dump_stack+0x94/0xbc
___might_sleep+0x14c/0x178
__might_sleep+0x58/0x90
kmem_cache_alloc_trace+0x1a4/0x300
xilinx_dpdma_chan_alloc_tx_desc.isra.0+0x24/0x40
xilinx_dpdma_prep_dma_cyclic+0xc0/0x208
snd_dmaengine_pcm_trigger+0xf8/0x198
soc_pcm_trigger+0xb4/0x128
snd_pcm_do_start+0x48/0x58
snd_pcm_action_single+0x4c/0xa0
snd_pcm_action+0x78/0x90
snd_pcm_action_lock_irq+0x3c/0x60
snd_pcm_common_ioctl+0x4ac/0x10c8
snd_pcm_ioctl+0x48/0x68
do_vfs_ioctl+0x6d4/0x968
ksys_ioctl+0x84/0xb8
__arm64_sys_ioctl+0x28/0x38
el0_svc_common.constprop.0+0x74/0x180
el0_svc_handler+0x70/0x90
el0_svc+0x8/0xc

Signed-off-by: Quanyang Wang 

Maybe the 'nonatomic' can be set to true in xilinx_dp_dai_links? But I

don't

know much about the flag, ex what's required, and I don't see any

problem with

this change. So,

   Reviewed-by: Hyun Kwon 

Hi Hyun,

Thank you for looking this patch over.

I test and the "nonatomic" can also eliminate the calltrace.  But I
don't know if there is any

problem with alsa driver running in a nonatomic context.

Me neither, so maybe changing gfp flag seems safer in that aspect. :-)


And do you think it's better using GFP_NOWAIT instead of GFP_ATOMIC ?
This can save some atomic memory.

In my limited view, GFP_ATOMIC seems better as the allocation is relatively
small but related to IO, rather than relying only on page reclaim. I see both
are quite commonly used for dma descriptor allocation, so it doesn't seem
like
one is clearly better than the other. But I may be wrong. Radhey may have
better answer.

_NOWAIT is like _ATOMIC except that no wait will not fall back to emergency
pools. http://lkml.iu.edu/hypermail/linux/kernel/0911.1/02316.html
IMO if allocation is in critical path and small then GFP_ATOMIC can be used
else switching to _NOWAIT might be preferred.


Hi Radhey,

Thank you for your suggestion.

Since xilinx_dpdma_tx_desc is small, it seems that GFP_ATOMIC is better.

Thanks,

Quanyang




Thanks,
-hyun

--
___
linux-yocto mailing list
linux-yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/linux-yocto

Re: [linux-yocto] [PATCH 1/1] dma: xilinx: dpdma: alloc xilinx_dpdma_tx_desc using GFP_ATOMIC

2019-10-16 Thread Quanyang Wang 


On 10/17/19 1:01 AM, Hyun Kwon wrote:

Hi Quanyang,

Thanks for the patch.

On Tue, 2019-10-15 at 22:49:41 -0700, Michal Simek wrote:

+Hyun,

On 15. 10. 19 16:56, quanyang.w...@windriver.com wrote:

From: Quanyang Wang 

The function xilinx_dpdma_chan_alloc_tx_desc can be called
from atomic context, hence use GFP_ATOMIC instead of GFP_KERNEL,
or else it will cause the calltrace as below:

BUG: sleeping function called from invalid context at mm/slab.h:418
in_atomic(): 1, irqs_disabled(): 128, pid: 416, name: alsa-sink-(null 
Preemption disabled at:
[] snd_pcm_stream_lock_irq+0x40/0x50
CPU: 2 PID: 416 Comm: alsa-sink-(null Tainted: G O 5.2.17-yocto-standard #1
Hardware name: ZynqMP ZCU102 Rev1.0 (DT) Call trace:
dump_backtrace+0x0/0x138
show_stack+0x24/0x30
dump_stack+0x94/0xbc
___might_sleep+0x14c/0x178
__might_sleep+0x58/0x90
kmem_cache_alloc_trace+0x1a4/0x300
xilinx_dpdma_chan_alloc_tx_desc.isra.0+0x24/0x40
xilinx_dpdma_prep_dma_cyclic+0xc0/0x208
snd_dmaengine_pcm_trigger+0xf8/0x198
soc_pcm_trigger+0xb4/0x128
snd_pcm_do_start+0x48/0x58
snd_pcm_action_single+0x4c/0xa0
snd_pcm_action+0x78/0x90
snd_pcm_action_lock_irq+0x3c/0x60
snd_pcm_common_ioctl+0x4ac/0x10c8
snd_pcm_ioctl+0x48/0x68
do_vfs_ioctl+0x6d4/0x968
ksys_ioctl+0x84/0xb8
__arm64_sys_ioctl+0x28/0x38
el0_svc_common.constprop.0+0x74/0x180
el0_svc_handler+0x70/0x90
el0_svc+0x8/0xc

Signed-off-by: Quanyang Wang 

Maybe the 'nonatomic' can be set to true in xilinx_dp_dai_links? But I don't
know much about the flag, ex what's required, and I don't see any problem with
this change. So,

  Reviewed-by: Hyun Kwon 


Hi Hyun,

Thank you for looking this patch over.

I test and the "nonatomic" can also eliminate the calltrace.  But I 
don't know if there is any


problem with alsa driver running in a nonatomic context.

And do you think it's better using GFP_NOWAIT instead of GFP_ATOMIC ? 
This can save some atomic memory.


Hope to get your advice.

Thanks,

Quanyang




Thanks,
-hyun


---
  drivers/dma/xilinx/xilinx_dpdma.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/xilinx/xilinx_dpdma.c 
b/drivers/dma/xilinx/xilinx_dpdma.c
index 1ea509e00742..89ade893f51c 100644
--- a/drivers/dma/xilinx/xilinx_dpdma.c
+++ b/drivers/dma/xilinx/xilinx_dpdma.c
@@ -797,7 +797,7 @@ xilinx_dpdma_chan_alloc_tx_desc(struct xilinx_dpdma_chan 
*chan)
  {
struct xilinx_dpdma_tx_desc *tx_desc;
  
-	tx_desc = kzalloc(sizeof(*tx_desc), GFP_KERNEL);

+   tx_desc = kzalloc(sizeof(*tx_desc), GFP_ATOMIC);
if (!tx_desc)
return NULL;
  


Hyun: Can you please review this?

Thanks,
Michal

--
___
linux-yocto mailing list
linux-yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/linux-yocto

[linux-yocto] [PATCH 1/1] dma: xilinx: dpdma: alloc xilinx_dpdma_tx_desc using GFP_ATOMIC

2019-10-15 Thread quanyang.wang 
From: Quanyang Wang 

The function xilinx_dpdma_chan_alloc_tx_desc can be called
from atomic context, hence use GFP_ATOMIC instead of GFP_KERNEL,
or else it will cause the calltrace as below:

BUG: sleeping function called from invalid context at mm/slab.h:418
in_atomic(): 1, irqs_disabled(): 128, pid: 416, name: alsa-sink-(null 
Preemption disabled at:
[] snd_pcm_stream_lock_irq+0x40/0x50
CPU: 2 PID: 416 Comm: alsa-sink-(null Tainted: G O 5.2.17-yocto-standard #1
Hardware name: ZynqMP ZCU102 Rev1.0 (DT) Call trace:
dump_backtrace+0x0/0x138
show_stack+0x24/0x30
dump_stack+0x94/0xbc
___might_sleep+0x14c/0x178
__might_sleep+0x58/0x90
kmem_cache_alloc_trace+0x1a4/0x300
xilinx_dpdma_chan_alloc_tx_desc.isra.0+0x24/0x40
xilinx_dpdma_prep_dma_cyclic+0xc0/0x208
snd_dmaengine_pcm_trigger+0xf8/0x198
soc_pcm_trigger+0xb4/0x128
snd_pcm_do_start+0x48/0x58
snd_pcm_action_single+0x4c/0xa0
snd_pcm_action+0x78/0x90
snd_pcm_action_lock_irq+0x3c/0x60
snd_pcm_common_ioctl+0x4ac/0x10c8
snd_pcm_ioctl+0x48/0x68
do_vfs_ioctl+0x6d4/0x968
ksys_ioctl+0x84/0xb8
__arm64_sys_ioctl+0x28/0x38
el0_svc_common.constprop.0+0x74/0x180
el0_svc_handler+0x70/0x90
el0_svc+0x8/0xc

Signed-off-by: Quanyang Wang 
---
 drivers/dma/xilinx/xilinx_dpdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/xilinx/xilinx_dpdma.c 
b/drivers/dma/xilinx/xilinx_dpdma.c
index 1ea509e00742..89ade893f51c 100644
--- a/drivers/dma/xilinx/xilinx_dpdma.c
+++ b/drivers/dma/xilinx/xilinx_dpdma.c
@@ -797,7 +797,7 @@ xilinx_dpdma_chan_alloc_tx_desc(struct xilinx_dpdma_chan 
*chan)
 {
struct xilinx_dpdma_tx_desc *tx_desc;
 
-   tx_desc = kzalloc(sizeof(*tx_desc), GFP_KERNEL);
+   tx_desc = kzalloc(sizeof(*tx_desc), GFP_ATOMIC);
if (!tx_desc)
return NULL;
 
-- 
2.17.1

-- 
___
linux-yocto mailing list
linux-yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/linux-yocto