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

[Quanyang Wang]

On 10/18/19 10:42 PM, Radhey Shyam Pandey wrote:
>> -----Original Message-----
>> From: Hyun Kwon <hyun.kwon at xilinx.com>
>> Sent: Thursday, October 17, 2019 11:01 PM
>> To: Quanyang Wang <quanyang.wang at windriver.com>
>> Cc: Hyun Kwon <hyunk at xilinx.com>; Michal Simek <michals at xilinx.com>;
>> Bruce <bruce.ashfield at gmail.com>; linux-yocto at yoctoproject.org; Radhey
>> Shyam Pandey <radheys at xilinx.com>
>> 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.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.
>> 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