Re: [PATCH 12/14] scsi: NCR5380: Remove in_interrupt().

From: Finn Thain <fthain@telegraphics.com.au>
To: Michael Schmitz <schmitzmic@gmail.com>
Cc: Andreas Schwab <schwab@linux-m68k.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	linux-scsi@vger.kernel.org,
	GR-QLogic-Storage-Upstream@marvell.com,
	Hannes Reinecke <hare@kernel.org>,
	Jack Wang <jinpu.wang@cloud.ionos.com>,
	John Garry <john.garry@huawei.com>,
	linux-m68k@lists.linux-m68k.org,
	Manish Rangankar <mrangankar@marvell.com>,
	MPT-FusionLinux.pdl@broadcom.com,
	Nilesh Javali <njavali@marvell.com>,
	Sathya Prakash <sathya.prakash@broadcom.com>,
	Sreekanth Reddy <sreekanth.reddy@broadcom.com>,
	Suganath Prabu Subramani  <suganath-prabu.subramani@broadcom.com>,
	Vikram Auradkar <auradkar@google.com>,
	Xiang Chen <chenxiang66@hisilicon.com>,
	Xiaofei Tan <tanxiaofei@huawei.com>,
	"James E . J . Bottomley" <jejb@linux.ibm.com>,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Ahmed S . Darwish" <a.darwish@linutronix.de>
Subject: Re: [PATCH 12/14] scsi: NCR5380: Remove in_interrupt().
Date: Mon, 30 Nov 2020 11:15:46 +1100 (AEDT)	[thread overview]
Message-ID: <alpine.LNX.2.23.453.2011301115360.6@nippy.intranet> (raw)
In-Reply-To: <3bf3baef-ea46-a9c3-10e9-7705650d07a6@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1571 bytes --]

On Sun, 29 Nov 2020, Michael Schmitz wrote:

> Am 28.11.20 um 10:48 schrieb Finn Thain:
> > 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.
> 
> Pretty sure this would deadlock when in interrupt context here.

When in interrupt context, irqs_disabled() is true due to 
spinlock_irqsave/restore() in NCR5380_intr().

My question was really about what would happen if we sleep with IPL == 2.

> Otherwise, IPL 2 is perfectly OK (which is why 
> arch_irqs_disabled_flags() returns false in that case).
> 
> If you want to be 100% certain, I can give this one a spin.
> 

Please only test it if you think it will work.

> Cheers,
> 
>     Michael
> 
> 