firewire/sbp: try to improve locking, plus a few style nits
authoravg <avg@FreeBSD.org>
Tue, 7 Mar 2017 16:07:52 +0000 (16:07 +0000)
committeravg <avg@FreeBSD.org>
Tue, 7 Mar 2017 16:07:52 +0000 (16:07 +0000)
commiteca08888524483e53203dc76a7cf3617fa411c83
tree745265f84b46216f96982017942ee2ced91eb25c
parent836c182d2ca3ddfa094207201d603b84132872c0
firewire/sbp: try to improve locking, plus a few style nits

This change tries to fix the most obvious locking problems.

sbp_cam_scan_lun() is never called with the sbp lock held, so the lock
needs to be acquired internally (if it's needed at all).
Without this change a kernel with INVARIANTS panics when a firewire disk
is connected:
  panic: mutex sbp not owned at /usr/src/sys/dev/firewire/sbp.c:967
  KDB: stack backtrace:
  db_trace_self_wrapper() at 0xffffffff80420bbb = db_trace_self_wrapper+0x2b/frame 0xfffffe0504df0930
  kdb_backtrace() at 0xffffffff80670359 = kdb_backtrace+0x39/frame 0xfffffe0504df09e0
  vpanic() at 0xffffffff8063986c = vpanic+0x14c/frame 0xfffffe0504df0a20
  panic() at 0xffffffff806395b3 = panic+0x43/frame 0xfffffe0504df0a80
  __mtx_assert() at 0xffffffff8061c40d = __mtx_assert+0xed/frame 0xfffffe0504df0ac0
  sbp_cam_scan_lun() at 0xffffffff80474667 = sbp_cam_scan_lun+0x37/frame 0xfffffe0504df0af0
  xpt_done_process() at 0xffffffff802aacfa = xpt_done_process+0x2da/frame 0xfffffe0504df0b30
  xpt_done_td() at 0xffffffff802ac2e5 = xpt_done_td+0xd5/frame 0xfffffe0504df0b80
  fork_exit() at 0xffffffff805ff72f = fork_exit+0xdf/frame 0xfffffe0504df0bf0
  fork_trampoline() at 0xffffffff8082483e = fork_trampoline+0xe/frame
  0xfffffe0504df0bf0
  --- trap 0, rip = 0, rsp = 0, rbp = 0 ---

Also, I tried to reduce the scope of the sbp lock to avoid holding it
while doing bus_dma allocations.

The code badly needs some re-engineering.  SBP really should implement
a CAM transport, so that it avoids control flow inversion when re-scanning
the bus.  Also, the sbp lock seems to be too coarse.

Additionally, the commit includes some changes not related to locking.

- sbp_cam_scan_lun: restore CAM_DEV_QFREEZE before re-queueing the ccb
  because xpt_setup_ccb resets ccb_h.flags
- sbp_post_busreset: call xpt_release_simq only if it's actually frozen
- don't place private SIMQ_FREEZED flag (sic, "freezed") into sim->flags,
  use sbp->flags for that
- some style fixes and control flow enhancements

Reviewed by: sbruno
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D9898
sys/dev/firewire/sbp.c