On Sat, 28 Nov 2020, Ahmed S. Darwish wrote: > On Sat, Nov 28, 2020 at 08:48:00AM +1100, Finn Thain wrote: > > > > On Sat, 28 Nov 2020, Finn Thain wrote: > > > > > > > > diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c > > > index d654a6cc4162..739def70cffb 100644 > > > --- a/drivers/scsi/NCR5380.c > > > +++ b/drivers/scsi/NCR5380.c > > > @@ -223,7 +223,10 @@ static int NCR5380_poll_politely2(struct NCR5380_hostdata *hostdata, > > > cpu_relax(); > > > } while (n--); > > > > > > - if (irqs_disabled() || in_interrupt()) > > > + /* We can't sleep when local irqs are disabled and callers ensure > > > + * that local irqs are disabled whenever we can't sleep. > > > + */ > > > + if (irqs_disabled()) > > > return -ETIMEDOUT; > > > > > > /* Repeatedly sleep for 1 ms until deadline */ > > > > > > > Michael, Andreas, would you please confirm that this is workable on Atari? > > The driver could sleep when IPL == 2 because arch_irqs_disabled_flags() > > would return false (on Atari). I'm wondering whether that would deadlock. > > Please re-check the commit log: > > "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." > Yes, I knew about the discussion around the issues with preempt_count() and CONFIG_PREEMPT. And I don't have any problem with removing in_interrupt(), as you can see from my patch. > So, sorry, drivers shouldn't do context-dependent dances anymore. > I don't know what is meant by 'context-dependent'. I suspect that it's left ill-defined because there are many cases where global state is needed, such as those mentioned in the thread you cited, like the memalloc_no*() calls. See also, in_compat_syscall(). > For more context (no pun intended), please check the thread mentioned in > the cover letter, and also below message: > > https://lkml.kernel.org/r/CAKMK7uHAk9-Vy2cof0ws=DrcD52GHiCDiyHbjLd19CgpBU2rKQ@xxxxxxxxxxxxxx > Are you also planning to remove spin_lock_irqsave/restore() and replace these with spin_lock_irq/unlock_irq()? And if not, why do you object to irqs_disabled()? Please also compare your patch and mine with regard to stack usage, readability and code size. Also consider that adding a new argument to all those functions creates new opportunities for mistakes in new callers. > Kind regards, > > -- > Ahmed S. Darwish > Linutronix GmbH >
