Re: [PATCH for v3.19, v2] Avoid that sd_shutdown() triggers a kernel warning
On Wed, 14 Jan 2015, Christoph Hellwig wrote: On Mon, Jan 12, 2015 at 11:29:15AM -0500, Alan Stern wrote: This seems like a good idea and the obvious (once it has been pointed out!) approach. Perhaps not directly related to the issue at hand is this question: In scsi_rescan_device() we will now have: mutex_lock(shost-scan_mutex); if (dev-driver try_module_get(dev-driver-owner)) { struct scsi_driver *drv = to_scsi_driver(dev-driver); if (drv-rescan) drv-rescan(dev); module_put(dev-driver-owner); } mutex_unlock(shost-scan_mutex); What prevents the device from being unbound from its driver while the rescan runs? Evaluating the argument to the module_put() would then dereference a NULL pointer. Unbind events that happen through the normal scsi_remove_host() mechanism are fine, because scsi_remove_host() locks the scan_mutex. But what about writes to the driver's sysfs unbind attribute? Looks like we should still get an unconditional reference to the device using get_device in scsi_rescan_device at least. But this seems like a more generic problem, and at least a quick glance at the pci_driver methods seems like others don't have a good synchroniation of -remove against random driver methods. This particular problem comes down to the fact that scsi_rescan_device() accesses dev-driver without appropriate mutual exclusion. SCSI's scan_mutex won't help because it doesn't protect dev-driver. Rather, dev-driver is protected by dev-mutex, and so scsi_rescan_device() needs to use device_lock/unlock. This suggests that the scan_mutex may not be necessary at all. Historically, it seems to be quite old, predating the device model. Now that we have the device model, maybe scan_mutex simply isn't needed. Scanning for channels or targets beneath a host should be protected by shost-gendev.mutex. Scanning for logical units beneath a target should be protected by starget-dev.mutex. Scanning for partitions beneath a SCSI drive should be protected by sdev-sdev_gendev.mutex. James, here's a related question. Suppose userspace writes to the rescan attribute file for a disk drive for sd_probe_async() has started. What will happen? What _ought_ to happen? Do we need to call async_synchronize_full_domain(scsi_sd_probe_domain); somewhere in this pathway, or will it be okay? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host
2015-01-19 23:22 GMT+09:00 Tejun Heo t...@kernel.org: On Mon, Jan 19, 2015 at 12:05:58AM +0900, Akinobu Mita wrote: While accessing a scsi_device, the use count of the underlying LLDD module is incremented. The module reference is retrieved through .module field of struct scsi_host_template. This mapping between scsi_device and underlying LLDD module works well except some drivers which consist with the core driver and the actual LLDDs and scsi_host_template is defined in the core driver. In these cases, the actual LLDDs can be unloaded even if the scsi_device is being accessed. This patch series fixes the module reference mismatch problem for ufs, usb-storage, esp_scsi, ahci_platform, and pata_platform drivers by moving owner module reference field from struct scsi_host_template to struct Scsi_Host and allowing the LLDDs to set their correct module reference. Hmmm, can't we just move sht definitions to actual LLDDs? libata can do that easily. sht, as its name implies, is the template for creating the scsi_hosts of a given type. We're now just moving module ownership from sht definition site to whatever callsite the actual instance is being created which can also be wrapped in a separate layer requiring explicit propagation. Why not just propagate sht's directly? What's the difference? The reason I didn't move sht from the core driver to the LLDDs for fixing ufs and ums-* in the first place is to avoid exporting many symbols for callbacks in sht. But I realized that we can do it without that many exported symbols by creating a single function that returns a kmemdup()ed sht with a few change including -module. So there are three options we can take for fixing this problem. I would like to know the opinions which one should be taken. (1) Add owner module field to Scsi_Host for allowing LLDDs to adjust it after scsi host allocation. This approach is used by v1,2,3 of this patch series and the scsi midlayer change is minimum. (2) Move owner module field from scsi_host_template to Scsi_Host. The owner module reference is retrieved from the callsite of scsi_host_alloc() by passing THIS_MODULE to the extra argument. The scsi midlayer change is small, but we need the same macro trick for each scsi_host_alloc() wrapper and these changes are relatively large (required by libata, libiscsi, libfc, cxgbi, 53c700, legacy). This approach is used by v4 of this patch series. (3) Allocate scsi host template for each module. No scsi midlayer change is required. Instead of sharing a single scsi host template defined in the core, create a single function that returns a kmemdup()ed sht with a few change including -module so that the sub modules can use it as their sht. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: module: fix module_refcount() return when running in a module exit routine
On Tue, 2015-01-20 at 11:15 +1030, Rusty Russell wrote: James Bottomley james.bottom...@hansenpartnership.com writes: On Mon, 2015-01-19 at 16:21 +1030, Rusty Russell wrote: Masami Hiramatsu masami.hiramatsu...@hitachi.com writes: (2015/01/19 1:55), James Bottomley wrote: From: James Bottomley jbottom...@parallels.com After e513cc1 module: Remove stop_machine from module unloading, module_refcount() is returning (unsigned long)-1 when called from within a routine that runs in module_exit. This is confusing the scsi device put code which is coded to detect a module_refcount() of zero for running within a module exit routine and not try to do another module_put. The fix is to restore the original behaviour of module_refcount() and return zero if we're running inside an exit routine. Fixes: e513cc1c07e2ab93a4514eec9833e031df3e30bb Reported-by: Bart Van Assche bvanass...@acm.org Signed-off-by: James Bottomley jbottom...@parallels.com Yes, this should be fixed as you said, since it must return unsigned long value. But there are only three non-module callers: drivers/scsi/scsi.c:1012: if (module module_refcount(module) != 0) drivers/staging/lustre/lustre/obdclass/lu_object.c:1359: LINVRNT(module_refcount(key-lct_owner) 0); include/linux/module.h:447:unsigned long module_refcount(struct module *mod); kernel/debug/kdb/kdb_main.c:2026: kdb_printf(%4ld , module_refcount(mod)); kernel/module.c:775:unsigned long module_refcount(struct module *mod) kernel/module.c:779:EXPORT_SYMBOL(module_refcount); kernel/module.c:859: seq_printf(m, %lu , module_refcount(mod)); kernel/module.c:911: return sprintf(buffer, %lu\n, module_refcount(mk-mod)); The first one I think should be eliminated, and the second one is simply an assertion before calling module_put() (which should probably be eliminated). The others are just printing information. If you really want to insist on module_reference() returning -1 when the module is in it's exit phase, OK, but in that case, I think it should return a signed value, not an unsigned one. Sure; I just didn't want to paper over the problem here. And I'm not sure we want to lose information, eg. in kgdb we're presumably looking at it because something went wrong... Thanks, Rusty. Subject: module: make module_refcount() a signed integer. James Bottomley points out that it will be -1 during unload. It's only used for diagnostics, so let's not hide that as it could be a clue as to what's gone wrong. Cc: Jason Wessel jason.wes...@windriver.com Signed-off-by: Rusty Russell ru...@rustcorp.com.au diff --git a/include/linux/module.h b/include/linux/module.h index ebfb0e153c6a..b653d7c0a05a 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -444,7 +444,7 @@ extern void __module_put_and_exit(struct module *mod, long code) #define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code) #ifdef CONFIG_MODULE_UNLOAD -unsigned long module_refcount(struct module *mod); +int module_refcount(struct module *mod); void __symbol_put(const char *symbol); #define symbol_put(x) __symbol_put(VMLINUX_SYMBOL_STR(x)) void symbol_put_addr(void *addr); diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index f191bddf64b8..7b40c5f07dce 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -2023,7 +2023,7 @@ static int kdb_lsmod(int argc, const char **argv) kdb_printf(%-20s%8u 0x%p , mod-name, mod-core_size, (void *)mod); #ifdef CONFIG_MODULE_UNLOAD - kdb_printf(%4ld , module_refcount(mod)); + kdb_printf(%4d , module_refcount(mod)); #endif if (mod-state == MODULE_STATE_GOING) kdb_printf( (Unloading)); diff --git a/kernel/module.c b/kernel/module.c index 3965511ae133..2387c98347c1 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -772,9 +772,9 @@ static int try_stop_module(struct module *mod, int flags, int *forced) return 0; } -unsigned long module_refcount(struct module *mod) +int module_refcount(struct module *mod) { - return (unsigned long)atomic_read(mod-refcnt) - MODULE_REF_BASE; + return atomic_read(mod-refcnt) - MODULE_REF_BASE; } EXPORT_SYMBOL(module_refcount); @@ -856,7 +856,7 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod) struct module_use *use; int printed_something = 0; - seq_printf(m, %lu , module_refcount(mod)); + seq_printf(m, %i , module_refcount(mod)); /* * Always include a trailing , so userspace can differentiate @@ -908,7 +908,7 @@ EXPORT_SYMBOL_GPL(symbol_put_addr); static ssize_t show_refcnt(struct module_attribute *mattr, struct module_kobject *mk, char
Re: [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host
Hello, Akinobu. On Tue, Jan 20, 2015 at 11:57:37PM +0900, Akinobu Mita wrote: The reason I didn't move sht from the core driver to the LLDDs for fixing ufs and ums-* in the first place is to avoid exporting many symbols for callbacks in sht. But I realized that we can do it without that many exported symbols by creating a single function that returns a kmemdup()ed sht with a few change including -module. Hmmm, libata already exports most of the necessary symbols. libahci or platform drivers might have to export more but that shouldn't be much. For libata, pushing sht's to the leaf drivers would make far more sense as sht's already get inherited and modified along the hierarchy. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host
On Tue, 20 Jan 2015, Akinobu Mita wrote: 2015-01-19 23:22 GMT+09:00 Tejun Heo t...@kernel.org: On Mon, Jan 19, 2015 at 12:05:58AM +0900, Akinobu Mita wrote: While accessing a scsi_device, the use count of the underlying LLDD module is incremented. The module reference is retrieved through .module field of struct scsi_host_template. This mapping between scsi_device and underlying LLDD module works well except some drivers which consist with the core driver and the actual LLDDs and scsi_host_template is defined in the core driver. In these cases, the actual LLDDs can be unloaded even if the scsi_device is being accessed. This patch series fixes the module reference mismatch problem for ufs, usb-storage, esp_scsi, ahci_platform, and pata_platform drivers by moving owner module reference field from struct scsi_host_template to struct Scsi_Host and allowing the LLDDs to set their correct module reference. Hmmm, can't we just move sht definitions to actual LLDDs? libata can do that easily. sht, as its name implies, is the template for creating the scsi_hosts of a given type. We're now just moving module ownership from sht definition site to whatever callsite the actual instance is being created which can also be wrapped in a separate layer requiring explicit propagation. Why not just propagate sht's directly? What's the difference? The reason I didn't move sht from the core driver to the LLDDs for fixing ufs and ums-* in the first place is to avoid exporting many symbols for callbacks in sht. But I realized that we can do it without that many exported symbols by creating a single function that returns a kmemdup()ed sht with a few change including -module. So there are three options we can take for fixing this problem. I would like to know the opinions which one should be taken. (1) Add owner module field to Scsi_Host for allowing LLDDs to adjust it after scsi host allocation. This approach is used by v1,2,3 of this patch series and the scsi midlayer change is minimum. (2) Move owner module field from scsi_host_template to Scsi_Host. The owner module reference is retrieved from the callsite of scsi_host_alloc() by passing THIS_MODULE to the extra argument. The scsi midlayer change is small, but we need the same macro trick for each scsi_host_alloc() wrapper and these changes are relatively large (required by libata, libiscsi, libfc, cxgbi, 53c700, legacy). This approach is used by v4 of this patch series. (3) Allocate scsi host template for each module. No scsi midlayer change is required. Instead of sharing a single scsi host template defined in the core, create a single function that returns a kmemdup()ed sht with a few change including -module so that the sub modules can use it as their sht. (3) means duplicating a reasonably large data structure in order to alter just one field. It also means changing all the subdrivers to make them call the new function. (1) is the simplest. Since the use of subdrivers in general tends to be a special case (most SCSI drivers don't do it), I prefer to keep the code optimized for it. In other words, I prefer option (1). If people think (2) is better, it can always be layered on top of (1). Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
tcmu-runner (target userspace passthrough daemon) development
Hi all, tcmu-runner is a userspace daemon that simplifies the configuration and processing of SCSI commands from LIO to userspace handlers, via the new TCMU userspace passthrough backstore. https://github.com/agrover/tcmu-runner As a proof-of-concept, I've implemented a Gluster backend handler. I'm looking for code review and collaborators, as well as suggestions on what other userspace handlers we might want to work on. Tape or optical jukebox emulation? VMDK files?? Video RAM??? Thanks -- Regards -- Andy (BTW, tcmu-runner is not kernel code but it's very tightly linked to LIO, so recommend we use this list (target-devel) for discussion.) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: qla2xxx: print port name via %*phC
On Thu, 2015-01-15 at 13:28 +0200, Andy Shevchenko wrote: Instead of pushing each byte via stack let's use custom specifier which allows to print small buffers as a hex string. Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com --- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c index 73f9fee..99f43b7 100644 --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c @@ -1570,9 +1570,7 @@ static int tcm_qla2xxx_check_initiator_node_acl( * match the format by tcm_qla2xxx explict ConfigFS NodeACLs. */ memset(port_name, 0, 36); - snprintf(port_name, 36, %02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x, - fc_wwpn[0], fc_wwpn[1], fc_wwpn[2], fc_wwpn[3], fc_wwpn[4], - fc_wwpn[5], fc_wwpn[6], fc_wwpn[7]); + snprintf(port_name, sizeof(port_name), %8phC, fc_wwpn); /* * Locate our struct se_node_acl either from an explict NodeACL created * via ConfigFS, or via running in TPG demo mode. Picking this one up, since it's specific to tcm_qla2xxx code. Applied to target-pending/for-next. Thanks Andy! --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: tcmu-runner (target userspace passthrough daemon) development
On Tue, 2015-01-20 at 15:27 -0800, Andy Grover wrote: Hi all, tcmu-runner is a userspace daemon that simplifies the configuration and processing of SCSI commands from LIO to userspace handlers, via the new TCMU userspace passthrough backstore. https://github.com/agrover/tcmu-runner Thanks for posting. ;) As a proof-of-concept, I've implemented a Gluster backend handler. I'm looking for code review and collaborators, as well as suggestions on what other userspace handlers we might want to work on. Tape or optical jukebox emulation? VMDK files?? Video RAM??? How about a user-space handler for qcow2, so we could have a mechanism for exposing qcow2 images into KVM guest using vhost-scsi export..? Ideally there would be a library that QEMU provides so that from TCMU's user-space handler perspective, it's simply issuing fd reads/writes into qcow2 library code that is doing the fancy feature stuff outside of user-space handler logic.. --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] block: Add discard flag to blkdev_issue_zeroout() function
blkdev_issue_discard() will zero a given block range. This is done by way of explicit writing, thus provisioning or allocating the blocks on disk. There are use cases where the desired behavior is to zero the blocks but unprovision them if possible. The blocks must deterministically contain zeroes when they are subsequently read back. This patch adds a flag to blkdev_issue_zeroout() that provides this variant. If the discard flag is set and a block device guarantees discard_zeroes_data we will use REQ_DISCARD to clear the block range. If the device does not support discard_zeroes_data or if the discard request fails we will fall back to first REQ_WRITE_SAME and then a regular REQ_WRITE. Also update the callers of blkdev_issue_zero() to reflect the new flag and make sb_issue_zeroout() prefer the discard approach. Signed-off-by: Martin K. Petersen martin.peter...@oracle.com Reviewed-by: Christoph Hellwig h...@lst.de --- block/blk-lib.c| 30 ++ block/ioctl.c | 2 +- drivers/block/drbd/drbd_receiver.c | 2 +- include/linux/blkdev.h | 4 ++-- 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/block/blk-lib.c b/block/blk-lib.c index 8411be3c19d3..715e948f58a4 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -283,23 +283,45 @@ static int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector, * @sector:start sector * @nr_sects: number of sectors to write * @gfp_mask: memory allocation flags (for bio_alloc) + * @discard: whether to discard the block range * * Description: - * Generate and issue number of bios with zerofiled pages. + + * Zero-fill a block range. If the discard flag is set and the block + * device guarantees that subsequent READ operations to the block range + * in question will return zeroes, the blocks will be discarded. Should + * the discard request fail, if the discard flag is not set, or if + * discard_zeroes_data is not supported, this function will resort to + * zeroing the blocks manually, thus provisioning (allocating, + * anchoring) them. If the block device supports the WRITE SAME command + * blkdev_issue_zeroout() will use it to optimize the process of + * clearing the block range. Otherwise the zeroing will be performed + * using regular WRITE calls. */ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector, -sector_t nr_sects, gfp_t gfp_mask) +sector_t nr_sects, gfp_t gfp_mask, bool discard) { + struct request_queue *q = bdev_get_queue(bdev); + unsigned char bdn[BDEVNAME_SIZE]; + + if (discard blk_queue_discard(q) q-limits.discard_zeroes_data) { + + if (!blkdev_issue_discard(bdev, sector, nr_sects, gfp_mask, 0)) + return 0; + + bdevname(bdev, bdn); + pr_warn(%s: DISCARD failed. Manually zeroing.\n, bdn); + } + if (bdev_write_same(bdev)) { - unsigned char bdn[BDEVNAME_SIZE]; if (!blkdev_issue_write_same(bdev, sector, nr_sects, gfp_mask, ZERO_PAGE(0))) return 0; bdevname(bdev, bdn); - pr_err(%s: WRITE SAME failed. Manually zeroing.\n, bdn); + pr_warn(%s: WRITE SAME failed. Manually zeroing.\n, bdn); } return __blkdev_issue_zeroout(bdev, sector, nr_sects, gfp_mask); diff --git a/block/ioctl.c b/block/ioctl.c index 6c7bf903742f..7d8befde2aca 100644 --- a/block/ioctl.c +++ b/block/ioctl.c @@ -198,7 +198,7 @@ static int blk_ioctl_zeroout(struct block_device *bdev, uint64_t start, if (start + len (i_size_read(bdev-bd_inode) 9)) return -EINVAL; - return blkdev_issue_zeroout(bdev, start, len, GFP_KERNEL); + return blkdev_issue_zeroout(bdev, start, len, GFP_KERNEL, false); } static int put_ushort(unsigned long arg, unsigned short val) diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c index d169b4a79267..cee20354ac37 100644 --- a/drivers/block/drbd/drbd_receiver.c +++ b/drivers/block/drbd/drbd_receiver.c @@ -1388,7 +1388,7 @@ int drbd_submit_peer_request(struct drbd_device *device, list_add_tail(peer_req-w.list, device-active_ee); spin_unlock_irq(device-resource-req_lock); if (blkdev_issue_zeroout(device-ldev-backing_bdev, - sector, data_size 9, GFP_NOIO)) + sector, data_size 9, GFP_NOIO, false)) peer_req-flags |= EE_WAS_ERROR; drbd_endio_write_sec_final(peer_req); return 0; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 92f4b4b288dd..669f747ce947 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1162,7 +1162,7 @@ extern int blkdev_issue_discard(struct
[PATCH 3/3] scsi: Remove VPD quirk for Seagate drives
Now that we sanity check the optimal I/O size reported by the device we no longer need to blacklist the VPD pages on certain Seagate drives. Signed-off-by: Martin K. Petersen martin.peter...@oracle.com Cc: sta...@vger.kernel.org --- drivers/scsi/scsi_devinfo.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c index 0118ddf163eb..ae86869ab6b6 100644 --- a/drivers/scsi/scsi_devinfo.c +++ b/drivers/scsi/scsi_devinfo.c @@ -231,7 +231,6 @@ static struct { {SanDisk, ImageMate CF-SD1, NULL, BLIST_FORCELUN}, {SEAGATE, ST34555N, 0930, BLIST_NOTQ},/* Chokes on tagged INQUIRY */ {SEAGATE, ST3390N, 9546, BLIST_NOTQ}, - {SEAGATE, ST900MM0006, NULL, BLIST_SKIP_VPD_PAGES}, {SGI, RAID3, *, BLIST_SPARSELUN}, {SGI, RAID5, *, BLIST_SPARSELUN}, {SGI, TP9100, *, BLIST_REPORTLUN2}, -- 1.9.3 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iscsi-target: use '%*ph' specifier to dump hex buffer
On Thu, 2015-01-15 at 13:40 +0200, Andy Shevchenko wrote: Instead of pushing each byte via stack the %*ph specifier allows to supply just a pointer and length of the buffer. The patch converts code to use the specifier. Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com --- drivers/target/iscsi/iscsi_target_configfs.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c index 9059c1e..526becd 100644 --- a/drivers/target/iscsi/iscsi_target_configfs.c +++ b/drivers/target/iscsi/iscsi_target_configfs.c @@ -674,12 +674,9 @@ static ssize_t lio_target_nacl_show_info( rb += sprintf(page+rb, InitiatorAlias: %s\n, sess-sess_ops-InitiatorAlias); - rb += sprintf(page+rb, LIO Session ID: %u - ISID: 0x%02x %02x %02x %02x %02x %02x - TSIH: %hu , sess-sid, - sess-isid[0], sess-isid[1], sess-isid[2], - sess-isid[3], sess-isid[4], sess-isid[5], - sess-tsih); + rb += sprintf(page+rb, + LIO Session ID: %u ISID: 0x%6ph TSIH: %hu , + sess-sid, sess-isid, sess-tsih); rb += sprintf(page+rb, SessionType: %s\n, (sess-sess_ops-SessionType) ? Discovery : Normal); @@ -1758,9 +1755,7 @@ static u32 lio_sess_get_initiator_sid( /* * iSCSI Initiator Session Identifier from RFC-3720. */ - return snprintf(buf, size, %02x%02x%02x%02x%02x%02x, - sess-isid[0], sess-isid[1], sess-isid[2], - sess-isid[3], sess-isid[4], sess-isid[5]); + return snprintf(buf, size, %6phN, sess-isid); } static int lio_queue_data_in(struct se_cmd *se_cmd) Applied to target-pending/for-next. Thanks Andy! --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] sd: Sanity check the optimal I/O size
We have come across a couple of devices that report crackpot values in the optimal I/O size in the Block Limits VPD page. Since this is a 32-bit entity that gets multiplied by the logical block size we can get disproportionately large values reported to the block layer. Cap io_opt at 256 MB. Reported-by: Chris Friesen chris.frie...@windriver.com Signed-off-by: Martin K. Petersen martin.peter...@oracle.com Cc: sta...@vger.kernel.org --- drivers/scsi/sd.c | 3 ++- drivers/scsi/sd.h | 9 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index ebf35cb64216..faf3725492ce 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2593,7 +2593,8 @@ static void sd_read_block_limits(struct scsi_disk *sdkp) blk_queue_io_min(sdkp-disk-queue, get_unaligned_be16(buffer[6]) * sector_sz); blk_queue_io_opt(sdkp-disk-queue, -get_unaligned_be32(buffer[12]) * sector_sz); +min_t(unsigned int, SD_MAX_IO_OPT_BYTES, + get_unaligned_be32(buffer[12]) * sector_sz)); if (buffer[3] == 0x3c) { unsigned int lba_count, desc_count; diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h index 63ba5ca7f9a1..f175a3f2944a 100644 --- a/drivers/scsi/sd.h +++ b/drivers/scsi/sd.h @@ -44,10 +44,11 @@ enum { }; enum { - SD_DEF_XFER_BLOCKS = 0x, - SD_MAX_XFER_BLOCKS = 0x, - SD_MAX_WS10_BLOCKS = 0x, - SD_MAX_WS16_BLOCKS = 0x7f, + SD_DEF_XFER_BLOCKS = 0x, + SD_MAX_XFER_BLOCKS = 0x, + SD_MAX_WS10_BLOCKS = 0x, + SD_MAX_WS16_BLOCKS = 0x7f, + SD_MAX_IO_OPT_BYTES = 256 * 1024 * 1024, }; enum { -- 1.9.3 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html