2015-07-28 13:57:31

by Tomas Winkler

[permalink] [raw]
Subject: [scsi 1/7 RESEND] scsi_debug: define pr_fmt() for consistent logging

Use pr_fmt with both module name and __func__
Also drop few bare printk leftovers

The log format should stay pretty much intact

Signed-off-by: Tomas Winkler <[email protected]>
Acked-by: Douglas Gilbert <[email protected]>
---
drivers/scsi/scsi_debug.c | 118 +++++++++++++++++++++-------------------------
1 file changed, 53 insertions(+), 65 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 30268bb2ddb6..6e6bf0f03cf7 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -25,6 +25,9 @@
* module options to "modprobe scsi_debug num_tgts=2" [20021221]
*/

+
+#define pr_fmt(fmt) KBUILD_MODNAME ":%s: " fmt, __func__
+
#include <linux/module.h>

#include <linux/kernel.h>
@@ -2446,8 +2449,7 @@ static int dif_verify(struct sd_dif_tuple *sdt, const void *data,
__be16 csum = dif_compute_csum(data, scsi_debug_sector_size);

if (sdt->guard_tag != csum) {
- pr_err("%s: GUARD check failed on sector %lu rcvd 0x%04x, data 0x%04x\n",
- __func__,
+ pr_err("GUARD check failed on sector %lu rcvd 0x%04x, data 0x%04x\n",
(unsigned long)sector,
be16_to_cpu(sdt->guard_tag),
be16_to_cpu(csum));
@@ -2455,14 +2457,14 @@ static int dif_verify(struct sd_dif_tuple *sdt, const void *data,
}
if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION &&
be32_to_cpu(sdt->ref_tag) != (sector & 0xffffffff)) {
- pr_err("%s: REF check failed on sector %lu\n",
- __func__, (unsigned long)sector);
+ pr_err("REF check failed on sector %lu\n",
+ (unsigned long)sector);
return 0x03;
}
if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION &&
be32_to_cpu(sdt->ref_tag) != ei_lba) {
- pr_err("%s: REF check failed on sector %lu\n",
- __func__, (unsigned long)sector);
+ pr_err("REF check failed on sector %lu\n",
+ (unsigned long)sector);
return 0x03;
}
return 0;
@@ -3449,7 +3451,7 @@ static void sdebug_q_cmd_complete(unsigned long indx)
atomic_inc(&sdebug_completions);
qa_indx = indx;
if ((qa_indx < 0) || (qa_indx >= SCSI_DEBUG_CANQUEUE)) {
- pr_err("%s: wild qa_indx=%d\n", __func__, qa_indx);
+ pr_err("wild qa_indx=%d\n", qa_indx);
return;
}
spin_lock_irqsave(&queued_arr_lock, iflags);
@@ -3457,21 +3459,21 @@ static void sdebug_q_cmd_complete(unsigned long indx)
scp = sqcp->a_cmnd;
if (NULL == scp) {
spin_unlock_irqrestore(&queued_arr_lock, iflags);
- pr_err("%s: scp is NULL\n", __func__);
+ pr_err("scp is NULL\n");
return;
}
devip = (struct sdebug_dev_info *)scp->device->hostdata;
if (devip)
atomic_dec(&devip->num_in_q);
else
- pr_err("%s: devip=NULL\n", __func__);
+ pr_err("devip=NULL\n");
if (atomic_read(&retired_max_queue) > 0)
retiring = 1;

sqcp->a_cmnd = NULL;
if (!test_and_clear_bit(qa_indx, queued_in_use_bm)) {
spin_unlock_irqrestore(&queued_arr_lock, iflags);
- pr_err("%s: Unexpected completion\n", __func__);
+ pr_err("Unexpected completion\n");
return;
}

@@ -3481,7 +3483,7 @@ static void sdebug_q_cmd_complete(unsigned long indx)
retval = atomic_read(&retired_max_queue);
if (qa_indx >= retval) {
spin_unlock_irqrestore(&queued_arr_lock, iflags);
- pr_err("%s: index %d too large\n", __func__, retval);
+ pr_err("index %d too large\n", retval);
return;
}
k = find_last_bit(queued_in_use_bm, retval);
@@ -3509,7 +3511,7 @@ sdebug_q_cmd_hrt_complete(struct hrtimer *timer)
atomic_inc(&sdebug_completions);
qa_indx = sd_hrtp->qa_indx;
if ((qa_indx < 0) || (qa_indx >= SCSI_DEBUG_CANQUEUE)) {
- pr_err("%s: wild qa_indx=%d\n", __func__, qa_indx);
+ pr_err("wild qa_indx=%d\n", qa_indx);
goto the_end;
}
spin_lock_irqsave(&queued_arr_lock, iflags);
@@ -3517,21 +3519,21 @@ sdebug_q_cmd_hrt_complete(struct hrtimer *timer)
scp = sqcp->a_cmnd;
if (NULL == scp) {
spin_unlock_irqrestore(&queued_arr_lock, iflags);
- pr_err("%s: scp is NULL\n", __func__);
+ pr_err("scp is NULL\n");
goto the_end;
}
devip = (struct sdebug_dev_info *)scp->device->hostdata;
if (devip)
atomic_dec(&devip->num_in_q);
else
- pr_err("%s: devip=NULL\n", __func__);
+ pr_err("devip=NULL\n");
if (atomic_read(&retired_max_queue) > 0)
retiring = 1;

sqcp->a_cmnd = NULL;
if (!test_and_clear_bit(qa_indx, queued_in_use_bm)) {
spin_unlock_irqrestore(&queued_arr_lock, iflags);
- pr_err("%s: Unexpected completion\n", __func__);
+ pr_err("Unexpected completion\n");
goto the_end;
}

@@ -3541,7 +3543,7 @@ sdebug_q_cmd_hrt_complete(struct hrtimer *timer)
retval = atomic_read(&retired_max_queue);
if (qa_indx >= retval) {
spin_unlock_irqrestore(&queued_arr_lock, iflags);
- pr_err("%s: index %d too large\n", __func__, retval);
+ pr_err("index %d too large\n", retval);
goto the_end;
}
k = find_last_bit(queued_in_use_bm, retval);
@@ -3580,7 +3582,7 @@ static struct sdebug_dev_info * devInfoReg(struct scsi_device * sdev)
return devip;
sdbg_host = *(struct sdebug_host_info **)shost_priv(sdev->host);
if (!sdbg_host) {
- pr_err("%s: Host info NULL\n", __func__);
+ pr_err("Host info NULL\n");
return NULL;
}
list_for_each_entry(devip, &sdbg_host->dev_info_list, dev_list) {
@@ -3596,8 +3598,7 @@ static struct sdebug_dev_info * devInfoReg(struct scsi_device * sdev)
if (!open_devip) { /* try and make a new one */
open_devip = sdebug_device_create(sdbg_host, GFP_ATOMIC);
if (!open_devip) {
- printk(KERN_ERR "%s: out of memory at line %d\n",
- __func__, __LINE__);
+ pr_err("out of memory at line %d\n", __LINE__);
return NULL;
}
}
@@ -3615,7 +3616,7 @@ static struct sdebug_dev_info * devInfoReg(struct scsi_device * sdev)
static int scsi_debug_slave_alloc(struct scsi_device *sdp)
{
if (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts)
- printk(KERN_INFO "scsi_debug: slave_alloc <%u %u %u %llu>\n",
+ pr_info("slave_alloc <%u %u %u %llu>\n",
sdp->host->host_no, sdp->channel, sdp->id, sdp->lun);
queue_flag_set_unlocked(QUEUE_FLAG_BIDI, sdp->request_queue);
return 0;
@@ -3626,7 +3627,7 @@ static int scsi_debug_slave_configure(struct scsi_device *sdp)
struct sdebug_dev_info *devip;

if (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts)
- printk(KERN_INFO "scsi_debug: slave_configure <%u %u %u %llu>\n",
+ pr_info("slave_configure <%u %u %u %llu>\n",
sdp->host->host_no, sdp->channel, sdp->id, sdp->lun);
if (sdp->host->max_cmd_len != SCSI_DEBUG_MAX_CMD_LEN)
sdp->host->max_cmd_len = SCSI_DEBUG_MAX_CMD_LEN;
@@ -3646,7 +3647,7 @@ static void scsi_debug_slave_destroy(struct scsi_device *sdp)
(struct sdebug_dev_info *)sdp->hostdata;

if (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts)
- printk(KERN_INFO "scsi_debug: slave_destroy <%u %u %u %llu>\n",
+ pr_info("slave_destroy <%u %u %u %llu>\n",
sdp->host->host_no, sdp->channel, sdp->id, sdp->lun);
if (devip) {
/* make this slot available for re-use */
@@ -3897,8 +3898,7 @@ static void __init sdebug_build_parts(unsigned char *ramp,
return;
if (scsi_debug_num_parts > SDEBUG_MAX_PARTS) {
scsi_debug_num_parts = SDEBUG_MAX_PARTS;
- pr_warn("%s: reducing partitions to %d\n", __func__,
- SDEBUG_MAX_PARTS);
+ pr_warn("reducing partitions to %d\n", SDEBUG_MAX_PARTS);
}
num_sectors = (int)sdebug_store_sectors;
sectors_per_part = (num_sectors - sdebug_sectors_per)
@@ -3945,8 +3945,7 @@ schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
struct scsi_device *sdp = cmnd->device;

if (NULL == cmnd || NULL == devip) {
- pr_warn("%s: called with NULL cmnd or devip pointer\n",
- __func__);
+ pr_warn("called with NULL cmnd or devip pointer\n");
/* no particularly good error to report back */
return SCSI_MLQUEUE_HOST_BUSY;
}
@@ -4383,8 +4382,7 @@ static ssize_t fake_rw_store(struct device_driver *ddp, const char *buf,

fake_storep = vmalloc(sz);
if (NULL == fake_storep) {
- pr_err("%s: out of memory, 9\n",
- __func__);
+ pr_err("out of memory, 9\n");
return -ENOMEM;
}
memset(fake_storep, 0, sz);
@@ -4784,8 +4782,7 @@ static int __init scsi_debug_init(void)
atomic_set(&retired_max_queue, 0);

if (scsi_debug_ndelay >= 1000000000) {
- pr_warn("%s: ndelay must be less than 1 second, ignored\n",
- __func__);
+ pr_warn("ndelay must be less than 1 second, ignored\n");
scsi_debug_ndelay = 0;
} else if (scsi_debug_ndelay > 0)
scsi_debug_delay = DELAY_OVERRIDDEN;
@@ -4797,8 +4794,7 @@ static int __init scsi_debug_init(void)
case 4096:
break;
default:
- pr_err("%s: invalid sector_size %d\n", __func__,
- scsi_debug_sector_size);
+ pr_err("invalid sector_size %d\n", scsi_debug_sector_size);
return -EINVAL;
}

@@ -4811,29 +4807,28 @@ static int __init scsi_debug_init(void)
break;

default:
- pr_err("%s: dif must be 0, 1, 2 or 3\n", __func__);
+ pr_err("dif must be 0, 1, 2 or 3\n");
return -EINVAL;
}

if (scsi_debug_guard > 1) {
- pr_err("%s: guard must be 0 or 1\n", __func__);
+ pr_err("guard must be 0 or 1\n");
return -EINVAL;
}

if (scsi_debug_ato > 1) {
- pr_err("%s: ato must be 0 or 1\n", __func__);
+ pr_err("ato must be 0 or 1\n");
return -EINVAL;
}

if (scsi_debug_physblk_exp > 15) {
- pr_err("%s: invalid physblk_exp %u\n", __func__,
- scsi_debug_physblk_exp);
+ pr_err("invalid physblk_exp %u\n", scsi_debug_physblk_exp);
return -EINVAL;
}

if (scsi_debug_lowest_aligned > 0x3fff) {
- pr_err("%s: lowest_aligned too big: %u\n", __func__,
- scsi_debug_lowest_aligned);
+ pr_err("lowest_aligned too big: %u\n",
+ scsi_debug_lowest_aligned);
return -EINVAL;
}

@@ -4863,7 +4858,7 @@ static int __init scsi_debug_init(void)
if (0 == scsi_debug_fake_rw) {
fake_storep = vmalloc(sz);
if (NULL == fake_storep) {
- pr_err("%s: out of memory, 1\n", __func__);
+ pr_err("out of memory, 1\n");
return -ENOMEM;
}
memset(fake_storep, 0, sz);
@@ -4877,11 +4872,10 @@ static int __init scsi_debug_init(void)
dif_size = sdebug_store_sectors * sizeof(struct sd_dif_tuple);
dif_storep = vmalloc(dif_size);

- pr_err("%s: dif_storep %u bytes @ %p\n", __func__, dif_size,
- dif_storep);
+ pr_err("dif_storep %u bytes @ %p\n", dif_size, dif_storep);

if (dif_storep == NULL) {
- pr_err("%s: out of mem. (DIX)\n", __func__);
+ pr_err("out of mem. (DIX)\n");
ret = -ENOMEM;
goto free_vm;
}
@@ -4903,18 +4897,17 @@ static int __init scsi_debug_init(void)
if (scsi_debug_unmap_alignment &&
scsi_debug_unmap_granularity <=
scsi_debug_unmap_alignment) {
- pr_err("%s: ERR: unmap_granularity <= unmap_alignment\n",
- __func__);
+ pr_err("ERR: unmap_granularity <= unmap_alignment\n");
return -EINVAL;
}

map_size = lba_to_map_index(sdebug_store_sectors - 1) + 1;
map_storep = vmalloc(BITS_TO_LONGS(map_size) * sizeof(long));

- pr_info("%s: %lu provisioning blocks\n", __func__, map_size);
+ pr_info("%lu provisioning blocks\n", map_size);

if (map_storep == NULL) {
- pr_err("%s: out of mem. (MAP)\n", __func__);
+ pr_err("out of mem. (MAP)\n");
ret = -ENOMEM;
goto free_vm;
}
@@ -4928,18 +4921,18 @@ static int __init scsi_debug_init(void)

pseudo_primary = root_device_register("pseudo_0");
if (IS_ERR(pseudo_primary)) {
- pr_warn("%s: root_device_register() error\n", __func__);
+ pr_warn("root_device_register() error\n");
ret = PTR_ERR(pseudo_primary);
goto free_vm;
}
ret = bus_register(&pseudo_lld_bus);
if (ret < 0) {
- pr_warn("%s: bus_register error: %d\n", __func__, ret);
+ pr_warn("bus_register error: %d\n", ret);
goto dev_unreg;
}
ret = driver_register(&sdebug_driverfs_driver);
if (ret < 0) {
- pr_warn("%s: driver_register error: %d\n", __func__, ret);
+ pr_warn("driver_register error: %d\n", ret);
goto bus_unreg;
}

@@ -4948,16 +4941,14 @@ static int __init scsi_debug_init(void)

for (k = 0; k < host_to_add; k++) {
if (sdebug_add_adapter()) {
- pr_err("%s: sdebug_add_adapter failed k=%d\n",
- __func__, k);
+ pr_err("sdebug_add_adapter failed k=%d\n", k);
break;
}
}

- if (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts) {
- pr_info("%s: built %d host(s)\n", __func__,
- scsi_debug_add_host);
- }
+ if (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts)
+ pr_info("built %d host(s)\n", scsi_debug_add_host);
+
return 0;

bus_unreg:
@@ -5012,8 +5003,7 @@ static int sdebug_add_adapter(void)

sdbg_host = kzalloc(sizeof(*sdbg_host),GFP_KERNEL);
if (NULL == sdbg_host) {
- printk(KERN_ERR "%s: out of memory at line %d\n",
- __func__, __LINE__);
+ pr_err("out of memory at line %d\n", __LINE__);
return -ENOMEM;
}

@@ -5023,8 +5013,7 @@ static int sdebug_add_adapter(void)
for (k = 0; k < devs_per_host; k++) {
sdbg_devinfo = sdebug_device_create(sdbg_host, GFP_KERNEL);
if (!sdbg_devinfo) {
- printk(KERN_ERR "%s: out of memory at line %d\n",
- __func__, __LINE__);
+ pr_err("out of memory at line %d\n", __LINE__);
error = -ENOMEM;
goto clean;
}
@@ -5338,7 +5327,7 @@ static int sdebug_driver_probe(struct device * dev)
sdebug_driver_template.use_clustering = ENABLE_CLUSTERING;
hpnt = scsi_host_alloc(&sdebug_driver_template, sizeof(sdbg_host));
if (NULL == hpnt) {
- pr_err("%s: scsi_host_alloc failed\n", __func__);
+ pr_err("scsi_host_alloc failed\n");
error = -ENODEV;
return error;
}
@@ -5381,7 +5370,7 @@ static int sdebug_driver_probe(struct device * dev)

scsi_host_set_prot(hpnt, host_prot);

- printk(KERN_INFO "scsi_debug: host protection%s%s%s%s%s%s%s\n",
+ pr_info("host protection%s%s%s%s%s%s%s\n",
(host_prot & SHOST_DIF_TYPE1_PROTECTION) ? " DIF1" : "",
(host_prot & SHOST_DIF_TYPE2_PROTECTION) ? " DIF2" : "",
(host_prot & SHOST_DIF_TYPE3_PROTECTION) ? " DIF3" : "",
@@ -5409,7 +5398,7 @@ static int sdebug_driver_probe(struct device * dev)

error = scsi_add_host(hpnt, &sdbg_host->dev);
if (error) {
- printk(KERN_ERR "%s: scsi_add_host failed\n", __func__);
+ pr_err("scsi_add_host failed\n");
error = -ENODEV;
scsi_host_put(hpnt);
} else
@@ -5426,8 +5415,7 @@ static int sdebug_driver_remove(struct device * dev)
sdbg_host = to_sdebug_host(dev);

if (!sdbg_host) {
- printk(KERN_ERR "%s: Unable to locate host info\n",
- __func__);
+ pr_err("Unable to locate host info\n");
return -ENODEV;
}

--
2.4.3


2015-07-28 13:59:09

by Tomas Winkler

[permalink] [raw]
Subject: [scsi 2/7 RESEND] scsi_debug: use SCSI_W_LUN_REPORT_LUNS instead of SAM2_WLUN_REPORT_LUNS;

use SCSI_W_LUN_REPORT_LUNS from scsi.h instead of localy defined
SAM2_WLUN_REPORT_LUNS

Signed-off-by: Tomas Winkler <[email protected]>
Acked-by: Douglas Gilbert <[email protected]>
---
drivers/scsi/scsi_debug.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 6e6bf0f03cf7..bbe04dae040c 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -204,7 +204,6 @@ static const char *scsi_debug_version_date = "20141022";
/* If REPORT LUNS has luns >= 256 it can choose "flat space" (value 1)
* or "peripheral device" addressing (value 0) */
#define SAM2_LUN_ADDRESS_METHOD 0
-#define SAM2_WLUN_REPORT_LUNS 0xc101

/* SCSI_DEBUG_CANQUEUE is the maximum number of commands that can be queued
* (for response) at one time. Can be reduced by max_queue option. Command
@@ -701,7 +700,7 @@ static void sdebug_max_tgts_luns(void)
else
hpnt->max_id = scsi_debug_num_tgts;
/* scsi_debug_max_luns; */
- hpnt->max_lun = SAM2_WLUN_REPORT_LUNS;
+ hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS;
}
spin_unlock(&sdebug_host_list_lock);
}
@@ -1291,7 +1290,7 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
arr = kzalloc(SDEBUG_MAX_INQ_ARR_SZ, GFP_ATOMIC);
if (! arr)
return DID_REQUEUE << 16;
- have_wlun = (scp->device->lun == SAM2_WLUN_REPORT_LUNS);
+ have_wlun = (scp->device->lun == SCSI_W_LUN_REPORT_LUNS);
if (have_wlun)
pq_pdt = 0x1e; /* present, wlun */
else if (scsi_debug_no_lun_0 && (0 == devip->lun))
@@ -3367,8 +3366,8 @@ static int resp_report_luns(struct scsi_cmnd * scp,
one_lun[i].scsi_lun[1] = lun & 0xff;
}
if (want_wlun) {
- one_lun[i].scsi_lun[0] = (SAM2_WLUN_REPORT_LUNS >> 8) & 0xff;
- one_lun[i].scsi_lun[1] = SAM2_WLUN_REPORT_LUNS & 0xff;
+ one_lun[i].scsi_lun[0] = (SCSI_W_LUN_REPORT_LUNS >> 8) & 0xff;
+ one_lun[i].scsi_lun[1] = SCSI_W_LUN_REPORT_LUNS & 0xff;
i++;
}
alloc_len = (unsigned char *)(one_lun + i) - arr;
@@ -5167,7 +5166,7 @@ scsi_debug_queuecommand(struct scsi_cmnd *scp)
}
sdev_printk(KERN_INFO, sdp, "%s: cmd %s\n", my_name, b);
}
- has_wlun_rl = (sdp->lun == SAM2_WLUN_REPORT_LUNS);
+ has_wlun_rl = (sdp->lun == SCSI_W_LUN_REPORT_LUNS);
if ((sdp->lun >= scsi_debug_max_luns) && !has_wlun_rl)
return schedule_resp(scp, NULL, errsts_no_connect, 0);

@@ -5338,7 +5337,7 @@ static int sdebug_driver_probe(struct device * dev)
hpnt->max_id = scsi_debug_num_tgts + 1;
else
hpnt->max_id = scsi_debug_num_tgts;
- hpnt->max_lun = SAM2_WLUN_REPORT_LUNS; /* = scsi_debug_max_luns; */
+ hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS; /* = scsi_debug_max_luns; */

host_prot = 0;

--
2.4.3

2015-07-28 13:58:46

by Tomas Winkler

[permalink] [raw]
Subject: [scsi 3/7 RESEND] scsi_debug: vfree is null safe so drop the check

Signed-off-by: Tomas Winkler <[email protected]>
Acked-by: Douglas Gilbert <[email protected]>
---
drivers/scsi/scsi_debug.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index bbe04dae040c..e2c509e39765 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4955,10 +4955,8 @@ bus_unreg:
dev_unreg:
root_device_unregister(pseudo_primary);
free_vm:
- if (map_storep)
- vfree(map_storep);
- if (dif_storep)
- vfree(dif_storep);
+ vfree(map_storep);
+ vfree(dif_storep);
vfree(fake_storep);

return ret;
@@ -4976,9 +4974,7 @@ static void __exit scsi_debug_exit(void)
bus_unregister(&pseudo_lld_bus);
root_device_unregister(pseudo_primary);

- if (dif_storep)
- vfree(dif_storep);
-
+ vfree(dif_storep);
vfree(fake_storep);
}

--
2.4.3

2015-07-28 13:57:36

by Tomas Winkler

[permalink] [raw]
Subject: [scsi 4/7 RESEND] scsi_debug: make dump_sector static

fixes warning:
warning: no previous prototype for ‘dump_sector’

Signed-off-by: Tomas Winkler <[email protected]>
Acked-by: Douglas Gilbert <[email protected]>
---
drivers/scsi/scsi_debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index e2c509e39765..3a70683cf9f9 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2681,7 +2681,7 @@ resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
return 0;
}

-void dump_sector(unsigned char *buf, int len)
+static void dump_sector(unsigned char *buf, int len)
{
int i, j, n;

--
2.4.3

2015-07-28 13:58:28

by Tomas Winkler

[permalink] [raw]
Subject: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable check

The function should never be called with cmnd NULL so
put a fat WARN there.
Fix also smatch wraning:
schedule_resp() warn: variable dereferenced before check 'cmnd'

Cc: Douglas Gilbert <[email protected]>
Signed-off-by: Tomas Winkler <[email protected]>
Acked-by: Douglas Gilbert <[email protected]>
---
drivers/scsi/scsi_debug.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 3a70683cf9f9..faa4ddd8decf 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3941,13 +3941,20 @@ schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
unsigned long iflags;
int k, num_in_q, qdepth, inject;
struct sdebug_queued_cmd *sqcp = NULL;
- struct scsi_device *sdp = cmnd->device;
+ struct scsi_device *sdp;
+
+ /* this should never happen */
+ if (WARN_ON(!cmnd))
+ return SCSI_MLQUEUE_HOST_BUSY;

- if (NULL == cmnd || NULL == devip) {
- pr_warn("called with NULL cmnd or devip pointer\n");
+ if (NULL == devip) {
+ pr_warn("called devip == NULL\n");
/* no particularly good error to report back */
return SCSI_MLQUEUE_HOST_BUSY;
}
+
+ sdp = cmnd->device;
+
if ((scsi_result) && (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts))
sdev_printk(KERN_INFO, sdp, "%s: non-zero result=0x%x\n",
__func__, scsi_result);
--
2.4.3

2015-07-28 13:58:03

by Tomas Winkler

[permalink] [raw]
Subject: [scsi 6/7 RESEND] scsi_debug: fix REPORT LUNS Well Known LU

The use case to report 'REPORT LUNS WLUN' described
in scsi_debug documentation didn't work because:
scsi_scan_host_selected() checks for:
lun < shost->max_lun

To fix this we set:
max_lun = SCSI_W_LUN_REPORT_LUNS + 1;

Signed-off-by: Tomas Winkler <[email protected]>
Acked-by: Douglas Gilbert <[email protected]>
---
drivers/scsi/scsi_debug.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index faa4ddd8decf..2e0b3d7dc40f 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -700,7 +700,7 @@ static void sdebug_max_tgts_luns(void)
else
hpnt->max_id = scsi_debug_num_tgts;
/* scsi_debug_max_luns; */
- hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS;
+ hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS + 1;
}
spin_unlock(&sdebug_host_list_lock);
}
@@ -5340,7 +5340,8 @@ static int sdebug_driver_probe(struct device * dev)
hpnt->max_id = scsi_debug_num_tgts + 1;
else
hpnt->max_id = scsi_debug_num_tgts;
- hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS; /* = scsi_debug_max_luns; */
+ /* = scsi_debug_max_luns; */
+ hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS + 1;

host_prot = 0;

--
2.4.3

2015-07-28 13:57:45

by Tomas Winkler

[permalink] [raw]
Subject: [scsi 7/7] scsi_debug: resp_request: remove unused variable

Fixes the following warning

In function ‘resp_requests’:
drivers/scsi//scsi_debug.c:1432:15: warning: variable ‘want_dsense’ set
but not used [-Wunused-but-set-variable]
bool dsense, want_dsense;

Signed-off-by: Tomas Winkler <[email protected]>
---
drivers/scsi/scsi_debug.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 2e0b3d7dc40f..dfcc45bb03b1 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1429,12 +1429,11 @@ static int resp_requests(struct scsi_cmnd * scp,
unsigned char * sbuff;
unsigned char *cmd = scp->cmnd;
unsigned char arr[SCSI_SENSE_BUFFERSIZE];
- bool dsense, want_dsense;
+ bool dsense;
int len = 18;

memset(arr, 0, sizeof(arr));
dsense = !!(cmd[1] & 1);
- want_dsense = dsense || scsi_debug_dsense;
sbuff = scp->sense_buffer;
if ((iec_m_pg[2] & 0x4) && (6 == (iec_m_pg[3] & 0xf))) {
if (dsense) {
--
2.4.3

2015-07-30 17:21:38

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [scsi 1/7 RESEND] scsi_debug: define pr_fmt() for consistent logging

>>>>> "Tomas" == Tomas Winkler <[email protected]> writes:

Tomas> Use pr_fmt with both module name and __func__ Also drop few bare
Tomas> printk leftovers

Tomas> The log format should stay pretty much intact

Reviewed-by: Martin K. Petersen <[email protected]>

--
Martin K. Petersen Oracle Linux Engineering

2015-07-30 17:22:13

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [scsi 2/7 RESEND] scsi_debug: use SCSI_W_LUN_REPORT_LUNS instead of SAM2_WLUN_REPORT_LUNS;

>>>>> "Tomas" == Tomas Winkler <[email protected]> writes:

Tomas> use SCSI_W_LUN_REPORT_LUNS from scsi.h instead of localy defined
Tomas> SAM2_WLUN_REPORT_LUNS

Reviewed-by: Martin K. Petersen <[email protected]>

--
Martin K. Petersen Oracle Linux Engineering

2015-07-30 17:22:41

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [scsi 3/7 RESEND] scsi_debug: vfree is null safe so drop the check

>>>>> "Tomas" == Tomas Winkler <[email protected]> writes:

Reviewed-by: Martin K. Petersen <[email protected]>

--
Martin K. Petersen Oracle Linux Engineering

2015-07-30 17:22:57

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [scsi 4/7 RESEND] scsi_debug: make dump_sector static

>>>>> "Tomas" == Tomas Winkler <[email protected]> writes:

Tomas> fixes warning: warning: no previous prototype for ‘dump_sector’

Reviewed-by: Martin K. Petersen <[email protected]>

--
Martin K. Petersen Oracle Linux Engineering

2015-07-30 17:23:43

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable check

>>>>> "Tomas" == Tomas Winkler <[email protected]> writes:

Tomas> The function should never be called with cmnd NULL so put a fat
Tomas> WARN there. Fix also smatch wraning: schedule_resp() warn:
Tomas> variable dereferenced before check 'cmnd'

Reviewed-by: Martin K. Petersen <[email protected]>

--
Martin K. Petersen Oracle Linux Engineering

2015-07-30 17:24:34

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [scsi 6/7 RESEND] scsi_debug: fix REPORT LUNS Well Known LU

>>>>> "Tomas" == Tomas Winkler <[email protected]> writes:

Tomas> The use case to report 'REPORT LUNS WLUN' described in scsi_debug
Tomas> documentation didn't work because: scsi_scan_host_selected()
Tomas> checks for: lun < shost->max_lun

Tomas> To fix this we set: max_lun = SCSI_W_LUN_REPORT_LUNS + 1;

Reviewed-by: Martin K. Petersen <[email protected]>

--
Martin K. Petersen Oracle Linux Engineering

2015-07-30 17:24:55

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [scsi 7/7] scsi_debug: resp_request: remove unused variable

>>>>> "Tomas" == Tomas Winkler <[email protected]> writes:

Tomas> Fixes the following warning In function ‘resp_requests’:
Tomas> drivers/scsi//scsi_debug.c:1432:15: warning: variable
Tomas> ‘want_dsense’ set but not used [-Wunused-but-set-variable] bool
Tomas> dsense, want_dsense;

Reviewed-by: Martin K. Petersen <[email protected]>

--
Martin K. Petersen Oracle Linux Engineering

2015-08-25 19:52:54

by Ewan Milne

[permalink] [raw]
Subject: Re: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable check

On Tue, 2015-07-28 at 16:54 +0300, Tomas Winkler wrote:
> The function should never be called with cmnd NULL so
> put a fat WARN there.
> Fix also smatch wraning:
> schedule_resp() warn: variable dereferenced before check 'cmnd'
>
> Cc: Douglas Gilbert <[email protected]>
> Signed-off-by: Tomas Winkler <[email protected]>
> Acked-by: Douglas Gilbert <[email protected]>
> ---
> drivers/scsi/scsi_debug.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 3a70683cf9f9..faa4ddd8decf 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -3941,13 +3941,20 @@ schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
> unsigned long iflags;
> int k, num_in_q, qdepth, inject;
> struct sdebug_queued_cmd *sqcp = NULL;
> - struct scsi_device *sdp = cmnd->device;
> + struct scsi_device *sdp;
> +
> + /* this should never happen */
> + if (WARN_ON(!cmnd))
> + return SCSI_MLQUEUE_HOST_BUSY;
>
> - if (NULL == cmnd || NULL == devip) {
> - pr_warn("called with NULL cmnd or devip pointer\n");
> + if (NULL == devip) {
> + pr_warn("called devip == NULL\n");
> /* no particularly good error to report back */
> return SCSI_MLQUEUE_HOST_BUSY;
> }

Please refer to the patch I just posted, we can't return _HOST_BUSY here
if devip == NULL. I posted a fix against the current "misc" branch as
I don't see this patch applied, let me know if I need to update it.

> +
> + sdp = cmnd->device;
> +
> if ((scsi_result) && (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts))
> sdev_printk(KERN_INFO, sdp, "%s: non-zero result=0x%x\n",
> __func__, scsi_result);

2015-08-25 21:04:07

by Tomas Winkler

[permalink] [raw]
Subject: RE: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable check


> > + /* this should never happen */
> > + if (WARN_ON(!cmnd))
> > + return SCSI_MLQUEUE_HOST_BUSY;
> >
> > - if (NULL == cmnd || NULL == devip) {
> > - pr_warn("called with NULL cmnd or devip pointer\n");
> > + if (NULL == devip) {
> > + pr_warn("called devip == NULL\n");
> > /* no particularly good error to report back */
> > return SCSI_MLQUEUE_HOST_BUSY;
> > }
>
> Please refer to the patch I just posted, we can't return _HOST_BUSY here
> if devip == NULL. I posted a fix against the current "misc" branch as
> I don't see this patch applied, let me know if I need to update it.


I'm just not sure why the patches are not merged or even rejected.
I'm submitting patches to the Linux kernel for more than 10 years to various trees and I can agree that these are not some urgent fixes, but this is the first time my effort is ignored for such long time by the maintainer.

Thanks
Tomas

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-08-26 23:38:05

by James Bottomley

[permalink] [raw]
Subject: Re: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable check

On Tue, 2015-08-25 at 21:03 +0000, Winkler, Tomas wrote:
>
> > > + /* this should never happen */
> > > + if (WARN_ON(!cmnd))
> > > + return SCSI_MLQUEUE_HOST_BUSY;
> > >
> > > - if (NULL == cmnd || NULL == devip) {
> > > - pr_warn("called with NULL cmnd or devip pointer\n");
> > > + if (NULL == devip) {
> > > + pr_warn("called devip == NULL\n");
> > > /* no particularly good error to report back */
> > > return SCSI_MLQUEUE_HOST_BUSY;
> > > }
> >
> > Please refer to the patch I just posted, we can't return _HOST_BUSY here
> > if devip == NULL. I posted a fix against the current "misc" branch as
> > I don't see this patch applied, let me know if I need to update it.
>
>
> I'm just not sure why the patches are not merged or even rejected.

Because ideally I want a Maintainer ack. That's Doug Gilbert.

> I'm submitting patches to the Linux kernel for more than 10 years to
> various trees and I can agree that these are not some urgent fixes,
> but this is the first time my effort is ignored for such long time by
> the maintainer.

Well, OK, I trust martin, I'll override the lack of Maintainer ack if
you fix as Ewan suggests.

James


> Thanks
> Tomas
>
> NrybXǧv^)޺{.n+{"{ayʇڙ,jfhzw j:+vwjmzZ+ݢj"!


2015-08-27 19:56:13

by Ewan Milne

[permalink] [raw]
Subject: Re: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable check

On Wed, 2015-08-26 at 16:38 -0700, James Bottomley wrote:
> On Tue, 2015-08-25 at 21:03 +0000, Winkler, Tomas wrote:
> >
> > > > + /* this should never happen */
> > > > + if (WARN_ON(!cmnd))
> > > > + return SCSI_MLQUEUE_HOST_BUSY;
> > > >
> > > > - if (NULL == cmnd || NULL == devip) {
> > > > - pr_warn("called with NULL cmnd or devip pointer\n");
> > > > + if (NULL == devip) {
> > > > + pr_warn("called devip == NULL\n");
> > > > /* no particularly good error to report back */
> > > > return SCSI_MLQUEUE_HOST_BUSY;
> > > > }
> > >
> > > Please refer to the patch I just posted, we can't return _HOST_BUSY here
> > > if devip == NULL. I posted a fix against the current "misc" branch as
> > > I don't see this patch applied, let me know if I need to update it.
> >
> >
> > I'm just not sure why the patches are not merged or even rejected.
>
> Because ideally I want a Maintainer ack. That's Doug Gilbert.
>
> > I'm submitting patches to the Linux kernel for more than 10 years to
> > various trees and I can agree that these are not some urgent fixes,
> > but this is the first time my effort is ignored for such long time by
> > the maintainer.
>
> Well, OK, I trust martin, I'll override the lack of Maintainer ack if
> you fix as Ewan suggests.
>

Just to clarify, I didn't have a problem with Tomas' patch per se, it's
just that my patch won't apply on top of his. I'll submit a v2 if you
want, so you can apply Tomas' patch first. The problem I'm fixing has
been in there for a while. Let me know if you want me to do that.

-Ewan

> James
>
>
> > Thanks
> > Tomas
> >
> > NrybXǧv^)޺{.n+{"{ayʇڙ,jfhzw j:+vwjmzZ+ݢj"!
>
>
>

2015-08-30 10:36:58

by Tomas Winkler

[permalink] [raw]
Subject: RE: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable check

> > >
> > >
> > > I'm just not sure why the patches are not merged or even rejected.
> >
> > Because ideally I want a Maintainer ack. That's Doug Gilbert.

James
The patches were discussed and the ACked by Doug in February and reviewed again after resend by Martin Petersen

> >
> > > I'm submitting patches to the Linux kernel for more than 10 years to
> > > various trees and I can agree that these are not some urgent fixes,
> > > but this is the first time my effort is ignored for such long time by
> > > the maintainer.
> >
> > Well, OK, I trust martin, I'll override the lack of Maintainer ack if
> > you fix as Ewan suggests.

>
> Just to clarify, I didn't have a problem with Tomas' patch per se, it's
> just that my patch won't apply on top of his. I'll submit a v2 if you
> want, so you can apply Tomas' patch first. The problem I'm fixing has
> been in there for a while. Let me know if you want me to do that.
>
Hopefully your patch should be applied above my patches but I can make that effort and stack them on top of yours.
Thanks

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-08-30 16:54:55

by James Bottomley

[permalink] [raw]
Subject: Re: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable check

On Sun, 2015-08-30 at 10:36 +0000, Winkler, Tomas wrote:
> > > >
> > > >
> > > > I'm just not sure why the patches are not merged or even rejected.
> > >
> > > Because ideally I want a Maintainer ack. That's Doug Gilbert.
>
> James
> The patches were discussed and the ACked by Doug in February

7/7 wasn't.

> and reviewed again after resend by Martin Petersen

Yes, that's why I'm willing to override the lack of maintainer ack.
I'll give Doug until Monday first.

James

2015-08-31 08:07:50

by Tomas Winkler

[permalink] [raw]
Subject: RE: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable check

>
> On Sun, 2015-08-30 at 10:36 +0000, Winkler, Tomas wrote:
> > > > >
> > > > >
> > > > > I'm just not sure why the patches are not merged or even rejected.
> > > >
> > > > Because ideally I want a Maintainer ack. That's Doug Gilbert.
> >
> > James
> > The patches were discussed and the ACked by Doug in February
>
> 7/7 wasn't.
James, fair enough, the reset though can be merged, right ?

>
> > and reviewed again after resend by Martin Petersen
>
> Yes, that's why I'm willing to override the lack of maintainer ack.
> I'll give Doug until Monday first.

Okay.

Thanks
Tomas


????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?