Re: [PATCH] scsi_set_host_offline (resend)

Mike Anderson
2003-03-25T18:45:30Z

James Bottomley [James.Bottomley@SteelEye.com] wrote:
> On Tue, 2003-03-25 at 04:07, Mike Anderson wrote:
>
> This isn't right:
>
> > + spin_lock_irqsave(shost->host_lock, flags);
> > + for (lh = shost->my_devices.next;
> > + (!error) && (lh != &shost->my_devices);) {
> > + sdev = list_entry(lh, struct scsi_device, siblings);
> > + scsi_device_get(sdev);
> > + spin_unlock_irqrestore(shost->host_lock, flags);
>
> You can't use the host_lock to protect the list and then drop it in the
> middle of list traversal. Doing this is bound to have repercussions for
> hotplug. I know we're a total mess for hotplug now, but I'd rather not
> add to it.
>

Probably already implied by your comment but...

I am using the lock to keep the list stable only during the traverse to
the next device which I will get a ref before dropping the lock. The
list could change during the call to the function (i.e an addition /
removal of nodes I do not have ref to).

Even if we went with a subsystem bus_for_each function I believe you are
asking for more state as previous discussed on the host offline subject
to not allow more additions. Correct?

> This problem can't be unique to SCSI, so I think what we need is
> something like a device generic function, like bus_for_each_device,
> except that it's device_for_each_child or something. where we get a
> properly ref counted and protected list traversal that will work for
> hotplugging.
>

The bus_for_each solution currently use a rw sema which I thought would
not be a good choice for these type of operations as there is a
possibility that we may want to call functions (offline) in interrupt
context.

Are you indicating that sema operations would be ok?

A possible side issue that Mochel can correct me on is that the calling
function would be restricted some as it cannot call device_register /
device_unregister as it would block.

If we want to head this direction we may need to alter the relationship
of children nodes off hosts.

-andmike
--
Michael Anderson
andmike@us.ibm.com