2022-02-15 23:55:25

by John Ogness

[permalink] [raw]
Subject: Re: [PATCH] drm: fb-helper: Avoid nesting spinlock_t into raw_spinlock_t

On 2022-02-15, Sebastian Siewior <[email protected]> wrote:
>> From: Jiri Kosina <[email protected]>
>>
>> drm_fb_helper_damage() is acquiring spinlock_t (helper->damage_lock),
>> while it can be called from contexts where raw_spinlock_t is held (e.g.
>> console_owner lock obtained on vprintk_emit() codepath).
>>
>> As the critical sections protected by damage_lock are super-tiny, let's
>> fix this by converting it to raw_spinlock_t in order not to violate
>> PREEMPT_RT-imposed lock nesting rules.
>>
>> This fixes the splat below.
>>
>> =============================
>> [ BUG: Invalid wait context ]
>> 5.17.0-rc4-00002-gd567f5db412e #1 Not tainted
>
> rc4. Is this also the case in the RT tree which includes John's printk
> changes?

In the RT tree, the fbcon's write() callback is only called in
preemptible() contexts. So this is only a mainline issue.

The series I recently posted to LKML [0] should also address this issue
(if/when it gets accepted).

John

[0] https://lore.kernel.org/lkml/[email protected]


2022-02-16 06:45:01

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH] drm: fb-helper: Avoid nesting spinlock_t into raw_spinlock_t

On Tue, 15 Feb 2022, John Ogness wrote:

> >> drm_fb_helper_damage() is acquiring spinlock_t (helper->damage_lock),
> >> while it can be called from contexts where raw_spinlock_t is held (e.g.
> >> console_owner lock obtained on vprintk_emit() codepath).
> >>
> >> As the critical sections protected by damage_lock are super-tiny, let's
> >> fix this by converting it to raw_spinlock_t in order not to violate
> >> PREEMPT_RT-imposed lock nesting rules.
> >>
> >> This fixes the splat below.
> >>
> >> =============================
> >> [ BUG: Invalid wait context ]
> >> 5.17.0-rc4-00002-gd567f5db412e #1 Not tainted
> >
> > rc4. Is this also the case in the RT tree which includes John's printk
> > changes?
>
> In the RT tree, the fbcon's write() callback is only called in
> preemptible() contexts. So this is only a mainline issue.
>
> The series I recently posted to LKML [0] should also address this issue
> (if/when it gets accepted).
>
> John
>
> [0] https://lore.kernel.org/lkml/[email protected]

Thanks for confirmation, seems like this problem is indeed cured by your
series.

I still believe though that we shouldn't let 5.17 released with this
warning being emitted into dmesg, so I'd suggest going with my patch for
mainline, and revert it in your series on top of it.

Thanks,

--
Jiri Kosina
SUSE Labs

Subject: Re: [PATCH] drm: fb-helper: Avoid nesting spinlock_t into raw_spinlock_t

On 2022-02-15 20:59:24 [+0100], Jiri Kosina wrote:
> Thanks for confirmation, seems like this problem is indeed cured by your
> series.

Oki.

> I still believe though that we shouldn't let 5.17 released with this
> warning being emitted into dmesg, so I'd suggest going with my patch for
> mainline, and revert it in your series on top of it.

No. That warning is only visible with CONFIG_PROVE_RAW_LOCK_NESTING with
the following paragraph in its help:
| NOTE: There are known nesting problems. So if you enable this
| option expect lockdep splats until these problems have been fully
| addressed which is work in progress. This config switch allows to
| identify and analyze these problems. It will be removed and the
| check permanently enabled once the main issues have been fixed.

This warning in this call chain should affect every console driver which
acquires a lock.

> Thanks,

Sebastian