* [PATCH v2 0/2] ide: Remove in_interrupt().
@ 2020-11-20 9:24 Sebastian Andrzej Siewior
2020-11-20 9:24 ` [PATCH v2 1/2] ide/Falcon: Remove in_interrupt() usage Sebastian Andrzej Siewior
2020-11-20 9:24 ` [PATCH v2 2/2] ide: Remove BUG_ON(in_interrupt() || irqs_disabled()) from ide_unregister() Sebastian Andrzej Siewior
0 siblings, 2 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2020-11-20 9:24 UTC (permalink / raw)
To: linux-ide; +Cc: linux-kernel, David S. Miller, Jens Axboe, Andrew Morton, tglx
In the discussion about preempt count consistency across kernel
configurations:
https://lore.kernel.org/r/20200914204209.256266093@linutronix.de/
it was concluded that the usage of in_interrupt() and related context
checks should be removed from non-core code.
This ide subsystem has two in_interrupts() checks before mutex/wait-event
invocations.
Changes since v1
https://lkml.kernel.org/r/20201113161021.2217361-1-bigeasy@linutronix.de
- added acks
- added lkml to Cc:
Sebastian
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] ide/Falcon: Remove in_interrupt() usage.
2020-11-20 9:24 [PATCH v2 0/2] ide: Remove in_interrupt() Sebastian Andrzej Siewior
@ 2020-11-20 9:24 ` Sebastian Andrzej Siewior
2020-11-20 22:35 ` Andrew Morton
2020-11-20 9:24 ` [PATCH v2 2/2] ide: Remove BUG_ON(in_interrupt() || irqs_disabled()) from ide_unregister() Sebastian Andrzej Siewior
1 sibling, 1 reply; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2020-11-20 9:24 UTC (permalink / raw)
To: linux-ide
Cc: linux-kernel, David S. Miller, Jens Axboe, Andrew Morton, tglx,
Sebastian Andrzej Siewior
falconide_get_lock() is called by ide_lock_host() and its caller
(ide_issue_rq()) has already a might_sleep() check.
stdma_lock() has wait_event() which also has a might_sleep() check.
Remove the in_interrupt() check.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-ide@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Jens Axboe <axboe@kernel.dk>
---
drivers/ide/falconide.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/ide/falconide.c b/drivers/ide/falconide.c
index dbeb2605e5f6e..77af4c1a3f38c 100644
--- a/drivers/ide/falconide.c
+++ b/drivers/ide/falconide.c
@@ -51,8 +51,6 @@ static void falconide_release_lock(void)
static void falconide_get_lock(irq_handler_t handler, void *data)
{
if (falconide_intr_lock == 0) {
- if (in_interrupt() > 0)
- panic("Falcon IDE hasn't ST-DMA lock in interrupt");
stdma_lock(handler, data);
falconide_intr_lock = 1;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/2] ide/Falcon: Remove in_interrupt() usage.
2020-11-20 9:24 ` [PATCH v2 1/2] ide/Falcon: Remove in_interrupt() usage Sebastian Andrzej Siewior
@ 2020-11-20 22:35 ` Andrew Morton
2020-11-21 12:44 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2020-11-20 22:35 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: linux-ide, linux-kernel, David S. Miller, Jens Axboe, tglx
On Fri, 20 Nov 2020 10:24:20 +0100 Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> falconide_get_lock() is called by ide_lock_host() and its caller
> (ide_issue_rq()) has already a might_sleep() check.
>
> stdma_lock() has wait_event() which also has a might_sleep() check.
>
> Remove the in_interrupt() check.
>
> ...
>
> --- a/drivers/ide/falconide.c
> +++ b/drivers/ide/falconide.c
> @@ -51,8 +51,6 @@ static void falconide_release_lock(void)
> static void falconide_get_lock(irq_handler_t handler, void *data)
> {
> if (falconide_intr_lock == 0) {
> - if (in_interrupt() > 0)
> - panic("Falcon IDE hasn't ST-DMA lock in interrupt");
> stdma_lock(handler, data);
> falconide_intr_lock = 1;
> }
The current mainline falconide_get_lock() is very different:
static void falconide_release_lock(void)
{
if (falconide_intr_lock == 0) {
printk(KERN_ERR "%s: bug\n", __func__);
return;
}
falconide_intr_lock = 0;
stdma_release();
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/2] ide/Falcon: Remove in_interrupt() usage.
2020-11-20 22:35 ` Andrew Morton
@ 2020-11-21 12:44 ` Sebastian Andrzej Siewior
2020-11-24 3:46 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2020-11-21 12:44 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-ide, linux-kernel, David S. Miller, Jens Axboe, tglx
On 2020-11-20 14:35:35 [-0800], Andrew Morton wrote:
> On Fri, 20 Nov 2020 10:24:20 +0100 Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
>
> > falconide_get_lock() is called by ide_lock_host() and its caller
> > (ide_issue_rq()) has already a might_sleep() check.
> >
> > stdma_lock() has wait_event() which also has a might_sleep() check.
> >
> > Remove the in_interrupt() check.
> >
> > ...
> >
> > --- a/drivers/ide/falconide.c
> > +++ b/drivers/ide/falconide.c
> > @@ -51,8 +51,6 @@ static void falconide_release_lock(void)
> > static void falconide_get_lock(irq_handler_t handler, void *data)
> > {
> > if (falconide_intr_lock == 0) {
> > - if (in_interrupt() > 0)
> > - panic("Falcon IDE hasn't ST-DMA lock in interrupt");
> > stdma_lock(handler, data);
> > falconide_intr_lock = 1;
> > }
>
> The current mainline falconide_get_lock() is very different:
I have this patch on-top of next-20201120 so it should apply. You
realize that the above hunk is against falconide_get_lock() while
the below is falconide_release_lock().
If there is something wrong with the patch (or its commit message) I'm
sorry but I don't understand your signal :)
> static void falconide_release_lock(void)
> {
> if (falconide_intr_lock == 0) {
> printk(KERN_ERR "%s: bug\n", __func__);
> return;
> }
> falconide_intr_lock = 0;
> stdma_release();
> }
Sebastian
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/2] ide/Falcon: Remove in_interrupt() usage.
2020-11-21 12:44 ` Sebastian Andrzej Siewior
@ 2020-11-24 3:46 ` Andrew Morton
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2020-11-24 3:46 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: linux-ide, linux-kernel, David S. Miller, Jens Axboe, tglx
On Sat, 21 Nov 2020 13:44:24 +0100 Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> On 2020-11-20 14:35:35 [-0800], Andrew Morton wrote:
> > On Fri, 20 Nov 2020 10:24:20 +0100 Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> >
> > > falconide_get_lock() is called by ide_lock_host() and its caller
> > > (ide_issue_rq()) has already a might_sleep() check.
> > >
> > > stdma_lock() has wait_event() which also has a might_sleep() check.
> > >
> > > Remove the in_interrupt() check.
> > >
> > > ...
> > >
> > > --- a/drivers/ide/falconide.c
> > > +++ b/drivers/ide/falconide.c
> > > @@ -51,8 +51,6 @@ static void falconide_release_lock(void)
> > > static void falconide_get_lock(irq_handler_t handler, void *data)
> > > {
> > > if (falconide_intr_lock == 0) {
> > > - if (in_interrupt() > 0)
> > > - panic("Falcon IDE hasn't ST-DMA lock in interrupt");
> > > stdma_lock(handler, data);
> > > falconide_intr_lock = 1;
> > > }
> >
> > The current mainline falconide_get_lock() is very different:
>
> I have this patch on-top of next-20201120 so it should apply. You
> realize that the above hunk is against falconide_get_lock() while
> the below is falconide_release_lock().
> If there is something wrong with the patch (or its commit message) I'm
> sorry but I don't understand your signal :)
>
oops, sorry, the MIME-encoded email messed me up, then I went and
confused myself. Got it now, thanks ;)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] ide: Remove BUG_ON(in_interrupt() || irqs_disabled()) from ide_unregister()
2020-11-20 9:24 [PATCH v2 0/2] ide: Remove in_interrupt() Sebastian Andrzej Siewior
2020-11-20 9:24 ` [PATCH v2 1/2] ide/Falcon: Remove in_interrupt() usage Sebastian Andrzej Siewior
@ 2020-11-20 9:24 ` Sebastian Andrzej Siewior
1 sibling, 0 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2020-11-20 9:24 UTC (permalink / raw)
To: linux-ide
Cc: linux-kernel, David S. Miller, Jens Axboe, Andrew Morton, tglx,
Sebastian Andrzej Siewior
Both BUG_ON() were introduced in commit
4015c949fb465 ("[PATCH] update ide core")
when ide_unregister() was extended with semaphore based locking. Both
checks won't complain about disabled preemption which is also wrong.
The might_sleep() in today's mutex_lock() will complain about the
missuses.
Remove the BUG_ON() statements.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-ide@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Jens Axboe <axboe@kernel.dk>
---
drivers/ide/ide-probe.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
index 1c1567bb51942..aefd74c0d8628 100644
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -1539,9 +1539,6 @@ EXPORT_SYMBOL_GPL(ide_port_unregister_devices);
static void ide_unregister(ide_hwif_t *hwif)
{
- BUG_ON(in_interrupt());
- BUG_ON(irqs_disabled());
-
mutex_lock(&ide_cfg_mtx);
if (hwif->present) {
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-24 3:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-20 9:24 [PATCH v2 0/2] ide: Remove in_interrupt() Sebastian Andrzej Siewior
2020-11-20 9:24 ` [PATCH v2 1/2] ide/Falcon: Remove in_interrupt() usage Sebastian Andrzej Siewior
2020-11-20 22:35 ` Andrew Morton
2020-11-21 12:44 ` Sebastian Andrzej Siewior
2020-11-24 3:46 ` Andrew Morton
2020-11-20 9:24 ` [PATCH v2 2/2] ide: Remove BUG_ON(in_interrupt() || irqs_disabled()) from ide_unregister() Sebastian Andrzej Siewior
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox