Commit Graph

10 Commits

Author SHA1 Message Date
Kenneth D. Merry
86d45c7f3b Fix a device departure bug for the the pass(4), enc(4), sg(4) and ch(4)
drivers.

The bug occurrs when a userland process has the driver instance
open and the underlying device goes away.  We get the devfs
callback that the device node has been destroyed, but not all of
the closes necessary to fully decrement the reference count on the
CAM peripheral.

The reason is that once devfs calls back and says the device has
been destroyed, it is moved off to deadfs, and devfs guarantees
that there will be no more open or close calls.  So the solution
is to keep track of how many outstanding open calls there are on
the device, and just release that many references when we get the
callback from devfs.

scsi_pass.c,
scsi_enc.c,
scsi_enc_internal.h:	Add an open count to the softc in these
			drivers.  Increment it on open and
			decrement it on close.

			When we get a devfs callback to say that
			the device node has gone away, decrement
			the peripheral reference count by the
			number of still outstanding opens.

			Make sure we don't access the peripheral
			with cam_periph_unlock() after what might
			be the final call to
			cam_periph_release_locked().  The
			peripheral might have been freed, and we
			will be dereferencing freed memory.

scsi_ch.c,
scsi_sg.c:		For the ch(4) and sg(4) drivers, add the
			same changes described above, and in
			addition, fix another bug that was
			previously fixed in the pass(4) and enc(4)
			drivers.

			These drivers were calling destroy_dev()
			from their cleanup routine, but that could
			cause a deadlock because the cleanup
			routine could be indirectly called from
			the driver's close routine.  This would
			cause a deadlock, because the device node
			is being held open by the active close
			call, and can't be destroyed.

Sponsored by:	Spectra Logic Corporation
MFC after:	1 week
2012-12-08 04:03:04 +00:00
Alexander Motin
e1c2df4d30 Remove one more numeric priority constant. 2012-10-27 08:52:33 +00:00
Alexander Motin
5fb9dc04df Remove two more 'periph == NULL' checks missed in r241404.
This condition can never be true as functions are called from single place
and the checks just pollute the code and confuse Clang Static Analyzer.
2012-10-23 16:03:00 +00:00
Eitan Adler
0af1b47258 s/ is is / is /g
s/ a a / a /g

Approved by:	cperciva
MFC after:	3 days
2012-09-14 22:00:03 +00:00
Matt Jacob
5e6609a21a 1. Remove SEN support. I doubt there are any working examples
of this hardware still running (close to twenty years now).

2. Quiesece and use ENC_VLOG instead of ENC_LOG for most
complaints. That is, they're visible with bootverbose, but
otherwise quiesced and not repeatedly spamming messages
with constant reminders that hardware in this space is
rarely fully compliant.

MFC after:	1 month
2012-08-12 17:01:07 +00:00
Bjoern A. Zeeb
caf144a280 Remove opt_enc.h from files committed with r235911. enc(4) is the
'encapsulating interface' used with IPsec and has nothing to do with
storage 'enclosure' services.

MFC after:	3 days
Noticed while:	debugging why enc(4) is no longer automatically created
2012-07-30 03:00:58 +00:00
Kenneth D. Merry
ea37f51942 Fix several reference counting and object lifetime issues between
the pass(4) and enc(4) drivers and devfs.

The pass(4) driver uses the destroy_dev_sched() routine to
schedule its device node for destruction in a separate thread
context.  It does this because the passcleanup() routine can get
called indirectly from the passclose() routine, and that would
cause a deadlock if the close routine tried to destroy its own
device node.

In any case, once a particular passthrough driver number, e.g.
pass3, is destroyed, CAM considers that unit number (3 in this
case) available for reuse.

The problem is that devfs may not be done cleaning up the previous
instance of pass3, and will panic if isn't done cleaning up the
previous instance.

The solution is to get a callback from devfs when the device node
is removed, and make sure we hold a reference to the peripheral
until that happens.

Testing exposed some other cases where we have reference counting
issues, and those were also fixed in the pass(4) driver.

