Re: [PATCH] scsi_set_host_offline (resend)

Matthew Dharm
2003-04-02T07:42:10Z

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

On Mon, Mar 31, 2003 at 06:48:06PM -0800, Mike Anderson wrote:
> Matthew / James I believe I am out of sync with how much is being done
> by userspace and how much is being done in the kernel. I believed there
> was always userspace action to umount / close devices.

I have no objection to userspace being notified -- I only object to
requiring userspace to complete the unplug operation.

> Matthew I know in the previous mail you wrote a numbered list of steps.
> I believe item #2 should have called scsi_set_host_offline or
> scsi_set_device_offline. Item #6 would only happen if scsi_remove_host
> was called sometime prior.

The assumption was that #5 was the call to scsi_remove_host() -- the only
question in my mind is: How do I know it is 'safe' to call that?

> I wrote the following text on the interface I thought we had / where
> trying to complete. I would like to add to this so that we possibly
> could archive an interface contract between the LLDD and scsi-mid.

Based on my reading of the following, userspace is notified but not
required to take any action. Would that be accurate?

> 2.5 SCSI host register interfaces (scsi_add_host / scsi_remove_host)
>
> 1. LLDD call graph standard
>
> (PROBE)
> LLDD Mid Sysfs
> scsi_register -->
> scsi_set_device --> (depreciated)
> scsi_add_host ---+
> |
> scsi_sysfs_add_host --> device_register (shost)
> scsi_scan_host (i)-> device_register (sdev)

This is how I see it (as an LLDD guy) -- I set up my internal state, then
call scsi_register() followed by scsi_add_host().

Then the world starts, and I just keep processing commands whenever I get
one. Then, someone yanks my plug, and:

> (REMOVE)
> LLDD Mid Sysfs
> scsi_remove_host ---+
> |
> scsi_sysfs_remove_host --> device_unregister (shost)
> scsi_forget_host (i)-> device_unregister (sdev)

I presume that I need to offline each device (sdev) before calling
scsi_remove_host(), as previously discussed? That isn't shown here...

> (hotplug "remove" events for shost and sdev where generated by
> device_unregister call).

Yes. Userspace is notified, but not required to take any action.

> (sdev device ref cnt == 0)
> koject_cleanup
> |
> scsi_device_release

Good. I presume that scsi_device_release() will decrement the ref cnt on
the shost device?

> (shost device ref cnt == 0)
> koject_cleanup
> |
> +-- scsi_host_release
> |
> shost->hostt->release

Good. release() then is responsible for cleaning up.

> 2. LLDD call graph offline event
>
> (OFFLINE single device)
> LLDD Mid Sysfs
> scsi_set_device_offline --+
> |
> scsi_eh_scmd_add
>
> Note: Do we need hotplug events here or wait until remove???

Good question. Here's a better one: Why do we separately offline the
device before removal? Why doesn't scsi_remove_host() do it all for us?

For example, when we unload a USB HCD driver, disconnect() gets called for
each device that was on that bus.

> (OFFLINE host)
> LLDD Mid Sysfs
> scsi_set_host_offline --+
> |
> scsi_set_device_offline
>
> (REMOVE)
> Same as above.

I presume that scsi_set_device_offline in this picture leads to
scsi_set_device_offline(), which leads to scsi_eh_scmd_add()?

> TODO (1):
> scsi_add_host scans the host and when outstanding patches are applied
> the scsi_host struct device will have a ref count of 1+N where N is the
> number of struct scsi_device's found off the host. In turn each struct
> scsi_device struct device will have a ref count of 1+N where N is the
> number of upper drivers bound or the number of opens (this may not be
> correct for sometime as we do not have an option for the multiple bind
> sg issue).

Okay.... I think we're on the same page. It appears that you're going to
require me to keep resources around for quite some time, until everything
closes/umounts -- but, since every device is offline, I don't have to do
any command processing, yes? That's good, because then I can free some
significant resources....

> TODO (2):
> We need a common method to stop gets to a struct device post a certain
> barrier. In the short term scsi could add a scsi_host flag to stop scan
> adds, but cannot effect sysfs gets.

Hrm... good point. How do other busses do this?

> TODO (3):
> When outstanding patches are applied scsi_remove_host will cause a
> device_unregister to be called on all child nodes (sdevs) of scsi_host
> and scsi_host (these unregisters will generate hot plug "remove" events)
> once the scsi_device ref cnt goes to zero scsi_device_release is called
> and once scsi_host ref cnt goes to zero scsi_host_release is called.

Sounds good to me.

Matt

--
Matthew Dharm Home: mdharm-usb@one-eyed-alien.net
Maintainer, Linux USB Mass Storage Driver

My mother not mind to die for stoppink Windows NT! She is rememberink
Stalin!
-- Pitr
User Friendly, 9/6/1998

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