Date: Tue, 13 Jun 2017 21:32:50 +0000 From: bugzilla-noreply@freebsd.org To: freebsd-scsi@FreeBSD.org Subject: [Bug 219857] panic in scsi_cd code Message-ID: <bug-219857-5312-VCK5a0zRXT@https.bugs.freebsd.org/bugzilla/> In-Reply-To: <bug-219857-5312@https.bugs.freebsd.org/bugzilla/> References: <bug-219857-5312@https.bugs.freebsd.org/bugzilla/>
next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D219857 Kenneth D. Merry <ken@FreeBSD.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|New |Open CC| |ken@FreeBSD.org --- Comment #1 from Kenneth D. Merry <ken@FreeBSD.org> --- I can see why this happened. It is a little strange that I don't recall se= eing this before, since the media check code was added in 2003, and it appears t= hat 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 a= nd 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 s= ends a read. In order to handle that case, and not sleep underneath cdstrategy(), we wou= ld 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, re= ad 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 de= tect the media and once it has validated the media or not, satisfy the I/O or se= nd it back with an error. As a bonus, we would re-implement the existing code so that it can work eit= her asynchronously or synchronously. (To avoid duplicate code paths.) --=20 You are receiving this mail because: You are the assignee for the bug.=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-219857-5312-VCK5a0zRXT>