cam_periph.c:	In camperiphfree(), reorder some of the operations.

		The peripheral destructor needs to be called before
		the peripheral is removed from the peripheral is
		removed from the list.  This is because once we
		remove the peripheral from the list, and drop the
		topology lock, the peripheral number may be reused.
		But if the destructor hasn't been called yet, there
		may still be resources hanging around (like devfs
		nodes) that haven't been fully cleaned up.

cam_xpt.c:	Add an argument to xpt_remove_periph() to indicate
		whether the topology lock is already held.

scsi_enc.c:	Acquire an extra reference to the peripheral during
		registration, and release it once we get a callback
		from devfs indicating that the device node is gone.

		Call destroy_dev_sched_cb() in enc_oninvalidate()
		instead of calling destroy_dev() in the cleanup
		routine.

scsi_pass.c:	Add reference counting to handle peripheral and
		devfs object lifetime issues.

		Add a reference to the peripheral and the devfs
		node in the peripheral registration.

		Don't attempt to add a physical path alias if the
		peripheral has been marked invalid.

		Release the devfs reference once the initial
		physical path alias taskqueue run has completed.

		Schedule devfs node destruction in the
		passoninvalidate(), and release our peripheral
		reference in a new routine, passdevgonecb() once
		the devfs node is gone.  This allows the peripheral
		to fully go away, and the peripheral destructor,
		passcleanup(), will get called.

MFC after:	3 days
Sponsored by:	Spectra Logic
2012-06-20 17:08:00 +00:00
Kenneth D. Merry
c552ebe12d Work around a race condition in devfs by changing the way closes
are handled in most CAM peripheral drivers that are not handled by
GEOM's disk class.

The usual character driver open and close semantics are that the
driver gets N open calls, but only one close, when the last caller
closes the device.

CAM peripheral drivers expect that behavior to be honored to the
letter, and the CAM peripheral driver code (specifically
cam_periph_release_locked_busses()) panics if it is done incorrectly.

Since devfs has to drop its locks while it calls a driver's close
routine, and it does not have a way to delay or prevent open calls
while it is calling the close routine, there is a race.

The sequence of events, simplified a bit, is:

- devfs acquires a lock
- devfs checks the reference count, and if it is 1, continues to close.
- devfs releases the lock

- 2nd process open call on the device happens here

- devfs calls the driver's close routine

- devfs acquires a lock
- devfs decrements the reference count
- devfs releases the lock

- 2nd process close call on the device happens here

At the second close, we get a panic in
cam_periph_release_locked_busses(), complaining that peripheral
has been released when the reference count is already 0.  This is
because we have gotten two closes in a row, which should not
happen.

The fix is to add the D_TRACKCLOSE flag to the driver's cdevsw, so
that we get a close() call for each open().  That does happen
reliably, so we can make sure that our reference counts are
correct.

Note that the sa(4) and pt(4) drivers only allow one context
through the open routine.  So these drivers aren't exposed to the
same race condition.

scsi_ch.c,
scsi_enc.c,
scsi_enc_internal.h,
scsi_pass.c,
scsi_sg.c:
		For these drivers, change the open() routine to
		increment the reference count for every open, and
		just decrement the reference count in the close.

		Call cam_periph_release_locked() in some scenarios
		to avoid additional lock and unlock calls.

scsi_pt.c:	Call cam_periph_release_locked() in some scenarios
		to avoid additional lock and unlock calls.

MFC after:	3 days
2012-05-27 06:11:09 +00:00
Alexander Motin
13cd92ccdb Remove sleep() from invalidate call in ses driver, waiting for daemon
process exit. Instead use CAM's standard reference counting to prevent
periph going away until process won't complete. I think that sleep in
single CAM SWI thread is not a good idea and may lead to deadlocks if
daemon process waits for some command completion. Combined with recent
patch avoiding use of CAM SWI for ATA it just causes panics because of
sleeps prohibited in interrupt thread context.
2012-05-25 07:57:17 +00:00
Alexander Motin
f6ad3f237a MFprojects/zfsd:
Revamp the CAM enclosure services driver.
This updated driver uses an in-kernel daemon to track state changes and
publishes physical path location information\for disk elements into the
CAM device database.

Sponsored by:   Spectra Logic Corporation
Sponsored by:   iXsystems, Inc.
Submitted by:   gibbs, will, mav
2012-05-24 14:07:44 +00:00