SCSI fixes for 4.16-rc5
James Bottomley
maintainerThis is four patches, consisting of one regression from the merge window (qla2xxx) one lonstanding memory leak (sd_zbc) one event queue mislabelling which we want to eliminate to discourage the pattern (mpt3sas) and one behaviour change because re-reading the partition table shouldn't clear the ro flag.
The patch is available here:
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes
The short changelog is:
Bill Kuzeja (1):
scsi: qla2xxx: Fix crashes in qla2x00_probe_one on probe failure
Damien Le Moal (1):
scsi: sd_zbc: Fix potential memory leak
Hannes Reinecke (1):
scsi: mpt3sas: Do not mark fw_event workqueue as WQ_MEM_RECLAIM
Jeremy Cline (1):
scsi: sd: Keep disk read-only when re-reading partition
And the diffstat:
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 2 +- drivers/scsi/qla2xxx/qla_os.c | 59 ++++++++++++++++++++++-------------- drivers/scsi/sd.c | 3 +- drivers/scsi/sd_zbc.c | 35 +++++++++------------
4 files changed, 55 insertions(+), 44 deletions(-)
With full diff below.
James
---
drivers/scsi/mpt3sas/mpt3sas_scsih.c +1 -1
@@ -10558,7 +10558,7 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id)snprintf (ioc->firmware_event_name,sizeof (ioc->firmware_event_name),"fw_event_%s%d" , ioc->driver_name, ioc->id); ioc->firmware_event_thread =alloc_ordered_workqueue ( - ioc->firmware_event_name,WQ_MEM_RECLAIM ); + ioc->firmware_event_name,0 );if (!ioc->firmware_event_thread) {pr_err (MPT3SAS_FMT "failure at %s:%d/%s()!\n" , ioc->name, __FILE__, __LINE__, __func__);
drivers/scsi/qla2xxx/qla_os.c +37 -22
123 lines not on this page · raw message
drivers/scsi/sd.c +2 -1
@@ -2624,6 +2624,7 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)int res;struct scsi_device *sdp = sdkp->device;struct scsi_mode_data data; +int disk_ro =get_disk_ro (sdkp->disk);int old_wp = sdkp->write_prot;set_disk_ro (sdkp->disk,0 );
@@ -2664,7 +2665,7 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer) "Test WP failed, assume Write Enabled\n"); }else { sdkp->write_prot = ((data.device_specific &0x80 ) !=0 ); -set_disk_ro (sdkp->disk, sdkp->write_prot); +set_disk_ro (sdkp->disk, sdkp->write_prot || disk_ro);if (sdkp->first_scan || old_wp != sdkp->write_prot) {sd_printk (KERN_NOTICE , sdkp,"Write Protect is %s\n" , sdkp->write_prot ?"on" :"off" );
drivers/scsi/sd_zbc.c +15 -20
@@ -486,7 +486,7 @@ static int sd_zbc_check_capacity(struct scsi_disk *sdkp, unsigned char *buf)*/ static int sd_zbc_check_zone_size (struct scsi_disk *sdkp) { -u64 zone_blocks; +u64 zone_blocks =0 ;sector_t block =0 ;unsigned char *buf;unsigned char *rec;
@@ -504,10 +504,8 @@ static int sd_zbc_check_zone_size(struct scsi_disk *sdkp)/* Do a report zone to get the same field */ ret =sd_zbc_report_zones (sdkp, buf,SD_ZBC_BUF_SIZE ,0 ); -if (ret) { - zone_blocks =0 ; - goto out; - } +if (ret) + goto out_free; same = buf[4 ] &0x0f ;if (same >0 ) {
@@ -547,7 +545,7 @@ static int sd_zbc_check_zone_size(struct scsi_disk *sdkp) ret =sd_zbc_report_zones (sdkp, buf,SD_ZBC_BUF_SIZE , block);if (ret) -return ret; + goto out_free; } }while (block < sdkp->capacity);
@@ -555,35 +553,32 @@ static int sd_zbc_check_zone_size(struct scsi_disk *sdkp) zone_blocks = sdkp->zone_blocks; out: -kfree (buf); -if (!zone_blocks) {if (sdkp->first_scan)sd_printk (KERN_NOTICE , sdkp,"Devices with non constant zone " "size are not supported\n" ); -return -ENODEV ; - } - -if (!is_power_of_2 (zone_blocks)) { + ret = -ENODEV ; + }else if (!is_power_of_2 (zone_blocks)) {if (sdkp->first_scan)sd_printk (KERN_NOTICE , sdkp,"Devices with non power of 2 zone " "size are not supported\n" ); -return -ENODEV ; - } - -if (logical_to_sectors (sdkp->device, zone_blocks) >UINT_MAX ) { + ret = -ENODEV ; + }else if (logical_to_sectors (sdkp->device, zone_blocks) >UINT_MAX ) {if (sdkp->first_scan)sd_printk (KERN_NOTICE , sdkp,"Zone size too large\n" ); -return -ENODEV ; + ret = -ENODEV ; + }else { + sdkp->zone_blocks = zone_blocks; + sdkp->zone_shift =ilog2 (zone_blocks); } - sdkp->zone_blocks = zone_blocks; - sdkp->zone_shift =ilog2 (zone_blocks); +out_free: +kfree (buf); -return 0 ; +return ret; }static int sd_zbc_setup (struct scsi_disk *sdkp)
Quoted 6 lines · James Bottomley
drivers/scsi/qla2xxx/qla_os.c +1
-5-vfree (ha->optrom_buffer); -kfree (ha->nvram); -kfree (ha->npiv_info); -kfree (ha->swl); -kfree (ha->loop_id_map); +
drivers/scsi/qla2xxx/qla_os.c qla2x00_mem_free(struct qla_hw_data *ha) · line 4521
Quoted 10 lines
drivers/scsi/qla2xxx/qla_os.c +10
-0+if (ha->optrom_buffer) +vfree (ha->optrom_buffer); +if (ha->nvram) +kfree (ha->nvram); +if (ha->npiv_info) +kfree (ha->npiv_info); +if (ha->swl) +kfree (ha->swl); +if (ha->loop_id_map) +kfree (ha->loop_id_map);
Why? This part is just garbage.
I've pulled it, but I don't see why (and how) this kind of garbage got reviewed and acked by multiple people.
Both vfree and kfree are perfectly happy with NULL pointers, and there doesn't seem to be any structural reason to have the test.
There *can* be valid reasons to do those kinds of things, if NULL is the common fast-path case, and you have profiles that show that the cost of just the call is noticeable. Then you probably also should have an "unlikely()" there to document that fact.
But this is not one of those cases. This is just garbage and generates less legible code.
Linus
Linus,
Quoted 2 lines
I've pulled it, but I don't see why (and how) this kind of garbage got reviewed and acked by multiple people.
My bad. I actually did notice the superfluous ifs and meant to nuke them when I committed the patch.
However, I had a freak accident with my fixes branch that day that compelled me to reset to my latest public hash and redo several commits. I completely forgot that I had intended to tweak this patch when I finally got around to applying it. I should have looked closer, but it was already in my "done" pile.
Botched it. Sorry about that!
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering