On Thu, 26 Nov 2020, Sebastian Andrzej Siewior wrote:
From: "Ahmed S. Darwish" <a.darwish@xxxxxxxxxxxxx>
NCR5380_poll_politely2() uses in_interrupt() to check if it is safe to
sleep.
The usage of in_interrupt() in drivers is phased out and Linus clearly
requested that code which changes behaviour depending on context should
either be separated, or the context be explicitly conveyed in an
argument passed by the caller.
Below is a context analysis of NCR5380_poll_politely2() uppermost
callers:
- NCR5380_maybe_reset_bus(), task, invoked during device probe.
-> NCR5380_poll_politely()
-> do_abort()
- NCR5380_select(), task, but can only sleep in the "release, then
re-acquire" regions of the spinlock held by its caller.
Sleeping invocations (lock released):
-> NCR5380_poll_politely2()
Atomic invocations (lock acquired):
-> NCR5380_reselect()
-> NCR5380_poll_politely()
-> do_abort()
-> NCR5380_transfer_pio()
- NCR5380_intr(), interrupt handler
-> NCR5380_dma_complete()
-> NCR5380_transfer_pio()
-> NCR5380_poll_politely()
-> NCR5380_reselect() (see above)
- NCR5380_information_transfer(), task, but can only sleep in the
"release, then re-acquire" regions of the caller-held spinlock.
Sleeping invocations (lock released):
- NCR5380_transfer_pio() -> NCR5380_poll_politely()
- NCR5380_poll_politely()
Atomic invocations (lock acquired):
- NCR5380_transfer_dma()
-> NCR5380_dma_recv_setup()
=> generic_NCR5380_precv() -> NCR5380_poll_politely()
=> macscsi_pread() -> NCR5380_poll_politely()
-> NCR5380_dma_send_setup()
=> generic_NCR5380_psend -> NCR5380_poll_politely2()
=> macscsi_pwrite() -> NCR5380_poll_politely()
-> NCR5380_poll_politely2()
-> NCR5380_dma_complete()
-> NCR5380_transfer_pio()
-> NCR5380_poll_politely()
- NCR5380_transfer_pio() -> NCR5380_poll_politely
- NCR5380_reselect(), atomic, always called with hostdata spinlock
held.
If direct callers are purely atomic, or purely task context, change
their specifications accordingly and mark them with "Context: " tags.
For the mixed ones, trickle-down context from upper layers.
Signed-off-by: Ahmed S. Darwish <a.darwish@xxxxxxxxxxxxx>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
Acked-by: Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx>
@@ -513,9 +513,11 @@ static void wait_for_53c80_access(struct NCR5380_hostdata *hostdata)
* @dst: buffer to write into
* @len: transfer size
*
+ * Context: atomic. This implements NCR5380.c NCR5380_dma_recv_setup(),
+ * which is always called with @hostdata spinlock held.
+ *
* Perform a pseudo DMA mode receive from a 53C400 or equivalent device.
*/
-
static inline int generic_NCR5380_precv(struct NCR5380_hostdata *hostdata,
unsigned char *dst, int len)
{
BTW, if I was doing this, I'd omit all of the many gratuitous whitespace
changes and I'd avoid copying so much program logic into the comments
where it is redundant -- here I'd have written only "Context: atomic,
spinlock held". However, no need to revise. Looks fine otherwise.
