Bug 5485 - cdrom door locked when pktcdvd is attached
Summary: cdrom door locked when pktcdvd is attached
Status: CLOSED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Other (show other bugs)
Hardware: i386 Linux
: P2 low
Assignee: Jens Axboe
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-23 06:15 UTC by Ulrich Holeschak
Modified: 2008-09-23 10:22 UTC (History)
6 users (show)

See Also:
Kernel Version: 2.6.13.3
Subsystem:
Regression: ---
Bisected commit-id:


Attachments

Description Ulrich Holeschak 2005-10-23 06:15:20 UTC
Distribution: SUSE 9.3, Standard kernel 2.6.13.3

Problem Description:
When pktcdvd driver is attached to a cdrom device, mounting the cdrom directly
(without the packet driver) leads to a locked cdrom door after unmount.
Reason: cdrom.c:cdrom_release() only unlocks the door if the use_count is zero.
The pktcdvd driver always keeps one O_NONBLOCK connection open, so use_count
will never get zero.

Solution:
I have attached a patch, that counts open device connections that don't have the
O_NONBLOCK flag set (data connections). If the data connection counter is zero,
the door is unlocked.
I have also modified the autoeject feature. Now the cdrom is also ejected if the
data connection counter is zero. This my be a problem if mount tries several
file systems (auto), because in this case the tray opens after every filesystem
mount tries to mount.

Steps to reproduce:
pktsetup 0 /dev/hdxx
mount /dev/hdxx /mnt
umount /mnt
-> cdrom door locked

Patches:
Here are the required patches. The patch is based on a SUSE kernel 2.6.11.4-21.9
kernel, but should also apply to recent standard kernels (the problem is the
same). Modified lines are marked with [UH], so i can find then easier.

Patch for cdrom.c:

--- cdrom.c.org	2005-03-02 08:37:49.000000000 +0100
+++ cdrom.c	2005-10-23 13:30:14.000000000 +0200
@@ -1021,6 +1021,15 @@
 	if (ret)
 		goto err;
 
+	/* [UH] start */
+	if (!(fp->f_flags & O_NONBLOCK))
+	{
+	  cdi->use_data_count++;
+	}
+	cdinfo(CD_OPEN, "Use data count for \"/dev/%s\" now %d\n",
+			cdi->name, cdi->use_data_count);
+	/* [UH] end */
+
 	cdinfo(CD_OPEN, "Use count for \"/dev/%s\" now %d\n",
 			cdi->name, cdi->use_count);
 	/* Do this on open.  Don't wait for mount, because they might
@@ -1200,23 +1209,38 @@
 int cdrom_release(struct cdrom_device_info *cdi, struct file *fp)
 {
 	struct cdrom_device_ops *cdo = cdi->ops;
-	int opened_for_data;
+	// [UH] int opened_for_data;
 
 	cdinfo(CD_CLOSE, "entering cdrom_release\n"); 
 
 	if (cdi->use_count > 0)
 		cdi->use_count--;
+	/* [UH] start */
+	if (!(fp && fp->f_flags & O_NONBLOCK))
+	{
+	        if (cdi->use_data_count > 0)
+		{
+		    cdi->use_data_count--;
+		}
+	}
+	if (cdi->use_count == 0)
+	        cdi->use_data_count = 0;
+	if (cdi->use_data_count == 0)
+		cdinfo(CD_CLOSE, "Use data count for \"/dev/%s\" now zero\n", cdi->name);
+	/* [UH] end */
+
 	if (cdi->use_count == 0)
 		cdinfo(CD_CLOSE, "Use count for \"/dev/%s\" now zero\n", cdi->name);
 	if (cdi->use_count == 0)
 		cdrom_dvd_rw_close_write(cdi);
