mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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?
Date: Mon, 02 Feb 2015 14:20:16 +0100	[thread overview]
Message-ID: <s5hmw4wfo1b.wl-tiwai@suse.de> (raw)

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;
 }

             reply	other threads:[~2015-02-02 13:20 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-02 13:20 Takashi Iwai [this message]
2015-02-02 19:03 ` Kay Sievers
2015-02-02 19:34   ` Maciej W. Rozycki
2015-02-02 19:45     ` Kay Sievers
2015-02-02 21:12       ` Maciej W. Rozycki
2015-02-03 12:36         ` Takashi Iwai
2015-02-03  8:52       ` Oliver Neukum
2015-02-03 13:34       ` One Thousand Gnomes
2015-02-03 17:53         ` Theodore Ts'o
2015-02-02 20:02     ` Austin S Hemmelgarn
2015-02-02 20:53       ` Ondrej Zary
2015-02-03 12:31   ` Takashi Iwai
2015-02-03 13:39     ` One Thousand Gnomes
2015-02-03 13:49       ` Takashi Iwai
2015-02-03 13:40     ` Austin S Hemmelgarn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hmw4wfo1b.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=axboe@kernel.dk \
    --cc=kay@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oneukum@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®