Bug 5485
| Summary: | cdrom door locked when pktcdvd is attached | ||
|---|---|---|---|
| Product: | Drivers | Reporter: | Ulrich Holeschak (ulrich) |
| Component: | Other | Assignee: | Jens Axboe (axboe) |
| Status: | CLOSED CODE_FIX | ||
| Severity: | low | CC: | akpm, alan, axboe, bunk, protasnb, satyam |
| Priority: | P2 | ||
| Hardware: | i386 | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.13.3 | Subsystem: | |
| Regression: | --- | Bisected commit-id: | |
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); 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.
so what do we do? I my case this is working perfecly, but perhaps it depends on how the application (umount) closes the device ... Jens? Any thought on a suitable fix for this one? Hi Ulrich, To refresh the data, can you please try newest kernel to confirm that the problem still exists? Thanks. 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 (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 ... 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. 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 ... Any update on the problem? Does it still behave the same with recent kernel, 2.6.25+? Thanks. |
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 */