-----Original Message-----
From: Finn Thain [mailto:fthain@xxxxxxxxxxxxxxxxxxx]
Sent: Wednesday, February 24, 2021 6:21 PM
To: Song Bao Hua (Barry Song) <song.bao.hua@xxxxxxxxxxxxx>
Cc: tanxiaofei <tanxiaofei@xxxxxxxxxx>; jejb@xxxxxxxxxxxxx;
martin.petersen@xxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx;
linux-kernel@xxxxxxxxxxxxxxx; linuxarm@xxxxxxxxxxxxx;
linux-m68k@xxxxxxxxxxxxxxx
Subject: [Linuxarm] Re: [PATCH for-next 00/32] spin lock usage optimization
for SCSI drivers
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.
Realtime requirement is definitely a true requirement on ARM Linux.
I once talked/worked with some guys who were using ARM for realtime
system.
The feasible approaches include:
1. Dual OS(RTOS + Linux): e.g. QNX+Linux XENOMAI+Linux L4+Linux
2. preempt-rt
Which is continuously maintained like:
https://lore.kernel.org/lkml/20210218201041.65fknr7bdplwqbez@xxxxxxxxxxxxx/
3. bootargs isolcpus=
to isolate a cpu for a specific realtime task or interrupt
https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux_for_real_time/7/html/tuning_guide/isolating_cpus_using_tuned-profiles-realtime
4. ARM FIQ which has separate fiq API, an example in fsl sound:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/sound/soc/fsl/imx-pcm-fiq.c
5. Let one core invisible to Linux
Running non-os system and rtos on the core
Honestly, I've never seen anyone who depends on irq priority to support
realtime in ARM Linux though ARM's RTOS-es use it quite commonly.
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.
Once preempt_rt is enabled, those who want a fast irq environment need
a no_thread flag, or need to set its irq thread to higher sched_fifo/rr
priority.
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.
I actually meant most irqsave(s) in hardirq were added carelessly.
When irq and threads could access same data, people added irqsave
in threads, that is perfectly good as it could block irq. But
people were likely to put an irqsave in irq without any thinking.
We do have some drivers which are doing that with a clear intention
as your sonic_interrupt(), but I bet most were done aimlessly.
Anyway, the debate is long enough, let's move to some more important
things. I appreciate that you shared a lot of knowledge of m68k.
Thanks
Barry
