Re: Kernel hangs when powering up/down drive using sysfs

[Lukas Wunner <lukas@wunner.de>]

On Mon, Mar 16, 2020 at 11:42:00AM -0700, Keith Busch wrote:
> On Mon, Mar 16, 2020 at 07:10:58PM +0100, Lukas Wunner wrote:
> > On Mon, Mar 16, 2020 at 09:15:43AM -0700, Keith Busch wrote:
> > > I'm not sure why the hard-irq context is even setting the thread running
> > > flag while it can still exit without handling anything. Shouldn't it
> > > leave the flag cleared until knows it's actually going to do something?
> > 
> > No, ist_running must be set to true before the invocation of
> > atomic_xchg(&ctrl->pending_events, 0).
> 
> Okay, I see what you mean.
> 
> Even with David's patch, there's still another condition that could exit
> with ist_running set. It may make sense to move the setting just above
> the atomic_xchg() so that clearing it doesn't need to be duplicated for
> the remaining uncommon exit case.

Right, or introduce a goto label before the teardown code at the end of
pciehp_ist() and replace the return statements with gotos.  I've compared
both approaches and the goto solution required the least lines of code
and subjectively looked the nicest, so I went with that.


> > There's a time window between the atomic_xchg() and actually
> > turning off the slot when pending_events is 0.  Previously we
> > only checked in the sysfs functions that pending_events is 0.
> > That was insufficient as we risked returning prematurely from
> > the sysfs functions.  The point of ist_running is to prevent
> > that.
> 
> Oh, right, I'm remembering now. And since we've a polled option for
> pciehp, using synchronize_irq() from the sysfs path isn't possible.

Precisely. :-)

Thanks,

Lukas