On Thu 24 Sep 2020 at 19:01, Kevin Hilman [off-list ref] wrote:

Hi Brad,

Brad Harper [off-list ref] writes:

quoted

Force threaded interrupts for meson_mmc_irq to prevent possible deadlock 
condition
during mmc operations when using preempt_rt with 5.9.0-rc3-rt3 patches 
on arm64.

Using meson-gx-mmc with an emmc device on Hardkernel Odroid N2+ 
configured with
preempt_rt resulted in the soc becoming unresponsive.  With lock 
checking enabled
the below inconsistent lock state was observed during boot.

After some discussions with tglx in IRC #linux-rt the attached patch was 
suggested
to remove IRQF_ONESHOT from request_threaded_irq.
This has been tested and confirmed by me to resolve both the 
unresponsive soc and
the inconsistent lock state warning when using 5.9.0-rc3-rt3 on arm64 
Odroid N2+.

Further review and testing is required to ensure there are no adverse 
impacts or
concerns and that is the correct method to resolve the problem.  I will 
continue
to test on various amlogic devices with both standard mainline low 
latency kernel
and preempt_rt kernel with -rt patches.

This looks right to me, thanks for sending a fix.

For broader testing, I can add this to my testing branch so it gets
booted on a bunch more platform in KernelCI also.

However...

[...]

quoted

Signed-off-by: Brad Harper <redacted>
---
  drivers/mmc/host/meson-gx-mmc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c 

b/drivers/mmc/host/meson-gx-mmc.c
index 08a3b1c05..130ac134d 100644

--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c

@@ -1139,7 +1139,7 @@ static int meson_mmc_probe(struct platform_device 

*pdev)
                host->regs + SD_EMMC_IRQ_EN);

         ret = request_threaded_irq(host->irq, meson_mmc_irq,
-                                  meson_mmc_irq_thread, IRQF_ONESHOT,
+                                  meson_mmc_irq_thread, 0,
                                    dev_name(&pdev->dev), host);

I'm not sure about this.
As you have explained on IRC, I understand that IRQF_ONESHOT is causing
trouble with RT as the hard IRQ part of the thread will not be migrated
to a thread. That was certainly not the intent when putting this flag.

As described in include/linux/interrupt.h:
 * IRQF_ONESHOT - Interrupt is not reenabled after the hardirq handler finished.
 *                Used by threaded interrupts which need to keep the
 *                irq line disabled until the threaded handler has been run.

The intent was only that, making sure the irq is not re-enabled until
the threaded part is done. AFAIU, removing this flag would allow the
hard irq handler to fire again while we are not done handling the
previous IRQ in threaded part.

This seems pretty unsafe to me. Maybe we could improve the driver so it
copes with this case gracefully. ATM, I don't think it would.

Maybe I missed something and I am happy to be enlightened if that is the
case :)

quoted

         if (ret)
                 goto err_init_clk;

This patch has been mangled by your mailer, so it doesn't apply cleanly.
If you're using the gmail web UI, this is a common problem.

I strongly recommend using git-send-email to send directly via gmail
SMTP.  The git-send-email docs[1] give some examples on how to set this
up.

Kevin

[1] https://git-scm.com/docs/git-send-email#_examples

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel