Ok, that works for me as well.
Please always check if it is really necessary before adding any
GFP_ATOMIC allocation, cause that is rather invasive and should be avoided.
Christian.
Am 01.06.2018 um 11:56 schrieb S, Shirish:
>
> The V2 of this patch is already reviewed by Harry.
> The change i have made in dc_create() is no more applicable.
>
> Regards,
> Shirish S
> On 5/31/2018 11:35 PM, Christian König wrote:
>> Am 30.05.2018 um 18:03 schrieb Harry Wentland:
>>> On 2018-05-30 06:17 AM, Shirish S wrote:
>>>> This patch fixes the warning messages that are caused due to calling
>>>> sleep in atomic context as below:
>>>>
>>>> BUG: sleeping function called from invalid context at mm/slab.h:419
>>>> in_atomic(): 1, irqs_disabled(): 1, pid: 5, name: kworker/u4:0
>>>> CPU: 1 PID: 5 Comm: kworker/u4:0 Tainted: GÂ Â Â Â Â Â Â W 4.14.35 #941
>>>> Workqueue: events_unbound commit_work
>>>> Call Trace:
>>>> Â dump_stack+0x4d/0x63
>>>> Â ___might_sleep+0x11f/0x12e
>>>> Â kmem_cache_alloc_trace+0x41/0xea
>>>> Â dc_create_state+0x1f/0x30
>>>> Â dc_commit_updates_for_stream+0x73/0x4cf
>>>> Â ? amdgpu_get_crtc_scanoutpos+0x82/0x16b
>>>> Â amdgpu_dm_do_flip+0x239/0x298
>>>> Â amdgpu_dm_commit_planes.isra.23+0x379/0x54b
>>>> Â ? dc_commit_state+0x3da/0x404
>>>> Â amdgpu_dm_atomic_commit_tail+0x4fc/0x5d2
>>>> Â ? wait_for_common+0x5b/0x69
>>>> Â commit_tail+0x42/0x64
>>>> Â process_one_work+0x1b0/0x314
>>>> Â worker_thread+0x1cb/0x2c1
>>>> Â ? create_worker+0x1da/0x1da
>>>> Â kthread+0x156/0x15e
>>>> Â ? kthread_flush_work+0xea/0xea
>>>> Â ret_from_fork+0x22/0x40
>>>>
>>>> Signed-off-by: Shirish S <shirish.s at amd.com>
>>>> ---
>>>> Â drivers/gpu/drm/amd/display/dc/core/dc.c | 4 ++--
>>>> Â 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
>>>> b/drivers/gpu/drm/amd/display/dc/core/dc.c
>>>> index 33149ed..d62206f 100644
>>>> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
>>>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
>>>> @@ -588,7 +588,7 @@ static void disable_dangling_plane(struct dc
>>>> *dc, struct dc_state *context)
>>>> Â Â struct dc *dc_create(const struct dc_init_data *init_params)
>>>> Â Â {
>>>> -Â Â Â struct dc *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
>>>> +Â Â Â struct dc *dc = kzalloc(sizeof(*dc), GFP_ATOMIC);
>>> Are you sure this one can be called in atomic_context?
>>>
>>> If so then everything in consstruct() would also need GFP_ATOMIC.
>>
>> Well the backtrace is quite obvious, but I agree that change still
>> looks fishy to me as well.
>>
>> Using GFP_ATOMIC should only be a last resort when nothing else
>> helps, but here it looks more like we misuse a spinlock where a mutex
>> or semaphore would be more appropriate.
>>
>> Where exactly becomes the context atomic in the call trace?
>>
>> Christian.
>>
>>>
>>> Harry
>>>
>>>> Â Â Â Â Â unsigned int full_pipe_count;
>>>> Â Â Â Â Â Â if (NULL == dc)
>>>> @@ -937,7 +937,7 @@ bool dc_post_update_surfaces_to_stream(struct
>>>> dc *dc)
>>>> Â struct dc_state *dc_create_state(void)
>>>> Â {
>>>> Â Â Â Â Â struct dc_state *context = kzalloc(sizeof(struct dc_state),
>>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â GFP_KERNEL);
>>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â GFP_ATOMIC);
>>>> Â Â Â Â Â Â if (!context)
>>>> Â Â Â Â Â Â Â Â Â return NULL;
>>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>