-	if (cdi->use_count == 0 &&
+	// [UH] if (cdi->use_count == 0 &&
+	if (cdi->use_data_count == 0 &&
 	    (cdo->capability & CDC_LOCK) && !keeplocked) {
 		cdinfo(CD_CLOSE, "Unlocking door!\n");
 		cdo->lock_door(cdi, 0);
 	}
-	opened_for_data = !(cdi->options & CDO_USE_FFLAGS) ||
-		!(fp && fp->f_flags & O_NONBLOCK);
+	// [UH] opened_for_data = !(cdi->options & CDO_USE_FFLAGS) ||
+	//	!(fp && fp->f_flags & O_NONBLOCK);
 
 	/*
 	 * flush cache on last write release
@@ -1225,8 +1249,10 @@
 		cdrom_close_write(cdi);
 
 	cdo->release(cdi);
-	if (cdi->use_count == 0) {      /* last process that closes dev*/
-		if (opened_for_data &&
+	// [UH] if (cdi->use_count == 0) {      /* last process that closes dev*/
+	if (cdi->use_data_count == 0) {      /* all data connections closed */
+	  // [UH] if (opened_for_data &&
+	  if (
 		    cdi->options & CDO_AUTO_EJECT && CDROM_CAN(CDC_OPEN_TRAY))
 			cdo->tray_move(cdi, 1);
 	}

Patch for cdrom.h:
--- cdrom.h.org	2005-03-02 08:37:31.000000000 +0100
+++ cdrom.h	2005-10-20 00:04:02.000000000 +0200
@@ -943,6 +943,7 @@
 	int options		: 30;	/* options flags */
 	unsigned mc_flags	: 2;	/* media change buffer flags */
     	int use_count;                  /* number of times device opened */
+    	int use_data_count;             /* [UH] number of times device opened in
data mode */
     	char name[20];                  /* name of the device type */
 /* per-device flags */
         __u8 sanyo_slot		: 2;	/* Sanyo 3 CD changer support */
Comment 1 Anonymous Emailer 2005-10-23 10:27:54 UTC
Reply-To: petero2@telia.com

On Sun, 23 Oct 2005, Andrew Morton wrote:

> When pktcdvd driver is attached to a cdrom device, mounting the cdrom
> directly (without the packet driver) leads to a locked cdrom door after
> unmount. Reason: cdrom.c:cdrom_release() only unlocks the door if the
> use_count is zero. The pktcdvd driver always keeps one O_NONBLOCK
> connection open, so use_count will never get zero.

> I have attached a patch, that counts open device connections that don't
> have the O_NONBLOCK flag set (data connections). If the data connection
> counter is zero, the door is unlocked.

Does this really work? I tried something similar myself several months
ago, but it didn't work because the fp parameter to cdrom_release() was
always NULL. This still appears to be the case. blkdev_put() calls the
release function like this:

	ret = disk->fops->release(bd_inode, NULL);

Comment 2 Jens Axboe 2005-10-23 10:43:40 UTC
On Sun, Oct 23 2005, Peter Osterlund wrote:
> On Sun, 23 Oct 2005, Andrew Morton wrote:
> 
> > When pktcdvd driver is attached to a cdrom device, mounting the cdrom
> > directly (without the packet driver) leads to a locked cdrom door after
> > unmount. Reason: cdrom.c:cdrom_release() only unlocks the door if the
> > use_count is zero. The pktcdvd driver always keeps one O_NONBLOCK
> > connection open, so use_count will never get zero.
> 
> > I have attached a patch, that counts open device connections that don't
> > have the O_NONBLOCK flag set (data connections). If the data connection
> > counter is zero, the door is unlocked.
> 
> Does this really work? I tried something similar myself several months
> ago, but it didn't work because the fp parameter to cdrom_release() was
> always NULL. This still appears to be the case. blkdev_put() calls the
> release function like this:
> 
> 	ret = disk->fops->release(bd_inode, NULL);

The patch is also broken, what if the app(s) use fcntl() to set/clear
O_NONBLOCK after open? Nogo.

Comment 3 Andrew Morton 2005-10-25 23:20:04 UTC
so what do we do?
Comment 4 Ulrich Holeschak 2005-10-26 23:10:38 UTC
I my case this is working perfecly, but perhaps it depends on how the
application (umount) closes the device ...
Comment 5 Andrew Morton 2007-01-31 00:21:09 UTC
Jens?  Any thought on a suitable fix for this one?
Comment 6 Natalie Protasevich 2007-06-16 09:46:42 UTC
Hi Ulrich,
To refresh the data, can you please try newest kernel to confirm that the problem still exists? 
Thanks.
Comment 7 Ulrich Holeschak 2007-07-01 12:50:22 UTC
At least this bug is still present in the 2.4.21 kernel.
At the moment i am using a mount.udf script to circumvent this problem.
This script dynamically calls pktsetup if an udf filesystem is mounted/unmounted.

Ulrich
Comment 8 Ulrich Holeschak 2007-07-01 13:03:21 UTC
(In reply to comment #7)
> At least this bug is still present in the 2.4.21 kernel.
> At the moment i am using a mount.udf script to circumvent this problem.
> This script dynamically calls pktsetup if an udf filesystem is
> mounted/unmounted.
> 
> Ulrich
> 
Sorry i mean kernel 2.6.21 ...
Comment 9 Satyam Sharma 2007-07-28 23:30:26 UTC
Hi Ulrich

Can you reproduce on 2.6.23-rc1? I tried reproducing this, but failed,
and saw some weird behaviour, see below:

(In reply to comment #0)
> Distribution: SUSE 9.3, Standard kernel 2.6.13.3
> 
> Problem Description:
> When pktcdvd driver is attached to a cdrom device, mounting the cdrom
> directly
> (without the packet driver) leads to a locked cdrom door after unmount.
> Reason: cdrom.c:cdrom_release() only unlocks the door if the use_count is
> zero.
> The pktcdvd driver always keeps one O_NONBLOCK connection open, so use_count
> will never get zero.

Right.

> Steps to reproduce:
> pktsetup 0 /dev/hdxx
> mount /dev/hdxx /mnt
> umount /mnt
> -> cdrom door locked

I reproduced the "situation", but not the the "problem". I did the above,
and although the use_count didn't become zero (was 1) after the umount(2),
*and* the last call to cdrom_ops->lock_door() was to *lock* it (I have an
"ide_cd" based driver, and this was from mount(2) codepath), I could still
use "eject" (or manually eject) the tray after the umount itself.

Which is weird, because it would appear that the "door_is_locked" state
and eject/open_tray commands are being maintained / given by independent
modules in the kernel ... so although I didn't reproduce the problem,
I think this could be a case of another bug hiding/"solving" the previous.
Comment 10 Ulrich Holeschak 2007-07-29 01:49:03 UTC
Hello,
i tried with kernel 2.6.22.1, you are right, something has changed in the behaviour ...
After umount /proc/sys/dev/cdrom/lock still shows that the device is locked, but i could eject the cdrom using the button on the front panel.
Also after the eject /proc/sys/dev/cdrom/lock still shows that the lock it set.
It seems that the 'lock' is not locking any more ...
Comment 11 Natalie Protasevich 2008-03-10 21:31:43 UTC
Any update on the problem? Does it still behave the same with recent kernel, 2.6.25+?
Thanks.

Note You need to log in before you can comment on or make changes to this bug.