Re: [SCSI] fix scsi_reap_target() device_del from atomic context

[Sergey Vlasov <vsu@altlinux.ru>]

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

On Fri, Dec 23, 2005 at 02:53:39PM -0600, James Bottomley wrote:
> On Fri, 2005-12-23 at 23:43 +0300, Sergey Vlasov wrote:
> > So if that GFP_ATOMIC allocation ever fails, the target is leaked
> > forever - does not look nice.
> 
> Yes, but it will print a warning.
> 
> > Does anything depend on starget->siblings being empty after we had
> > removed it from the shost->__targets list it was on?  Seems that all
> > other uses of this field are:
> >
> > drivers/scsi/scsi_scan.c:313:   list_for_each_entry(starget, &shost->__targets, siblings) {
> > drivers/scsi/scsi_scan.c:365:   INIT_LIST_HEAD(&starget->siblings);
> > drivers/scsi/scsi_scan.c:373:   list_add_tail(&starget->siblings, &shost->__targets);
> > 
> > So probably we can reuse this field and do deferred reaping without any
> > memory allocation at all.  The following patch should be applied
> > _instead_ of the James' patch, not on top of it (I can make a combined
> > patch if it is desired).  The difference with the previous patch is that
> > scsi_target_reap() still removes the target from shost->__targets
> > immediately - only device_del() and subsequent actions are deferred to a
> > workqueue.
> 
> No, you can't.
> 
> If you do this, the target namespace will potentially be in use in sysfs
> after the system thinks the target is gone.  Thus, any reallocation
> fails because you can't add a new target with the same name as an
> existing one.

Ah, I see now... scsi_alloc_target() checks if a target with the same
channel/id combination is on the list and adds new target only if
there was no such one before.

However, what prevents a race between scsi_target_reap_work() and
scsi_alloc_target()?  If the worker thread is interrupted/preempted
just after releasing host_lock (when it has already removed the target
from the list, but before it has called device_del()),
scsi_alloc_target() might consider the target as new and get to
device_add() faster.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]