Very slow throughput when using cdparanoia on two SATA CDROM drives with /dev/sr but not /dev/sg

Hello,

I've got a big box of audio CDs to read, so I hooked up a load of SATA CDROM drives to a machine (Intel motherboard AHCI, and SATA SiI3124 controllers - in this example one was attached to each host controller), so that I could read them in parallel.

I'm using kernel 3.16.3, with cdparanoia 3.10.2 - both from Debian Jessie. https://www.xiph.org/paranoia/manual.html

When two (or more) drives are read simultaneously, the performance falls to pieces (throughput from each drive drops by 95%) if more than one /dev/sr* device is being read by cdparanoia.

If I tell cdparanoia to use the corresponding /dev/sg devices, then no significant throughput drop is experienced when reading multiple drives simultaneously.

As an example, using these two drives:

[1:0:0:0]    cd/dvd  TSSTcorp DVD+-RW TS-H653G DW10  /dev/sr0   /dev/sg4
[14:0:0:0]   cd/dvd  PLDS     DVD+-RW DH-16A6S YD11  /dev/sr9   /dev/sg16

... in the following results I used "time cdparanoia -v -d /dev/XXX 1 /tmp/1.wav" - where XXX was substituted for either sr9 or sg16

On an otherwise idle machine, I did these two sequentially:

sr9: 38 seconds

sg16: 38 seconds

Simultaneous with: cdparanoia -d /dev/sr0 11 /tmp/11.wav (and auto restarted that command when it completed) I then ran these two sequentially:

sr9: 680 seconds

sg16: 38 seconds

Simultaneous with: cdparanoia -d /dev/sg4 11 /tmp/11.wav as above:

sr9: 40 seconds

sg16: 40 seconds

This is a diff of the two sets of cdparanoia -v output (using the sr devices vs the sg devices):

@@ -2,9 +2,9 @@
 
 Using cdda library version: 10.2
 Using paranoia library version: 10.2
-Checking /dev/sr9 for cdrom...
-        Testing /dev/sr9 for SCSI/MMC interface
-                SG_IO device: /dev/sr9
+Checking /dev/sg16 for cdrom...
+        Testing /dev/sg16 for SCSI/MMC interface
+                SG_IO device: /dev/sg16
 
 CDROM model sensed sensed: PLDS DVD+-RW DH-16A6S YD11
 

@@ -13,9 +13,9 @@
 
 Checking for MMC style command set...
         Drive is MMC style
-        DMA scatter/gather table entries: 1
+        DMA scatter/gather table entries: 167
         table entry size: 131072 bytes
-        maximum theoretical transfer: 55 sectors
+        maximum theoretical transfer: 9185 sectors
         Setting default read size to 27 sectors (63504 bytes).
 
 Verifying CDDA command set...

@@ -23,3 +23,5 @@
 
 

I'm happy to try other kernel versions to gather more data. Which kernel trees/branches should I try?

I'm also assuming this is more likely to be a SCSI layer bug than a SATA one, so let me know if that's probably wrong. Also, is reporting here best or bugzilla?

Cheers,

Tim.

I had a similar problem burning multiple DVDs at the same time. I asked about this on the list more than 2 years ago and was pointed to a patch that fixed it for me. It involves sr_mod. You can unload it, patch the source and recomple. When sr_mod.ko is built, insmod that and it worked for me.

See https://lkml.org/lkml/2012/2/28/230 for the patch.

The machine I use to do this is using 3.3.0 with the patch and quite stable. I was using 3.0.0 at the time. I haven't tested on any newer kernels.

Do a search for the thread "Burning multiple DVDs at one time".

 Microsoft has beaten Volkswagen's world record.  Volkswagen only created 22
 million bugs.

Wakko, any chance you could resend a patch to remove the mutex from the ioctl path?

