[Bug 219857] panic in scsi_cd code

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219857

Kenneth D. Merry <ken at FreeBSD.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|New                         |Open
                 CC|                            |ken at FreeBSD.org

--- Comment #1 from Kenneth D. Merry <ken at FreeBSD.org> ---
I can see why this happened.  It is a little strange that I don't recall seeing
this before, since the media check code was added in 2003, and it appears that
sleeping has been prohibited for a similar amount of time.

The problem is:

1.  In g_io_schedule_down(), THREAD_NO_SLEEPING() is called before the start
routine is called.

2.  In cdstrategy(), if the CD_FLAG_VALID_MEDIA flag isn't set, the cd(4)
driver probes to see if there is valid media in the drive.

3.  In cdcheckmedia(), the cd(4) driver first prevents CD removal by calling
cdprevent().

4.  In cdprevent(), the cd(4) driver calls cdrunccb(), which calls
cam_periph_runccb(), which sleeps and runs into the panic.

In most cases, this won't happen because the cd(4) driver checks for media on
open(), and prevents removal when it finds media.  The driver does allow the
open() when no media is present, so that the user can call the CDIOCCLOSE and
CDIOCEJECT ioctls.

One possible solution is just failing the I/O in cdstrategy() if the
CD_FLAG_VALID_MEDIA flag is not set.  We would need to add code that checks for
media if the user does a CDIOCCLOSE ioctl.

The challenge there, however, is that it would not catch the case where the
user physically closes the drive while they have the device open and then sends
a read.

In order to handle that case, and not sleep underneath cdstrategy(), we would
need to do an asynchronous media check.

The way I would probably do it is to add a series of states into cdstart() and
cddone() that mirror the way that cdcheckmedia() operates.  So, prevent, read
capacity, read TOC, and so on.  We would still call xpt_schedule() from
cdstrategy(), but cdstart() would go into a media probe state machine to detect
the media and once it has validated the media or not, satisfy the I/O or send
it back with an error.

As a bonus, we would re-implement the existing code so that it can work either
asynchronously or synchronously.  (To avoid duplicate code paths.)

-- 
You are receiving this mail because:
You are the assignee for the bug.