On 1/16/24 13:41, Bartosz Golaszewski wrote:
On Tue, Jan 16, 2024 at 7:23 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
Hi,
On Tue, Dec 19, 2023 at 09:11:02PM +0100, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
extra_checks is only used in a few places. It also depends on
a non-standard DEBUG define one needs to add to the source file. The
overhead of removing it should be minimal (we already use pure
might_sleep() in the code anyway) so drop it.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
This patch triggers (exposes) the following backtrace.
BUG: sleeping function called from invalid context at drivers/gpio/gpiolib.c:3738
in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 7, name: kworker/0:0
preempt_count: 1, expected: 0
RCU nest depth: 0, expected: 0
3 locks held by kworker/0:0/7:
#0: c181b3a4 ((wq_completion)events_freezable){+.+.}-{0:0}, at: process_scheduled_works+0x23c/0x644
#1: c883df28 ((work_completion)(&(&host->detect)->work)){+.+.}-{0:0}, at: process_scheduled_works+0x23c/0x644
#2: c24e1720 (&host->lock){-...}-{2:2}, at: sdhci_check_ro+0x14/0xd4
irq event stamp: 2916
hardirqs last enabled at (2915): [<c0b18838>] _raw_spin_unlock_irqrestore+0x70/0x84
hardirqs last disabled at (2916): [<c0b1853c>] _raw_spin_lock_irqsave+0x74/0x78
softirqs last enabled at (2360): [<c00098a4>] __do_softirq+0x28c/0x4b0
softirqs last disabled at (2347): [<c0022774>] __irq_exit_rcu+0x15c/0x1a4
CPU: 0 PID: 7 Comm: kworker/0:0 Tainted: G N 6.7.0-09928-g052d534373b7 #1
Hardware name: Freescale i.MX25 (Device Tree Support)
Workqueue: events_freezable mmc_rescan
unwind_backtrace from show_stack+0x10/0x18
show_stack from dump_stack_lvl+0x34/0x54
dump_stack_lvl from __might_resched+0x188/0x274
__might_resched from gpiod_get_value_cansleep+0x14/0x60
gpiod_get_value_cansleep from mmc_gpio_get_ro+0x20/0x30
When getting GPIO value with a spinlock taken the driver *must* use
the non-sleeping variant of this function: gpiod_get_value(). If the
underlying driver can sleep then the developer seriously borked. The
API contract has always been this way so I wouldn't treat it as a
regression.
I said
"This patch triggers (exposes) the following backtrace"
and
"It isn't really surprising since sdhci_check_ro() calls the gpio code under
spin_lock_irqsave().
"
I didn't (intend to) claim that this would be a regression. It was
supposed to be a report. My apologies if it came along the wrong way.
Guenter
