On Tue, 23 Feb 2021, Song Bao Hua (Barry Song) wrote:
Regarding m68k, your analysis overlooks the timing issue. E.g. patch
11/32 could be a problem because removing the irqsave would allow PDMA
transfers to be interrupted. Aside from the timing issues, I agree
with your analysis above regarding m68k.
You mentioned you need realtime so you want an interrupt to be able to
preempt another one.
That's not what I said. But for the sake of discussion, yes, I do know
people who run Linux on ARM hardware (if Android vendor kernels can be
called "Linux") and who would benefit from realtime support on those
devices.
Now you said you want an interrupt not to be preempted as it will make a
timing issue.
mac_esp deliberately constrains segment sizes so that it can harmlessly
disable interrupts for the duration of the transfer.
Maybe the irqsave in this driver is over-cautious. Who knows? The PDMA
timing problem relates to SCSI bus signalling and the tolerance of real-
world SCSI devices to same. The other problem is that the PDMA logic
circuit is undocumented hardware. So there may be further timing
requirements lurking there. Therefore, patch 11/32 is too risky.
If this PDMA transfer will have some problem when it is preempted, I
believe we need some enhanced ways to handle this, otherwise, once we
enable preempt_rt or threaded_irq, it will get the timing issue. so here
it needs a clear comment and IRQF_NO_THREAD if this is the case.
People who require fast response times cannot expect random drivers or
platforms to meet such requirements. I fear you may be asking too much
from Mac Quadra machines.
With regard to other architectures and platforms, in specific cases,
e.g. where there's never more than one IRQ involved, then I could
agree that your assumptions probably hold and an irqsave would be
probably redundant.
When you find a redundant irqsave, to actually patch it would bring a
risk of regression with little or no reward. It's not my place to veto
this entire patch series on that basis but IMO this kind of churn is
misguided.
Nope.
I would say the real misguidance is that the code adds one lock while it
doesn't need the lock. Easily we can add redundant locks or exaggerate
the coverage range of locks, but the smarter way is that people add
locks only when they really need the lock by considering concurrency and
realtime performance.
You appear to be debating a strawman. No-one is advocating excessive
locking in new code.
Thanks
Barry
