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

[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.wang at windriver.com wrote:
>>> From: Quanyang Wang <quanyang.wang at windriver.com>
>>>
>>> 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:
>>> [<ffffff80109f4560>] 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 <quanyang.wang at windriver.com>
> 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 <hyun.kwon at xilinx.com>

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