Date: Mon, 02 Feb 2015 14:20:16 +0100
Message-ID: <s5hmw4wfo1b.wl-tiwai@suse.de>
From: Takashi Iwai <tiwai@suse.de>
To: Jens Axboe <axboe@kernel.dk>
Cc: Kay Sievers <kay@vrfy.org>, Oliver Neukum <oneukum@suse.de>,
        linux-kernel@vger.kernel.org
Subject: How to fix CDROM/DVD eject mess?
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka)
 FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.4
 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3555
Lines: 88

Hi,

we've got a bug report about the mishandling of DVD/CDROM media eject
button, and it seems indeed broken since some time ago.  In short:
when the eject button is pressed, the media is forcibly ejected no
matter whether it's mounted or in use.  And, the mount remains even
after ejecting the media, eventually the kernel spews error messages
when the further access happens.

There seem many problems behind the scene.  First of all, udev tries
to lock the media unconditionally at the media insert.  This seems to
be a workaround for making DISK_EVENT_EJECT_REQUEST working.  Then,
udev unlocks the media and issues the SCSI eject ioctl unconditionally 
when DISK_EVENT_EJECT_REQUEST event is received.  Since SCSI ioctl
doesn't take the open refcount into account, it results in the
forcible eject.

(A relevant problem is that CDROM_IOCTL doesn't behave consistently;
 it checks the open refcount only for IDE.  For SCSI, it bypasses and
 gives the control directly to SCSI backend.  So, using CDROM_EJECT
 ioctl won't help as of now.)

I thought that fixing the udev behavior would solve the problem.  But
it turned out that I was too naive.  A bigger problem is that all
user-space stuff misinterprets DISK_EVENT_EJECT_REQUEST event: they
see this as if the disk is *ready* to be ejected.  KDE, for example,
dismisses the DVD icon when it receives this event even if it's still
mounted.

I can think of a few possible solutions, but all imperfect.

A. As a short-cut solution, filter out DISK_EVENT_EJECT_REQUEST in
   drivers/block/cdrom.c when the device is in use (the patch below).
   This avoids the misbehavior, obviously, at least.  But it also
   misses the events if the user-space really expects the eject button
   press state.

B. Fix the kernel media polling to work without explicit media lock,
   remove udev hackish workarounds.  *In addition* fix all misuses of
   DISK_EVENT_EJECT_REQUESTEd in the desktop scenes.

C. Fix CDROM_EJECT incompatible behavior so that udev helper can use
   CDROM_EJECT instead of SCSI eject command, and relock the media if
   the eject fails.  And, fix all misuses of
   DISK_EVENT_EJECT_REQUESTED in the desktop scenes as well.

Do you guys have any better / feasible solution in mind?


thanks,

Takashi

---
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 5d28a45d2960..03c073eb5396 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -294,12 +294,15 @@ static bool lockdoor = 1;
 static bool check_media_type;
 /* automatically restart mrw format */
 static bool mrw_format_restart = 1;
+/* filter out the eject event when the device is being used */
+static bool filter_eject_event = 1;
 module_param(debug, bool, 0);
 module_param(autoclose, bool, 0);
 module_param(autoeject, bool, 0);
 module_param(lockdoor, bool, 0);
 module_param(check_media_type, bool, 0);
 module_param(mrw_format_restart, bool, 0);
+module_param(filter_eject_event, bool, 0);
 
 static DEFINE_MUTEX(cdrom_mutex);
 
@@ -1477,6 +1480,8 @@ static void cdrom_update_events(struct cdrom_device_info *cdi,
 	unsigned int events;
 
 	events = cdi->ops->check_events(cdi, clearing, CDSL_CURRENT);
+	if (filter_eject_event && cdi->use_count > 0)
+		events &= ~DISK_EVENT_EJECT_REQUEST;
 	cdi->vfs_events |= events;
 	cdi->ioctl_events |= events;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/