Re: [PATCH for v3.19, v2] Avoid that sd_shutdown() triggers a kernel warning

2015-01-20 Thread Alan Stern
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-20 Thread Akinobu Mita
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

2015-01-20 Thread James Bottomley
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

2015-01-20 Thread Tejun Heo
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

2015-01-20 Thread Alan Stern
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

2015-01-20 Thread Andy Grover

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

2015-01-20 Thread Nicholas A. Bellinger
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

2015-01-20 Thread Nicholas A. Bellinger
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

2015-01-20 Thread Martin K. Petersen
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

2015-01-20 Thread Martin K. Petersen
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

2015-01-20 Thread Nicholas A. Bellinger
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

2015-01-20 Thread Martin K. Petersen
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