7966 sd attach should not issue potentially slow scsi commands

Review Request #399 — Created March 13, 2017 and discarded

yuripv
illumos-gate
master
7966
general

Work by Hans Rosenfeld (mostly).

Move everything requiring sending SCSI commands to device out of initial attach routine and do the remaining work using taskq.

As we had the PM completely disabled in sd, I have fixed the PM paths as well for upstreaming. As we need to have pm-components available when finishing attach, install the initial property having all states for both "power conditition supported" and "start/stop" cases in sdattach(). Remap the "start/stop" power levels to the "power condition supported" ones, and update the property afterwards if needed in sd_setup_pm()->sd_update_pm_components(), so now a "start/stop" would look like the following:

name='pm-components' type=string items=3 dev=none
    value='NAME=spindle-motor' + '0=stopped' + '3=active'

This have been in production for quite some time.

I've retested the power stuff after having fixed it on a bunch of systems, where drives are not pm-capable at all, support only start/stop, and the desktop ones supporting power conditition.

  • 1
  • 0
  • 0
  • 3
  • 4
Description From Last Updated
What is 'smart probing'? rm rm
yuripv
rm
  1. Can you clarify how a failure to attach in the taskq will lead to the memory and related device nodes being cleaned up? It feels like we'll need something to call detach, but blocking something forcing that, it'll just sit around. Is that right? I also, more generally, am not sure about all the power state changes. You said that you generally have tested all this with power management disabled, is that right? Do you have any idea of what the impact of that kind of overarching change is on something like this: https://github.com/joyent/illumos-joyent/commit/b380d4fce9c309a568bb92531d552f1cc69d985b which I was getting ready to upstream?

    1. Please do uupstream your change.

  2. usr/src/uts/common/io/scsi/targets/sd.c (Diff revision 1)
     
     
    What is 'smart probing'?
  3. usr/src/uts/common/io/scsi/targets/sd.c (Diff revision 1)
     
     
    Is there a reason that this comment has been deleted, it still seems valid.
    1. Read below, please.

  4. usr/src/uts/common/io/scsi/targets/sd.c (Diff revision 1)
     
     
    Why is this now only being run if the reservation flag is set? It didn't used to.
    1. You are reading it wrong, it's now ran 2 times, once with the flag set, so we don't issue commands, and otherwise doing everything we need.

  5. usr/src/uts/common/io/scsi/targets/sd.c (Diff revision 1)
     
     
    Is there a reason why everything has changed to SD_SPINDLE_ACTIVE here?
    1. Yes, I did describe it above (as a part of general change).

  6. 
      
yuripv
Review request changed

Status: Discarded

yuripv
  1. Closing this as Hans is now working for Joyent and will be able to upstream and answer all question better than me.

  2. 
      
andy_js
  1. I reviewed these changes months ago and completely forgot about them. Sorry! To me they look good, but please address others' concerns first.

  2. 
      
Loading...