Re: [PATCH] scsi: check for device state in __scsi_remove_target()

["Ewan D. Milne" <emilne@redhat.com>]

On Thu, 2017-12-14 at 10:02 +0100, Hannes Reinecke wrote:
> On 12/14/2017 09:05 AM, Jason Yan wrote:
> > 
> > On 2017/12/14 6:23, Bart Van Assche wrote:
> >> On Wed, 2017-12-13 at 14:21 +0100, Hannes Reinecke wrote:
> >>> As it turned out device_get() doesn't use kref_get_unless_zero(),
> >>> so we will be always getting a device pointer.
> >>> So we need to check for the device state in __scsi_remove_target()
> >>> to avoid tripping over deleted objects.
> >>>
> >>> Fixes: fbce4d9 ("scsi: fixup kernel warning during rmmod()")
> >>
> >> How about adding Reported-by: Jason Yan? See also
> >> https://www.spinics.net/lists/linux-scsi/msg115295.html
> >>
> >> Anyway:
> >>
> >> Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
> >>
> > 
> > Seems the same as my patch.So how do we plan to fix this issue,
> > pick this approach up or the approach James Bottomley suggested?
> > I have sent a patch to change get_device() but Greg seems do not
> > like this way.
> > 
> This is actually a real regression, which can be trivially exercised by
> eg logging out from two connections to an iSCSI target.
> (Our QA tripped across that one).
> So I'd rather have to have it fixed reasonably soon.
> 
> While 'get_device' is IMO the 'correct' solution it surely warrants a
> broader discussion, plus one would need to audit all callers to check
> the return value. If we were going down that route we should probably
> add a __must_check to get_device(), too.
> But again, this will probably drag out for quite some time, and I'd
> prefer to have the fix in the meantime.
> 
> Cheers,
> 
> Hannes

We have 2 reproducible test cases, this patch fixes one of them,
which was a continually oscillating FC target port w/short dev_loss_tmo.
I'm still waiting for a report on the iSCSI test.  The code looks good.
We need to get some kind of fix for this sooner rather than later.

Reviewed-by: Ewan D. Milne <emilne@redhat.com>