Commit 51a85881 authored by Merlijn Wajer's avatar Merlijn Wajer Committed by Martin K. Petersen

scsi: sr: get rid of sr global mutex

When replacing the Big Kernel Lock in commit 2a48fc0a ("block:
autoconvert trivial BKL users to private mutex"), the lock was replaced
with a sr-wide lock.

This causes very poor performance when using multiple sr devices, as the sr
driver was not able to execute more than one command to one drive at any
given time, even when there were many CD drives available.

Replace the global mutex with per-sr-device mutex.

Someone tried this patch at the time, but it never made it upstream, due to
possible concerns with race conditions, but it's not clear the patch
actually caused those:

https://www.spinics.net/lists/linux-scsi/msg63706.html
https://www.spinics.net/lists/linux-scsi/msg63750.html

Also see

http://lists.xiph.org/pipermail/paranoia/2019-December/001647.html

Link: https://lore.kernel.org/r/20200218143918.30267-1-merlijn@archive.orgAcked-by: default avatarArnd Bergmann <arnd@arndb.de>
Signed-off-by: default avatarMerlijn Wajer <merlijn@archive.org>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 679b2ec8
...@@ -79,7 +79,6 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_WORM); ...@@ -79,7 +79,6 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_WORM);
CDC_CD_R|CDC_CD_RW|CDC_DVD|CDC_DVD_R|CDC_DVD_RAM|CDC_GENERIC_PACKET| \ CDC_CD_R|CDC_CD_RW|CDC_DVD|CDC_DVD_R|CDC_DVD_RAM|CDC_GENERIC_PACKET| \
CDC_MRW|CDC_MRW_W|CDC_RAM) CDC_MRW|CDC_MRW_W|CDC_RAM)
static DEFINE_MUTEX(sr_mutex);
static int sr_probe(struct device *); static int sr_probe(struct device *);
static int sr_remove(struct device *); static int sr_remove(struct device *);
static blk_status_t sr_init_command(struct scsi_cmnd *SCpnt); static blk_status_t sr_init_command(struct scsi_cmnd *SCpnt);
...@@ -536,9 +535,9 @@ static int sr_block_open(struct block_device *bdev, fmode_t mode) ...@@ -536,9 +535,9 @@ static int sr_block_open(struct block_device *bdev, fmode_t mode)
scsi_autopm_get_device(sdev); scsi_autopm_get_device(sdev);
check_disk_change(bdev); check_disk_change(bdev);
mutex_lock(&sr_mutex); mutex_lock(&cd->lock);
ret = cdrom_open(&cd->cdi, bdev, mode); ret = cdrom_open(&cd->cdi, bdev, mode);
mutex_unlock(&sr_mutex); mutex_unlock(&cd->lock);
scsi_autopm_put_device(sdev); scsi_autopm_put_device(sdev);
if (ret) if (ret)
...@@ -551,10 +550,10 @@ static int sr_block_open(struct block_device *bdev, fmode_t mode) ...@@ -551,10 +550,10 @@ static int sr_block_open(struct block_device *bdev, fmode_t mode)
static void sr_block_release(struct gendisk *disk, fmode_t mode) static void sr_block_release(struct gendisk *disk, fmode_t mode)
{ {
struct scsi_cd *cd = scsi_cd(disk); struct scsi_cd *cd = scsi_cd(disk);
mutex_lock(&sr_mutex); mutex_lock(&cd->lock);
cdrom_release(&cd->cdi, mode); cdrom_release(&cd->cdi, mode);
scsi_cd_put(cd); scsi_cd_put(cd);
mutex_unlock(&sr_mutex); mutex_unlock(&cd->lock);
} }
static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd, static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
...@@ -565,7 +564,7 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd, ...@@ -565,7 +564,7 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
void __user *argp = (void __user *)arg; void __user *argp = (void __user *)arg;
int ret; int ret;
mutex_lock(&sr_mutex); mutex_lock(&cd->lock);
ret = scsi_ioctl_block_when_processing_errors(sdev, cmd, ret = scsi_ioctl_block_when_processing_errors(sdev, cmd,
(mode & FMODE_NDELAY) != 0); (mode & FMODE_NDELAY) != 0);
...@@ -595,7 +594,7 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd, ...@@ -595,7 +594,7 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
scsi_autopm_put_device(sdev); scsi_autopm_put_device(sdev);
out: out:
mutex_unlock(&sr_mutex); mutex_unlock(&cd->lock);
return ret; return ret;
} }
...@@ -608,7 +607,7 @@ static int sr_block_compat_ioctl(struct block_device *bdev, fmode_t mode, unsign ...@@ -608,7 +607,7 @@ static int sr_block_compat_ioctl(struct block_device *bdev, fmode_t mode, unsign
void __user *argp = compat_ptr(arg); void __user *argp = compat_ptr(arg);
int ret; int ret;
mutex_lock(&sr_mutex); mutex_lock(&cd->lock);
ret = scsi_ioctl_block_when_processing_errors(sdev, cmd, ret = scsi_ioctl_block_when_processing_errors(sdev, cmd,
(mode & FMODE_NDELAY) != 0); (mode & FMODE_NDELAY) != 0);
...@@ -638,7 +637,7 @@ static int sr_block_compat_ioctl(struct block_device *bdev, fmode_t mode, unsign ...@@ -638,7 +637,7 @@ static int sr_block_compat_ioctl(struct block_device *bdev, fmode_t mode, unsign
scsi_autopm_put_device(sdev); scsi_autopm_put_device(sdev);
out: out:
mutex_unlock(&sr_mutex); mutex_unlock(&cd->lock);
return ret; return ret;
} }
...@@ -745,6 +744,7 @@ static int sr_probe(struct device *dev) ...@@ -745,6 +744,7 @@ static int sr_probe(struct device *dev)
disk = alloc_disk(1); disk = alloc_disk(1);
if (!disk) if (!disk)
goto fail_free; goto fail_free;
mutex_init(&cd->lock);
spin_lock(&sr_index_lock); spin_lock(&sr_index_lock);
minor = find_first_zero_bit(sr_index_bits, SR_DISKS); minor = find_first_zero_bit(sr_index_bits, SR_DISKS);
...@@ -1055,6 +1055,8 @@ static void sr_kref_release(struct kref *kref) ...@@ -1055,6 +1055,8 @@ static void sr_kref_release(struct kref *kref)
put_disk(disk); put_disk(disk);
mutex_destroy(&cd->lock);
kfree(cd); kfree(cd);
} }
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include <linux/genhd.h> #include <linux/genhd.h>
#include <linux/kref.h> #include <linux/kref.h>
#include <linux/mutex.h>
#define MAX_RETRIES 3 #define MAX_RETRIES 3
#define SR_TIMEOUT (30 * HZ) #define SR_TIMEOUT (30 * HZ)
...@@ -51,6 +52,7 @@ typedef struct scsi_cd { ...@@ -51,6 +52,7 @@ typedef struct scsi_cd {
bool ignore_get_event:1; /* GET_EVENT is unreliable, use TUR */ bool ignore_get_event:1; /* GET_EVENT is unreliable, use TUR */
struct cdrom_device_info cdi; struct cdrom_device_info cdi;
struct mutex lock;
/* We hold gendisk and scsi_device references on probe and use /* We hold gendisk and scsi_device references on probe and use
* the refs on this kref to decide when to release them */ * the refs on this kref to decide when to release them */
struct kref kref; struct kref kref;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment