From 0761df9c4b2d966da3af2ac4ee7372afa681ce63 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Fri, 10 May 2013 11:06:16 +0200 Subject: [SCSI] sd: avoid deadlocks when running under multipath When multipathed systems run into an all-paths-down scenario all devices might be dropped, too. This causes 'del_gendisk' to be called, which will unregister the kobj_map->probe() function for all disk device numbers. When the device comes back the default ->probe() function is run which will call __request_module(), which will deadlock. As 'del_gendisk' typically does _not_ trigger a module unload the default ->probe() function is pointless anyway. This patch implements a dummy ->probe() function, which will just return NULL if the disk is not registered. This will avoid the deadlock. Plus it'll speed up device scanning. Signed-off-by: Hannes Reinecke Signed-off-by: James Bottomley --- drivers/scsi/sd.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) (limited to 'drivers/scsi/sd.c') diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index c1c555242d0d..a37eda9f11f5 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -503,6 +503,16 @@ static struct scsi_driver sd_template = { .eh_action = sd_eh_action, }; +/* + * Dummy kobj_map->probe function. + * The default ->probe function will call modprobe, which is + * pointless as this module is already loaded. + */ +static struct kobject *sd_default_probe(dev_t devt, int *partno, void *data) +{ + return NULL; +} + /* * Device no to disk mapping: * @@ -2970,8 +2980,10 @@ static int sd_probe(struct device *dev) static int sd_remove(struct device *dev) { struct scsi_disk *sdkp; + dev_t devt; sdkp = dev_get_drvdata(dev); + devt = disk_devt(sdkp->disk); scsi_autopm_get_device(sdkp->device); async_synchronize_full_domain(&scsi_sd_probe_domain); @@ -2981,6 +2993,9 @@ static int sd_remove(struct device *dev) del_gendisk(sdkp->disk); sd_shutdown(dev); + blk_register_region(devt, SD_MINORS, NULL, + sd_default_probe, NULL, NULL); + mutex_lock(&sd_ref_mutex); dev_set_drvdata(dev, NULL); put_device(&sdkp->dev); @@ -3124,9 +3139,13 @@ static int __init init_sd(void) SCSI_LOG_HLQUEUE(3, printk("init_sd: sd driver entry point\n")); - for (i = 0; i < SD_MAJORS; i++) - if (register_blkdev(sd_major(i), "sd") == 0) - majors++; + for (i = 0; i < SD_MAJORS; i++) { + if (register_blkdev(sd_major(i), "sd") != 0) + continue; + majors++; + blk_register_region(sd_major(i), SD_MINORS, NULL, + sd_default_probe, NULL, NULL); + } if (!majors) return -ENODEV; @@ -3185,8 +3204,10 @@ static void __exit exit_sd(void) class_unregister(&sd_disk_class); - for (i = 0; i < SD_MAJORS; i++) + for (i = 0; i < SD_MAJORS; i++) { + blk_unregister_region(sd_major(i), SD_MINORS); unregister_blkdev(sd_major(i), "sd"); + } } module_init(init_sd); -- cgit From 2ee3e26c673e75c05ef8b914f54fadee3d7b9c88 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Mon, 27 May 2013 19:07:19 +0100 Subject: [SCSI] sd: Fix parsing of 'temporary ' cache mode prefix Commit 39c60a0948cc '[SCSI] sd: fix array cache flushing bug causing performance problems' added temp as a pointer to "temporary " and used sizeof(temp) - 1 as its length. But sizeof(temp) is the size of the pointer, not the size of the string constant. Change temp to a static array so that sizeof() does what was intended. Signed-off-by: Ben Hutchings Cc: stable@vger.kernel.org Signed-off-by: James Bottomley --- drivers/scsi/sd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/scsi/sd.c') diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index a37eda9f11f5..91e8a953d1de 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -142,7 +142,7 @@ sd_store_cache_type(struct device *dev, struct device_attribute *attr, char *buffer_data; struct scsi_mode_data data; struct scsi_sense_hdr sshdr; - const char *temp = "temporary "; + static const char temp[] = "temporary "; int len; if (sdp->type != TYPE_DISK) -- cgit From 66c28f97120e8a621afd5aa7a31c4b85c547d33d Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Thu, 6 Jun 2013 22:15:55 -0400 Subject: [SCSI] sd: Update WRITE SAME heuristics SATA drives located behind a SAS controller would incorrectly receive WRITE SAME commands. Tweak the heuristics so that: - If REPORT SUPPORTED OPERATION CODES is provided we will use that to choose between WRITE SAME(16), WRITE SAME(10) and disabled. This also fixes an issue with the old code which would issue WRITE SAME(10) despite the command not being whitelisted in REPORT SUPPORTED OPERATION CODES. - If REPORT SUPPORTED OPERATION CODES is not provided we will fall back to WRITE SAME(10) unless the device has an ATA Information VPD page. The assumption is that a SATL which is smart enough to implement WRITE SAME would also provide REPORT SUPPORTED OPERATION CODES. To facilitate the new heuristics scsi_report_opcode() has been modified to so we can distinguish between "operation not supported" and "RSOC not supported". Reported-by: H. Peter Anvin Tested-by: Bernd Schubert Signed-off-by: Martin K. Petersen Cc: Signed-off-by: James Bottomley --- drivers/scsi/sd.c | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-) (limited to 'drivers/scsi/sd.c') diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 91e8a953d1de..34da1a1acb09 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -442,8 +442,10 @@ sd_store_write_same_blocks(struct device *dev, struct device_attribute *attr, if (max == 0) sdp->no_write_same = 1; - else if (max <= SD_MAX_WS16_BLOCKS) + else if (max <= SD_MAX_WS16_BLOCKS) { + sdp->no_write_same = 0; sdkp->max_ws_blocks = max; + } sd_config_write_same(sdkp); @@ -750,7 +752,6 @@ static void sd_config_write_same(struct scsi_disk *sdkp) { struct request_queue *q = sdkp->disk->queue; unsigned int logical_block_size = sdkp->device->sector_size; - unsigned int blocks = 0; if (sdkp->device->no_write_same) { sdkp->max_ws_blocks = 0; @@ -762,18 +763,20 @@ static void sd_config_write_same(struct scsi_disk *sdkp) * blocks per I/O unless the device explicitly advertises a * bigger limit. */ - if (sdkp->max_ws_blocks == 0) - sdkp->max_ws_blocks = SD_MAX_WS10_BLOCKS; - - if (sdkp->ws16 || sdkp->max_ws_blocks > SD_MAX_WS10_BLOCKS) - blocks = min_not_zero(sdkp->max_ws_blocks, - (u32)SD_MAX_WS16_BLOCKS); - else - blocks = min_not_zero(sdkp->max_ws_blocks, - (u32)SD_MAX_WS10_BLOCKS); + if (sdkp->max_ws_blocks > SD_MAX_WS10_BLOCKS) + sdkp->max_ws_blocks = min_not_zero(sdkp->max_ws_blocks, + (u32)SD_MAX_WS16_BLOCKS); + else if (sdkp->ws16 || sdkp->ws10 || sdkp->device->no_report_opcodes) + sdkp->max_ws_blocks = min_not_zero(sdkp->max_ws_blocks, + (u32)SD_MAX_WS10_BLOCKS); + else { + sdkp->device->no_write_same = 1; + sdkp->max_ws_blocks = 0; + } out: - blk_queue_max_write_same_sectors(q, blocks * (logical_block_size >> 9)); + blk_queue_max_write_same_sectors(q, sdkp->max_ws_blocks * + (logical_block_size >> 9)); } /** @@ -2645,9 +2648,24 @@ static void sd_read_block_provisioning(struct scsi_disk *sdkp) static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer) { - if (scsi_report_opcode(sdkp->device, buffer, SD_BUF_SIZE, - WRITE_SAME_16)) + struct scsi_device *sdev = sdkp->device; + + if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, INQUIRY) < 0) { + sdev->no_report_opcodes = 1; + + /* Disable WRITE SAME if REPORT SUPPORTED OPERATION + * CODES is unsupported and the device has an ATA + * Information VPD page (SAT). + */ + if (!scsi_get_vpd_page(sdev, 0x89, buffer, SD_BUF_SIZE)) + sdev->no_write_same = 1; + } + + if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, WRITE_SAME_16) == 1) sdkp->ws16 = 1; + + if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, WRITE_SAME) == 1) + sdkp->ws10 = 1; } static int sd_try_extended_inquiry(struct scsi_device *sdp) -- cgit