I'm trying out some local changes which removes the ex-BKL mutex from sr_block_ioctl() in sr.c, and instead uses a per-drive mutex which I've added to cdrom_device_info (as per Arnd Bergmann/ James Bottomley's discussion here https://lkml.org/lkml/2012/2/28/305 ).

This seems to work in a quick test testing (simultaneous cdparanoia on 8-or-so drives took ~10 minutes, whereas last time I tried that it took more than 18 hours), but I've been short on time to do much more.

At first inspection, it looks like it's just the cdrom-specific ioctls in drivers/cdrom/cdrom.c which aren't safely preemptible (my conclusion based on the fact that scsi_ioctl etc. is called elsewhere without locking (e.g. from /dev/sg and /dev/sd devices - I haven't looked at ).

As a result, I was wondering about moving the mutex_lock / unlock, so that they wrap just the cdrom-specific ioctl calls instead, although I suspect there won't be a huge performance benefit from doing so, it would have the advantage that all the code related to the new mutex to be moved into cdrom.c

Hopefully I'll grab a bit of time to work on this later this week, and post a patch.

Cheers,

Tim.

Yes, they are safe. Even inside cdrom_ioctl very little locking is required, but it might not be worth a lot of effort to go for fine grained locking there.

That sounds fine. It gets the performance critical SG_IO out of the critical region, and replaces the silly global lock in all the CDROM drivers with a per-device lock colocated with the data protected.

Totally offtopic: Jens, is it time to drop the silly paride driver?

Fix performance burning or extracting audio etc. from multiple optical drives.

Patches are against 3.18.0-rc6+

Currently each CDROM driver uses it's own per-driver mutex to protect non-thread-safe code within their ioctl paths.

These per-driver mutexes were created during the removal of the BKL.

An in-progress ioctl on one optical drive will force an ioctl on another optical drive in the same system to block (as long as both drives use the same driver e.g. sr_mod).

Since user-space programs which carry out tasks such as recording data, or reading raw audio must use ioctls to do-so, this makes it impractical to carry out more than one such operation simultaneosly on systems with multiple optical drives (e.g. record two DVDs at the same time, and/or extract raw audio data from two CDs simultaneously).

The calls scsi_ioctl() and related now happen without any locking, as these are thread-safe (e.g. as called by sd.c or sg.c).

With these patches applied, the time to simultaneously extract audio from 10 CDs, using the 'cdparanoia' utility, reduced more than 100 fold from around 18 hours to 10 minutes.

I've only done little bits and bobs of driver development in the past, so hopefully this first attempt isn't too far of the mark...

I've tested the IDE and SCSI subsystem patches, but I don't have any paride, or GDROM hardware (however the changes which are specific to these two are trivial).

Cheers,

Tim.

Tim Small (4):
enable cdrom_ioctl() to be called without holding ex-BKL mutexes Remove ex-BKL lock from ioctl path; fix simultaneous record on >1
drive
Remove ex-BKL lock from ioctl path; fix simultaneous record on >1
drive
Remove ex-BKL lock from ioctl path; fix simultaneous record on >1
drive

 drivers/block/paride/pcd.c |  8 +----
 drivers/cdrom/cdrom.c      | 86 +++++++++++++++++++++++++++++++---------------
 drivers/cdrom/gdrom.c      |  8 +----
 drivers/ide/ide-cd.c       | 42 ++++++++++------------
 drivers/scsi/sr.c          | 14 +++-----
 include/linux/cdrom.h      |  1 +
 6 files changed, 85 insertions(+), 74 deletions(-)

2.1.3

---
 drivers/cdrom/cdrom.c | 86 +++++++++++++++++++++++++++++++++++----------------
 include/linux/cdrom.h |  1 +
 2 files changed, 60 insertions(+), 27 deletions(-)

index 5d28a45..c39bef1 100644
@@ -633,6 +633,8 @@ int register_cdrom(struct cdrom_device_info *cdi)
 	if (!cdo->generic_packet)
 		cdo->generic_packet = cdrom_dummy_generic_packet;
 
+	mutex_init(&cdi->ioctl_mutex);
+
 	cd_dbg(CD_REG_UNREG, "drive \"/dev/%s\" registered\n", cdi->name);
 	mutex_lock(&cdrom_mutex);
 	list_add(&cdi->list, &cdrom_list);

@@ -3315,41 +3317,60 @@ int cdrom_ioctl(struct cdrom_device_info *cdi, struct block_device *bdev,
 	if (ret != -ENOTTY)
 		return ret;
 
+	mutex_lock(&cdi->ioctl_mutex);
+
 	switch (cmd) {
 	case CDROMMULTISESSION:
-		return cdrom_ioctl_multisession(cdi, argp);
+		ret = cdrom_ioctl_multisession(cdi, argp);
+		goto out;
 	case CDROMEJECT:
-		return cdrom_ioctl_eject(cdi);
+		ret = cdrom_ioctl_eject(cdi);
+		goto out;
 	case CDROMCLOSETRAY:
-		return cdrom_ioctl_closetray(cdi);
+		ret = cdrom_ioctl_closetray(cdi);
+		goto out;
 	case CDROMEJECT_SW:
-		return cdrom_ioctl_eject_sw(cdi, arg);
+		ret = cdrom_ioctl_eject_sw(cdi, arg);
+		goto out;
 	case CDROM_MEDIA_CHANGED:
-		return cdrom_ioctl_media_changed(cdi, arg);
+		ret = cdrom_ioctl_media_changed(cdi, arg);
+		goto out;
 	case CDROM_SET_OPTIONS:
-		return cdrom_ioctl_set_options(cdi, arg);
+		ret = cdrom_ioctl_set_options(cdi, arg);
+		goto out;
 	case CDROM_CLEAR_OPTIONS:
-		return cdrom_ioctl_clear_options(cdi, arg);
+		ret = cdrom_ioctl_clear_options(cdi, arg);
+		goto out;
 	case CDROM_SELECT_SPEED:
-		return cdrom_ioctl_select_speed(cdi, arg);
+		ret = cdrom_ioctl_select_speed(cdi, arg);
+		goto out;
 	case CDROM_SELECT_DISC:
-		return cdrom_ioctl_select_disc(cdi, arg);
+		ret = cdrom_ioctl_select_disc(cdi, arg);
+		goto out;
 	case CDROMRESET:
-		return cdrom_ioctl_reset(cdi, bdev);
+		ret = cdrom_ioctl_reset(cdi, bdev);
+		goto out;
 	case CDROM_LOCKDOOR:
-		return cdrom_ioctl_lock_door(cdi, arg);
+		ret = cdrom_ioctl_lock_door(cdi, arg);
+		goto out;
 	case CDROM_DEBUG:
-		return cdrom_ioctl_debug(cdi, arg);
+		ret = cdrom_ioctl_debug(cdi, arg);
+		goto out;
 	case CDROM_GET_CAPABILITY:
-		return cdrom_ioctl_get_capability(cdi);
+		ret = cdrom_ioctl_get_capability(cdi);
+		goto out;
 	case CDROM_GET_MCN:
-		return cdrom_ioctl_get_mcn(cdi, argp);
+		ret = cdrom_ioctl_get_mcn(cdi, argp);
+		goto out;
 	case CDROM_DRIVE_STATUS:
-		return cdrom_ioctl_drive_status(cdi, arg);
+		ret = cdrom_ioctl_drive_status(cdi, arg);
+		goto out;
 	case CDROM_DISC_STATUS:
-		return cdrom_ioctl_disc_status(cdi);
+		ret = cdrom_ioctl_disc_status(cdi);
+		goto out;
 	case CDROM_CHANGER_NSLOTS:
-		return cdrom_ioctl_changer_nslots(cdi);
+		ret = cdrom_ioctl_changer_nslots(cdi);
+		goto out;
 	}
 
 	/*

@@ -3361,7 +3382,7 @@ int cdrom_ioctl(struct cdrom_device_info *cdi, struct block_device *bdev,
 	if (CDROM_CAN(CDC_GENERIC_PACKET)) {
 		ret = mmc_ioctl(cdi, cmd, arg);
 		if (ret != -ENOTTY)
-			return ret;
+			goto out;
 	}
 
 	/*

@@ -3371,27 +3392,38 @@ int cdrom_ioctl(struct cdrom_device_info *cdi, struct block_device *bdev,
 	 */
 	switch (cmd) {
 	case CDROMSUBCHNL:
-		return cdrom_ioctl_get_subchnl(cdi, argp);
+		ret = cdrom_ioctl_get_subchnl(cdi, argp);
+		goto out;
 	case CDROMREADTOCHDR:
-		return cdrom_ioctl_read_tochdr(cdi, argp);
+		ret = cdrom_ioctl_read_tochdr(cdi, argp);
+		goto out;
 	case CDROMREADTOCENTRY:
-		return cdrom_ioctl_read_tocentry(cdi, argp);
+		ret = cdrom_ioctl_read_tocentry(cdi, argp);
+		goto out;
 	case CDROMPLAYMSF:
-		return cdrom_ioctl_play_msf(cdi, argp);
+		ret = cdrom_ioctl_play_msf(cdi, argp);
+		goto out;
 	case CDROMPLAYTRKIND:
-		return cdrom_ioctl_play_trkind(cdi, argp);
+		ret = cdrom_ioctl_play_trkind(cdi, argp);
+		goto out;
 	case CDROMVOLCTRL:
-		return cdrom_ioctl_volctrl(cdi, argp);
+		ret = cdrom_ioctl_volctrl(cdi, argp);
+		goto out;
 	case CDROMVOLREAD:
-		return cdrom_ioctl_volread(cdi, argp);
+		ret = cdrom_ioctl_volread(cdi, argp);
+		goto out;
 	case CDROMSTART:
 	case CDROMSTOP:
 	case CDROMPAUSE:
 	case CDROMRESUME:
-		return cdrom_ioctl_audioctl(cdi, cmd);
+		ret = cdrom_ioctl_audioctl(cdi, cmd);
+		goto out;
 	}
 
-	return -ENOSYS;
+	ret = -ENOSYS;
+out:
+	mutex_unlock(&cdi->ioctl_mutex);
+	return ret;
 }
 
 EXPORT_SYMBOL(cdrom_get_last_written);

index 8609d57..954e659 100644
@@ -40,6 +40,7 @@ struct cdrom_device_info {
 	struct list_head list;		/* linked list of all device_info */
 	struct gendisk *disk;		/* matching block layer disk */
 	void *handle;		        /* driver-dependent data */
+	struct mutex ioctl_mutex;	/* cdrom_ioctl_* not all thread safe */
 /* specifications */
 	int mask;                       /* mask of capability: disables them */
 	int speed;			/* maximum speed for reading data */

2.1.3

---
 drivers/scsi/sr.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

index 2de44cc..93111b1 100644
@@ -547,8 +547,6 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 	void __user *argp = (void __user *)arg;
 	int ret;
 
-	mutex_lock(&sr_mutex);
-
 	/*
 	 * Send SCSI addressing ioctls directly to mid level, send other
 	 * ioctls to cdrom/block level.

@@ -556,13 +554,12 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 	switch (cmd) {
 	case SCSI_IOCTL_GET_IDLUN:
 	case SCSI_IOCTL_GET_BUS_NUMBER:
-		ret = scsi_ioctl(sdev, cmd, argp);
-		goto out;
+		return scsi_ioctl(sdev, cmd, argp);
 	}
 
 	ret = cdrom_ioctl(&cd->cdi, bdev, mode, cmd, arg);
 	if (ret != -ENOSYS)
-		goto out;
+		return ret;
 
 	/*
 	 * ENODEV means that we didn't recognise the ioctl, or that we

@@ -573,12 +570,9 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 	ret = scsi_nonblockable_ioctl(sdev, cmd, argp,
 					(mode & FMODE_NDELAY) != 0);
 	if (ret != -ENODEV)
-		goto out;
-	ret = scsi_ioctl(sdev, cmd, argp);
+		return ret;
 
-out:
-	mutex_unlock(&sr_mutex);
-	return ret;
+	return scsi_ioctl(sdev, cmd, argp);
 }
 
 static unsigned int sr_block_check_events(struct gendisk *disk,

2.1.3

---
 drivers/block/paride/pcd.c | 8 +-------
 drivers/cdrom/gdrom.c      | 8 +-------
 2 files changed, 2 insertions(+), 14 deletions(-)

index 3b7c9f1..61cabfd 100644
@@ -248,13 +248,7 @@ static int pcd_block_ioctl(struct block_device *bdev, fmode_t mode,
 				unsigned cmd, unsigned long arg)
 {
 	struct pcd_unit *cd = bdev->bd_disk->private_data;
-	int ret;
-
-	mutex_lock(&pcd_mutex);
-	ret = cdrom_ioctl(&cd->info, bdev, mode, cmd, arg);
-	mutex_unlock(&pcd_mutex);
-
-	return ret;
+	return cdrom_ioctl(&cd->info, bdev, mode, cmd, arg);
 }
 
 static unsigned int pcd_block_check_events(struct gendisk *disk,

index 584bc31..f01d01f 100644
@@ -519,13 +519,7 @@ static unsigned int gdrom_bdops_check_events(struct gendisk *disk,
 static int gdrom_bdops_ioctl(struct block_device *bdev, fmode_t mode,
 	unsigned cmd, unsigned long arg)
 {
-	int ret;
-
-	mutex_lock(&gdrom_mutex);
-	ret = cdrom_ioctl(gd.cd_info, bdev, mode, cmd, arg);
-	mutex_unlock(&gdrom_mutex);
-
-	return ret;
+	return cdrom_ioctl(gd.cd_info, bdev, mode, cmd, arg);
 }
 
 static const struct block_device_operations gdrom_bdops = {

2.1.3

---
 drivers/ide/ide-cd.c | 42 +++++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

index 0b510ba..946d458 100644
@@ -1656,37 +1656,33 @@ static int idecd_get_spindown(struct cdrom_device_info *cdi, unsigned long arg)
 	return 0;
 }
 
-static int idecd_locked_ioctl(struct block_device *bdev, fmode_t mode,
-			unsigned int cmd, unsigned long arg)
-{
-	struct cdrom_info *info = ide_drv_g(bdev->bd_disk, cdrom_info);
-	int err;
-
-	switch (cmd) {
-	case CDROMSETSPINDOWN:
-		return idecd_set_spindown(&info->devinfo, arg);
-	case CDROMGETSPINDOWN:
-		return idecd_get_spindown(&info->devinfo, arg);
-	default:
-		break;
-	}
-
-	err = generic_ide_ioctl(info->drive, bdev, cmd, arg);
-	if (err == -EINVAL)
-		err = cdrom_ioctl(&info->devinfo, bdev, mode, cmd, arg);
-
-	return err;
-}
-
 static int idecd_ioctl(struct block_device *bdev, fmode_t mode,
 			     unsigned int cmd, unsigned long arg)
 {
+	struct cdrom_info *info;
 	int ret;
 
 	mutex_lock(&ide_cd_mutex);
-	ret = idecd_locked_ioctl(bdev, mode, cmd, arg);
+	info = ide_drv_g(bdev->bd_disk, cdrom_info);
+
+	switch (cmd) {
+	case CDROMSETSPINDOWN:
+		ret = idecd_set_spindown(&info->devinfo, arg);
+		goto outunlock;
+	case CDROMGETSPINDOWN:
+		ret = idecd_get_spindown(&info->devinfo, arg);
+		goto outunlock;
+	}
+
+	ret = generic_ide_ioctl(info->drive, bdev, cmd, arg);
+	if (ret != -EINVAL)
+		goto outunlock;
+
 	mutex_unlock(&ide_cd_mutex);
+	return cdrom_ioctl(&info->devinfo, bdev, mode, cmd, arg);
 
+outunlock:
+	mutex_unlock(&ide_cd_mutex);
 	return ret;
 }
 

2.1.3

This looks reasonable to me. Jens, do you want to takes this through the block tree?

Yeah I can do that, though do we really need to do paride here?

Tim, you also need to work on your changelogs. Please resend (dropping paride), and add proper subject and commit message. Patches 2-4 have identical subjects, and no commit message...

Jens Axboe

Well, no need to keep the mutex around in paride for no reason. But my preference would be to just kill off paride anyway :)

That's sort of my point. Performance optimization and paride, well... But it's not a huge deal, we can apply it.

Jens Axboe

I did consider this, but I made the change there too on the basis that:

. paride has received a few commits this year (and is listed as being
maintained)
. The change is trivial
. It fixes a performance regression which was introduced during the BKL
removal (mutex being retained by sleeping processes).

I'm happy to drop it, if you prefer.

Sorry about that, will fix it with next version.

Having just seen this thread from 2013:

http://permalink.gmane.org/gmane.linux.scsi/79483

I decided to exercise the eject code path a bit more by triggering simultaneous eject commands on all 11 optical drives in my test box, followed by simultaneous close-tray commands, repeatedly.

I haven't been able to reproduce the error reported in that email, but from observing the behaviour of the drives it looks like access to pata drives is being serialising elsewhere, so the issue in that link may have been fixed?

Unfortunately running these tests did eventually make all further attempts to open /dev/sr* block on my test box.

I've stared at the code for a while, but not making any headway currently, except that a blocking blk_execute_rq (called by test_unit_ready) is then causing all over cdrom open/close calls to block (because sr_mutex is held by sr_block_open(), and in turn calls check_disk_change... scsi_test_unit_ready).

How do I work out why blk_execute_rq is blocking?

# ps -l 3779 2383 3780
F S   UID   PID  PPID  C PRI  NI ADDR SZ WCHAN  TTY        TIME CMD
1 D     0  2383     2  0  80   0 -     0 blk_ex ?          0:00 [kworker/1:2]
0 D     0  3779  1034  0  80   0 -  1057 blk_ex pts/0      0:00 eject -t /dev/sr7
0 D     0  3780  1034  0  80   0 -     0 sr_blo pts/0      0:00 [eject]

/proc/3779/stack-[<ffffffff812e47cb>] blk_execute_rq+0x16b/0x210 /proc/3779/stack-[<ffffffffa0291cb1>] scsi_execute+0x141/0x1f0 [scsi_mod] /proc/3779/stack-[<ffffffffa0293e1e>] scsi_execute_req_flags+0x8e/0x100 [scsi_mod] /proc/3779/stack-[<ffffffffa02944f3>] scsi_test_unit_ready+0x83/0x130 [scsi_mod] /proc/3779/stack:[<ffffffffa05a9a7c>] sr_check_events+0x13c/0x310 [sr_mod] /proc/3779/stack-[<ffffffffa06cd05c>] cdrom_check_events+0x1c/0x40 [cdrom] /proc/3779/stack:[<ffffffffa05a9ecd>] sr_block_check_events+0x2d/0x30 [sr_mod] /proc/3779/stack-[<ffffffff812eed41>] disk_check_events+0x51/0x170 /proc/3779/stack-[<ffffffff812f068c>] disk_clear_events+0x6c/0x130 /proc/3779/stack-[<ffffffff81227150>] check_disk_change+0x30/0x80 /proc/3779/stack-[<ffffffffa06cfd9a>] cdrom_open+0x4a/0xba0 [cdrom] /proc/3779/stack:[<ffffffffa05aa6c2>] sr_block_open+0x92/0x130 [sr_mod] /proc/3779/stack-[<ffffffff81227b71>] __blkdev_get+0xd1/0x4b0 /proc/3779/stack-[<ffffffff81227f91>] blkdev_get+0x41/0x3e0 /proc/3779/stack-[<ffffffff812283fe>] blkdev_open+0x6e/0x90 /proc/3779/stack-[<ffffffff811e6cbd>] do_dentry_open+0x1ed/0x360 /proc/3779/stack-[<ffffffff811e6f99>] vfs_open+0x49/0x50 /proc/3779/stack-[<ffffffff811fa849>] do_last+0x239/0x1520 /proc/3779/stack-[<ffffffff811fbbe7>] path_openat+0xb7/0x6a0 /proc/3779/stack-[<ffffffff811fd3ea>] do_filp_open+0x3a/0xb0 /proc/3779/stack-[<ffffffff811e896c>] do_sys_open+0x12c/0x220 /proc/3779/stack-[<ffffffff811e8a7e>] SyS_open+0x1e/0x20

/proc/2383/stack-[<ffffffff812e47cb>] blk_execute_rq+0x16b/0x210 /proc/2383/stack-[<ffffffffa0291cb1>] scsi_execute+0x141/0x1f0 [scsi_mod] /proc/2383/stack-[<ffffffffa0293e1e>] scsi_execute_req_flags+0x8e/0x100 [scsi_mod] /proc/2383/stack-[<ffffffffa02944f3>] scsi_test_unit_ready+0x83/0x130 [scsi_mod] /proc/2383/stack:[<ffffffffa05a9a7c>] sr_check_events+0x13c/0x310 [sr_mod] /proc/2383/stack-[<ffffffffa06cd05c>] cdrom_check_events+0x1c/0x40 [cdrom] /proc/2383/stack:[<ffffffffa05a9ecd>] sr_block_check_events+0x2d/0x30 [sr_mod] /proc/2383/stack-[<ffffffff812eed41>] disk_check_events+0x51/0x170 /proc/2383/stack-[<ffffffff812eee7c>] disk_events_workfn+0x1c/0x20 /proc/2383/stack-[<ffffffff8108b91f>] process_one_work+0x1df/0x520 /proc/2383/stack-[<ffffffff8108bf3b>] worker_thread+0x6b/0x4a0 /proc/2383/stack-[<ffffffff8109130b>] kthread+0x10b/0x130 /proc/2383/stack-[<ffffffff815b8afc>] ret_from_fork+0x7c/0xb0 /proc/2383/stack-[<ffffffffffffffff>] 0xffffffffffffffff

/proc/3780/stack:[<ffffffffa05aa604>] sr_block_release+0x24/0x50 [sr_mod] /proc/3780/stack-[<ffffffff81227a65>] __blkdev_put+0x185/0x1c0 /proc/3780/stack-[<ffffffff81228472>] blkdev_put+0x52/0x180 /proc/3780/stack-[<ffffffff81228658>] blkdev_close+0x28/0x30 /proc/3780/stack-[<ffffffff811eb4e5>] __fput+0xf5/0x210 /proc/3780/stack-[<ffffffff811eb64e>] ____fput+0xe/0x10 /proc/3780/stack-[<ffffffff8108f824>] task_work_run+0xc4/0xf0 /proc/3780/stack-[<ffffffff810734fa>] do_exit+0x3da/0xb70 /proc/3780/stack-[<ffffffff81073d34>] do_group_exit+0x54/0xe0 /proc/3780/stack-[<ffffffff81073dd4>] SyS_exit_group+0x14/0x20 /proc/3780/stack-[<ffffffff815b8bad>] system_call_fastpath+0x16/0x1b

# grep -l scsi_exec /proc/*/stack
/proc/2383/stack
/proc/3779/stack
# grep -l ioctl /proc/*/stack
# grep -l blk_execute /proc/*/stack
/proc/2383/stack
/proc/3779/stack
#

Cheers,

Tim.

...

An extra data point, the drive is showing busy:

# cat /sys/block/sr7/device/device_busy
1

I was wondering if this might be an unrelated bug in the drive or host adaptor driver?

This device is attached to an old PCI card using pata_pdc2027x, and has an old firmware version, so I'm going to try and change both of those things and try again.

Tim.

Hi Tim,

As you're playing with locks, I assume you're running with LOCKDEP enabled? If not, that might tell you what's going on.

Thanks,

Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/

Hi,

Sorry - I should have said - LOCKDEP is on, and not reporting anything...

Cheers,

Tim.

I know this is a bit late and is still not in 4.3. I applied 2 of the patches. I did not apply the ide-cd, the paride nor the gdrom since I don't have any of those.

I had to make some modifications to the patch for sr.c since it has changed since. cdrom.[ch] worked verbatim.

I tested on a system with 3 drives. ejecting all drives didn't happen at the same time, but I think it's because they are different brands and one didn't have a disc in. I did notice the leds coming on about the same time though. eject -t on all drives happened at the same time.

The patch I used previously on 3.3.0 removed all mutex_lock and mutex_unlock lines from sr.c where as this patchset didn't. I plan on trying to burn 3 dvds to see if it works.

Thanks for your work on the patches.

 Microsoft has beaten Volkswagen's world record.  Volkswagen only created 22
 million bugs.

No problem. I haven't had any time to follow up (and probably won't for the foreseeable - I've got far too much on at the moment unfortunately), and the locking issues looked non-trivial unfortunately.

In my testing burning, and audio extracting etc. worked pretty flawlessly IIRC, it was just the eject/load path which seemed to have locking issues.

The test was just a shell scripts which ran:

while true ; do eject /dev/sr0 ; eject -T /dev/sr0 ; done

for every drive in the system simultaneously.

Hopefully it's a good start if someone wants to pick it up. It's possible that there's an easy way of leaving the old mutexes (or adding more) around the relevant open/eject/load paths only, but I can't remember the code now unfortunately.

If anyone wants to have a go, I think I can probably rig up about 8 drives to a testrig here, and will be happy to give it a test.

Tim.