Re: [PATCH v3] qla2xxx: silence two GCC warnings

2013-01-30 Thread Saurav Kashyap


On Mon, 2012-10-08 at 11:15 -0500, Saurav Kashyap wrote:
 Acked-by: Saurav Kashyap saurav.kash...@qlogic.com

 Thanks,
 ~Saurav

 Compiling qla_gs.o (part of the qla2xxx module) triggers two GCC
 warnings:
 drivers/scsi/qla2xxx/qla_gs.c: In function Œqla2x00_fdmi_rhba¹:
 drivers/scsi/qla2xxx/qla_gs.c:1339:7: warning: array subscript is
 above array bounds [-Warray-bounds]
 drivers/scsi/qla2xxx/qla_gs.c: In function Œqla2x00_fdmi_register¹:
 drivers/scsi/qla2xxx/qla_gs.c:1663:15: warning: array subscript is
 above array bounds [-Warray-bounds]

This patch was originally posted to silence two GCC warnings while
building v3.6-rc7. Basically identical warnings can still be seen while
building v3.8-rc5. What's the status of this patch?

Hi Paul,
I am submitting some correction patches today and this patch will be part
of the scsi-misc submission after that set.

Thanks,
~Saurav


Paul Bolle

--
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





This message and any attached documents contain information from QLogic 
Corporation or its wholly-owned subsidiaries that may be confidential. If you 
are not the intended recipient, you may not read, copy, distribute, or use this 
information. If you have received this transmission in error, please notify the 
sender immediately by reply e-mail and then delete this message.


Re: [PATCH v3] qla2xxx: silence two GCC warnings

2013-01-30 Thread Paul Bolle
On Wed, 2013-01-30 at 08:07 +, Saurav Kashyap wrote:
 I am submitting some correction patches today and this patch will be part
 of the scsi-misc submission after that set.

Thanks.


Paul Bolle

--
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] libosd: check for kzalloc() failure

2013-01-30 Thread walter harms


Am 30.01.2013 08:06, schrieb Dan Carpenter:
 There wasn't any error handling for this kzalloc().
 
 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
 
 diff --git a/drivers/scsi/osd/osd_initiator.c 
 b/drivers/scsi/osd/osd_initiator.c
 index c06b8e5..d8293f2 100644
 --- a/drivers/scsi/osd/osd_initiator.c
 +++ b/drivers/scsi/osd/osd_initiator.c
 @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev *od,
   odi-osdname_len = get_attrs[a].len;
   /* Avoid NULL for memcmp optimization 0-length is good enough */
   odi-osdname = kzalloc(odi-osdname_len + 1, GFP_KERNEL);
 + if (!odi-osdname) {
 + ret = -ENOMEM;
 + goto out;
 + }
   if (odi-osdname_len)
   memcpy(odi-osdname, get_attrs[a].val_ptr, odi-osdname_len);
   OSD_INFO(OSD_NAME   [%s]\n, odi-osdname);
 --

this looks like strdup() ?

re,
 wh
--
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] libosd: check for kzalloc() failure

2013-01-30 Thread Dan Carpenter
On Wed, Jan 30, 2013 at 09:15:43AM +0100, walter harms wrote:
 
 
 Am 30.01.2013 08:06, schrieb Dan Carpenter:
  There wasn't any error handling for this kzalloc().
  
  Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
  
  diff --git a/drivers/scsi/osd/osd_initiator.c 
  b/drivers/scsi/osd/osd_initiator.c
  index c06b8e5..d8293f2 100644
  --- a/drivers/scsi/osd/osd_initiator.c
  +++ b/drivers/scsi/osd/osd_initiator.c
  @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev 
  *od,
  odi-osdname_len = get_attrs[a].len;
  /* Avoid NULL for memcmp optimization 0-length is good enough */
  odi-osdname = kzalloc(odi-osdname_len + 1, GFP_KERNEL);
  +   if (!odi-osdname) {
  +   ret = -ENOMEM;
  +   goto out;
  +   }
  if (odi-osdname_len)
  memcpy(odi-osdname, get_attrs[a].val_ptr, odi-osdname_len);
  OSD_INFO(OSD_NAME   [%s]\n, odi-osdname);
  --
 
 this looks like strdup() ?
 

Maybe?  It's a funny thing going on with the NUL terminator and I
don't understand what the comment is about.

It appears that normally get_attrs[a].val_ptr is a NUL terminated
string but get_attrs[a].len does not count the terminator.

Odd.

regards,
dan carpenter

--
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] target/iblock: Use backend REQ_FLUSH hint for WriteCacheEnabled status

2013-01-30 Thread Nicholas A. Bellinger
On Wed, 2013-01-30 at 15:39 +0800, Asias He wrote:
 On 01/30/2013 02:57 PM, Nicholas A. Bellinger wrote:
  From: Nicholas Bellinger n...@linux-iscsi.org
  
  This patch allows IBLOCK to check block hints in request_queue-flush_flags
  when reporting current backend device WriteCacheEnabled status to a remote
  SCSI initiator port.
  
  This is done via a se_subsystem_api-get_write_cache() call instead of a
  backend se_device creation time flag, as we expect REQ_FLUSH bits to 
  possibly
  change from an underlying blk_queue_flush() by the SCSI disk driver, or
  internal raw struct block_device driver usage.
  
  Also go ahead and update iblock_execute_rw() bio I/O path code to use
  REQ_FLUSH + REQ_FUA hints when determining WRITE_FUA usage, and make SPC
  emulation code use a spc_check_dev_wce() helper to handle both types of
  cases for virtual backend subsystem drivers.
  
  Reported-by: majianpeng majianp...@gmail.com
  Cc: majianpeng majianp...@gmail.com
  Cc: Christoph Hellwig h...@infradead.org
  Cc: Jens Axboe ax...@kernel.dk
  Cc: James Bottomley jbottom...@parallels.com
  Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
  ---
   drivers/target/target_core_device.c  |6 ++
   drivers/target/target_core_iblock.c  |   31 +++
   drivers/target/target_core_spc.c |   21 ++---
   include/target/target_core_backend.h |1 +
   4 files changed, 48 insertions(+), 11 deletions(-)
  
  diff --git a/drivers/target/target_core_device.c 
  b/drivers/target/target_core_device.c
  index e269510..1d71930 100644
  --- a/drivers/target/target_core_device.c
  +++ b/drivers/target/target_core_device.c
  @@ -772,6 +772,12 @@ int se_dev_set_emulate_write_cache(struct se_device 
  *dev, int flag)
  pr_err(emulate_write_cache not supported for pSCSI\n);
  return -EINVAL;
  }
  +   if (dev-transport-get_write_cache != NULL) {
 
 one nit:
 
   if (dev-transport-get_write_cache) {
 
 ?

Dropping extra NULL comparison here.

 
  +   pr_warn(emulate_write_cache cannot be changed when underlying
  +HW reports WriteCacheEnabled, ignoring request\n);
  +   return 0;
  +   }
  +
  dev-dev_attrib.emulate_write_cache = flag;
  pr_debug(dev[%p]: SE Device WRITE_CACHE_EMULATION flag: %d\n,
  dev, dev-dev_attrib.emulate_write_cache);
  diff --git a/drivers/target/target_core_iblock.c 
  b/drivers/target/target_core_iblock.c
  index b526d23..fc45683 100644
  --- a/drivers/target/target_core_iblock.c
  +++ b/drivers/target/target_core_iblock.c
  @@ -154,6 +154,7 @@ static int iblock_configure_device(struct se_device 
  *dev)
   
  if (blk_queue_nonrot(q))
  dev-dev_attrib.is_nonrot = 1;
  +
  return 0;
   
   out_free_bioset:
  @@ -654,20 +655,24 @@ iblock_execute_rw(struct se_cmd *cmd)
  u32 sg_num = sgl_nents;
  sector_t block_lba;
  unsigned bio_cnt;
  -   int rw;
  +   int rw = 0;
  int i;
   
  if (data_direction == DMA_TO_DEVICE) {
  +   struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
  +   struct request_queue *q = bdev_get_queue(ib_dev-ibd_bd);
  /*
  -* Force data to disk if we pretend to not have a volatile
  -* write cache, or the initiator set the Force Unit Access bit.
  +* Force writethrough using WRITE_FUA if a volatile write cache
  +* is not enabled, or if initiator set the Force Unit Access 
  bit.
   */
  -   if (dev-dev_attrib.emulate_write_cache == 0 ||
  -   (dev-dev_attrib.emulate_fua_write  0 
  -(cmd-se_cmd_flags  SCF_FUA)))
  -   rw = WRITE_FUA;
  -   else
  +   if (q-flush_flags  REQ_FUA) {
  +   if (cmd-se_cmd_flags  SCF_FUA)
  +   rw = WRITE_FUA;
  +   else if (!(q-flush_flags  REQ_FLUSH))
  +   rw = WRITE_FUA;
  +   } else {
  rw = WRITE;
  +   }
  } else {
  rw = READ;
  }
  @@ -774,6 +779,15 @@ iblock_parse_cdb(struct se_cmd *cmd)
  return sbc_parse_cdb(cmd, iblock_sbc_ops);
   }
   
  +bool iblock_get_write_cache(struct se_device *dev)
  +{
  +   struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
  +   struct block_device *bd = ib_dev-ibd_bd;
  +   struct request_queue *q = bdev_get_queue(bd);
  +
  +   return (q-flush_flags  REQ_FLUSH);
 
 no need of the ().
 

Dropping unnecessary () for bit-wise comparison.

Thanks Asias!

--nab

  +}
  +
   static struct se_subsystem_api iblock_template = {
  .name   = iblock,
  .inquiry_prod   = IBLOCK,
  @@ -790,6 +804,7 @@ static struct se_subsystem_api iblock_template = {
  .show_configfs_dev_params = iblock_show_configfs_dev_params,
  .get_device_type= sbc_get_device_type,
  .get_blocks = iblock_get_blocks,
  +   

[PATCH 4/6] qla2xxx: Allow ISP81xx to create ATIO queues.

2013-01-30 Thread Saurav Kashyap
From: Arun Easi arun.e...@qlogic.com

Signed-off-by: Arun Easi arun.e...@qlogic.com
Signed-off-by: Saurav Kashyap saurav.kash...@qlogic.com
---
 drivers/scsi/qla2xxx/qla_os.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index e93408f..2d2afdb 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -2425,6 +2425,7 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct 
pci_device_id *id)
ha-mbx_count = MAILBOX_REGISTER_COUNT;
req_length = REQUEST_ENTRY_CNT_24XX;
rsp_length = RESPONSE_ENTRY_CNT_2300;
+   ha-tgt.atio_q_length = ATIO_ENTRY_CNT_24XX;
ha-max_loop_id = SNS_LAST_LOOP_ID_2300;
ha-init_cb_size = sizeof(struct mid_init_cb_81xx);
ha-gid_list_info_size = 8;
-- 
1.7.7

--
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 0/6] qla2xxx: Patches for scsi misc branch.

2013-01-30 Thread Saurav Kashyap
Hi James,

Please apply the following patches to the scsi tree at your earliest
convenience for inclusion in the next mainline merge window.

Thanks,
~Saurav

Arun Easi (3):
  qla2xxx: Enable target mode support for ISP83xx.
  qla2xxx: Allow ISP81xx to create ATIO queues.
  qla2xxx: Prevent enabling target mode for unsupported HBAs.

Chad Dupuis (2):
  qla2xxx: Determine the number of outstanding commands based on
available resources.
  qla2xxx: Ramp down queue depth for attached SCSI devices when driver
resources are low.

Saurav Kashyap (1):
  qla2xxx: Correction to the message ids.

 drivers/scsi/qla2xxx/qla_bsg.c|2 +-
 drivers/scsi/qla2xxx/qla_dbg.c|9 +-
 drivers/scsi/qla2xxx/qla_def.h|   30 +--
 drivers/scsi/qla2xxx/qla_fw.h |3 +-
 drivers/scsi/qla2xxx/qla_gbl.h|4 +
 drivers/scsi/qla2xxx/qla_init.c   |   60 +-
 drivers/scsi/qla2xxx/qla_inline.h |   19 
 drivers/scsi/qla2xxx/qla_iocb.c   |   37 
 drivers/scsi/qla2xxx/qla_isr.c|   30 +--
 drivers/scsi/qla2xxx/qla_mbx.c|8 +-
 drivers/scsi/qla2xxx/qla_mid.c|7 ++-
 drivers/scsi/qla2xxx/qla_nx.c |2 +-
 drivers/scsi/qla2xxx/qla_os.c |  123 +--
 drivers/scsi/qla2xxx/qla_target.c |  169 +
 drivers/scsi/qla2xxx/qla_target.h |   19 +++--
 15 files changed, 442 insertions(+), 80 deletions(-)

-- 
1.7.7

--
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 3/6] qla2xxx: Enable target mode support for ISP83xx.

2013-01-30 Thread Saurav Kashyap
From: Arun Easi arun.e...@qlogic.com

Signed-off-by: Arun Easi arun.e...@qlogic.com
Signed-off-by: Saurav Kashyap saurav.kash...@qlogic.com
---
 drivers/scsi/qla2xxx/qla_dbg.c|2 +-
 drivers/scsi/qla2xxx/qla_def.h|8 ++
 drivers/scsi/qla2xxx/qla_fw.h |3 +-
 drivers/scsi/qla2xxx/qla_init.c   |6 +-
 drivers/scsi/qla2xxx/qla_isr.c|   16 +++-
 drivers/scsi/qla2xxx/qla_os.c |4 +
 drivers/scsi/qla2xxx/qla_target.c |  159 +
 drivers/scsi/qla2xxx/qla_target.h |   14 +++-
 8 files changed, 187 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index f7cdd25..e690d05 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -39,7 +39,7 @@
  * | MultiQ   |   0xc00c   |   |
  * | Misc |   0xd010   |   |
  * | Target Mode |   0xe06f   ||
- * | Target Mode Management  |   0xf071   ||
+ * | Target Mode Management  |   0xf072   ||
  * | Target Mode Task Management  |  0x1000b  ||
  * --
  */
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 06c6271..b5fc478 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -537,6 +537,8 @@ struct device_reg_25xxmq {
uint32_t req_q_out;
uint32_t rsp_q_in;
uint32_t rsp_q_out;
+   uint32_t atio_q_in;
+   uint32_t atio_q_out;
 };
 
 typedef union {
@@ -563,6 +565,9 @@ typedef union {
 (reg)-u.isp2100.mailbox5 : \
 (reg)-u.isp2300.rsp_q_out)
 
+#define ISP_ATIO_Q_IN(vha) (vha-hw-tgt.atio_q_in)
+#define ISP_ATIO_Q_OUT(vha) (vha-hw-tgt.atio_q_out)
+
 #define MAILBOX_REG(ha, reg, num) \
(IS_QLA2100(ha) || IS_QLA2200(ha) ? \
 (num  8 ? \
@@ -2559,6 +2564,8 @@ struct qlt_hw_data {
struct atio *atio_ring_ptr; /* Current address. */
uint16_t atio_ring_index; /* Current index. */
uint16_t atio_q_length;
+   uint32_t __iomem *atio_q_in;
+   uint32_t __iomem *atio_q_out;
 
void *target_lport_ptr;
struct qla_tgt_func_tmpl *tgt_ops;
@@ -2790,6 +2797,7 @@ struct qla_hw_data {
 #define IS_PI_SPLIT_DET_CAPABLE_HBA(ha)(IS_QLA83XX(ha))
 #define IS_PI_SPLIT_DET_CAPABLE(ha)(IS_PI_SPLIT_DET_CAPABLE_HBA(ha)  \
 (((ha)-fw_attributes_h  16 | (ha)-fw_attributes)  BIT_22))
+#define IS_ATIO_MSIX_CAPABLE(ha) (IS_QLA83XX(ha))
 
/* HBA serial number */
uint8_t serial0;
diff --git a/drivers/scsi/qla2xxx/qla_fw.h b/drivers/scsi/qla2xxx/qla_fw.h
index be6d61a..7105d5e 100644
--- a/drivers/scsi/qla2xxx/qla_fw.h
+++ b/drivers/scsi/qla2xxx/qla_fw.h
@@ -300,7 +300,8 @@ struct init_cb_24xx {
uint32_t prio_request_q_address[2];
 
uint16_t msix;
-   uint8_t reserved_2[6];
+   uint16_t msix_atio;
+   uint8_t reserved_2[4];
 
uint16_t atio_q_inpointer;
uint16_t atio_q_length;
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 81e8cca..a581c85 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -1964,7 +1964,7 @@ qla24xx_config_rings(struct scsi_qla_host *vha)
WRT_REG_DWORD(reg-isp24.rsp_q_in, 0);
WRT_REG_DWORD(reg-isp24.rsp_q_out, 0);
}
-   qlt_24xx_config_rings(vha, reg);
+   qlt_24xx_config_rings(vha);
 
/* PCI posting */
RD_REG_DWORD(ioreg-hccr);
@@ -5579,6 +5579,8 @@ qla81xx_nvram_config(scsi_qla_host_t *vha)
if (IS_T10_PI_CAPABLE(ha))
nv-frame_payload_size = ~7;
 
+   qlt_81xx_config_nvram_stage1(vha, nv);
+
/* Reset Initialization control block */
memset(icb, 0, ha-init_cb_size);
 
@@ -5619,6 +5621,8 @@ qla81xx_nvram_config(scsi_qla_host_t *vha)
qla2x00_set_model_info(vha, nv-model_name, sizeof(nv-model_name),
QLE8XXX);
 
+   qlt_81xx_config_nvram_stage2(vha, icb);
+
/* Use alternate WWN? */
if (nv-host_p  __constant_cpu_to_le32(BIT_15)) {
memcpy(icb-node_name, nv-alternate_node_name, WWN_SIZE);
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 1b192c8..26a3086 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -13,6 +13,8 @@
 #include scsi/scsi_bsg_fc.h
 #include scsi/scsi_eh.h
 
+#include qla_target.h
+
 static void qla2x00_mbx_completion(scsi_qla_host_t *, uint16_t);
 static void qla2x00_process_completed_request(struct scsi_qla_host *,
struct req_que *, uint32_t);
@@ -2751,6 +2753,12 @@ static struct qla_init_msix_entry 
qla82xx_msix_entries[2] = {
{ qla2xxx (rsp_q), qla82xx_msix_rsp_q },
 };
 
+static struct qla_init_msix_entry 

[PATCH 2/6] qla2xxx: Ramp down queue depth for attached SCSI devices when driver resources are low.

2013-01-30 Thread Saurav Kashyap
From: Chad Dupuis chad.dup...@qlogic.com

Signed-off-by: Chad Dupuis chad.dup...@qlogic.com
Signed-off-by: Saurav Kashyap saurav.kash...@qlogic.com
---
 drivers/scsi/qla2xxx/qla_dbg.c|2 +-
 drivers/scsi/qla2xxx/qla_def.h|9 +++
 drivers/scsi/qla2xxx/qla_gbl.h|1 +
 drivers/scsi/qla2xxx/qla_inline.h |   19 +++
 drivers/scsi/qla2xxx/qla_isr.c|4 ++
 drivers/scsi/qla2xxx/qla_os.c |  107 ++--
 6 files changed, 135 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index 2f3e765..f7cdd25 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -17,7 +17,7 @@
  * |  || 0x113a |
  * | Device Discovery |   0x2087   | 0x2020-0x2022, |
  * |  || 0x2016 |
- * | Queue Command and IO tracing |   0x3030   | 0x3006-0x300b  |
+ * | Queue Command and IO tracing |   0x3031   | 0x3006-0x300b  |
  * |  || 0x3027-0x3028  |
  * |  || 0x302d-0x302e  |
  * | DPC Thread   |   0x401d   | 0x4002,0x4013  |
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index a84bb8d..06c6271 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -2536,6 +2536,7 @@ struct req_que {
srb_t **outstanding_cmds;
uint32_t current_outstanding_cmd;
uint16_t num_outstanding_cmds;
+#defineMAX_Q_DEPTH 32
int max_q_depth;
 };
 
@@ -3058,6 +3059,12 @@ struct qla_hw_data {
struct work_struct idc_state_handler;
struct work_struct nic_core_unrecoverable;
 
+#define HOST_QUEUE_RAMPDOWN_INTERVAL   (60 * HZ)
+#define HOST_QUEUE_RAMPUP_INTERVAL (30 * HZ)
+   unsigned long   host_last_rampdown_time;
+   unsigned long   host_last_rampup_time;
+   int cfg_lun_q_depth;
+
struct qlt_hw_data tgt;
 };
 
@@ -3117,6 +3124,8 @@ typedef struct scsi_qla_host {
 #define MPI_RESET_NEEDED   19  /* Initiate MPI FW reset */
 #define ISP_QUIESCE_NEEDED 20  /* Driver need some quiescence */
 #define SCR_PENDING21  /* SCR in target mode */
+#define HOST_RAMP_DOWN_QUEUE_DEPTH 22
+#define HOST_RAMP_UP_QUEUE_DEPTH   23
 
uint32_tdevice_flags;
 #define SWITCH_FOUND   BIT_0
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index fba0651..1732713 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -97,6 +97,7 @@ extern int qlport_down_retry;
 extern int ql2xplogiabsentdevice;
 extern int ql2xloginretrycount;
 extern int ql2xfdmienable;
+extern int ql2xmaxqdepth;
 extern int ql2xallocfwdump;
 extern int ql2xextended_error_logging;
 extern int ql2xiidmaenable;
diff --git a/drivers/scsi/qla2xxx/qla_inline.h 
b/drivers/scsi/qla2xxx/qla_inline.h
index c0462c0..deb8618 100644
--- a/drivers/scsi/qla2xxx/qla_inline.h
+++ b/drivers/scsi/qla2xxx/qla_inline.h
@@ -213,3 +213,22 @@ qla2x00_gid_list_size(struct qla_hw_data *ha)
 {
return sizeof(struct gid_list_info) * ha-max_fibre_devices;
 }
+
+static inline void
+qla2x00_do_host_ramp_up(scsi_qla_host_t *vha)
+{
+   if (vha-hw-cfg_lun_q_depth = ql2xmaxqdepth)
+   return;
+
+   /* Wait at least HOST_QUEUE_RAMPDOWN_INTERVAL before ramping up */
+   if (time_before(jiffies, (vha-hw-host_last_rampdown_time +
+   HOST_QUEUE_RAMPDOWN_INTERVAL)))
+   return;
+
+   /* Wait at least HOST_QUEUE_RAMPUP_INTERVAL between each ramp up */
+   if (time_before(jiffies, (vha-hw-host_last_rampup_time +
+   HOST_QUEUE_RAMPUP_INTERVAL)))
+   return;
+
+   set_bit(HOST_RAMP_UP_QUEUE_DEPTH, vha-dpc_flags);
+}
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 4513073..1b192c8 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1934,6 +1934,7 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct rsp_que 
*rsp, void *pkt)
 
/* Fast path completion. */
if (comp_status == CS_COMPLETE  scsi_status == 0) {
+   qla2x00_do_host_ramp_up(vha);
qla2x00_process_completed_request(vha, req, handle);
 
return;
@@ -2193,6 +2194,9 @@ out:
cp-cmnd[8], cp-cmnd[9], scsi_bufflen(cp), rsp_info_len,
resid_len, fw_resid_len);
 
+   if (!res)
+   qla2x00_do_host_ramp_up(vha);
+
if (rsp-status_srb == NULL)
sp-done(ha, sp, res);
 }
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 53efffc..da86d38 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -111,8 

[PATCH 5/6] qla2xxx: Prevent enabling target mode for unsupported HBAs.

2013-01-30 Thread Saurav Kashyap
From: Arun Easi arun.e...@qlogic.com

Signed-off-by: Arun Easi arun.e...@qlogic.com
Signed-off-by: Saurav Kashyap saurav.kash...@qlogic.com
---
 drivers/scsi/qla2xxx/qla_dbg.c|2 +-
 drivers/scsi/qla2xxx/qla_def.h|1 +
 drivers/scsi/qla2xxx/qla_target.c |6 ++
 3 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index e690d05..f81e938 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -38,7 +38,7 @@
  * | ISP82XX Specific |   0xb084   | 0xb002,0xb024  |
  * | MultiQ   |   0xc00c   |   |
  * | Misc |   0xd010   |   |
- * | Target Mode |   0xe06f   ||
+ * | Target Mode |   0xe070   ||
  * | Target Mode Management  |   0xf072   ||
  * | Target Mode Task Management  |  0x1000b  ||
  * --
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index b5fc478..5c42c91 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -2798,6 +2798,7 @@ struct qla_hw_data {
 #define IS_PI_SPLIT_DET_CAPABLE(ha)(IS_PI_SPLIT_DET_CAPABLE_HBA(ha)  \
 (((ha)-fw_attributes_h  16 | (ha)-fw_attributes)  BIT_22))
 #define IS_ATIO_MSIX_CAPABLE(ha) (IS_QLA83XX(ha))
+#define IS_TGT_MODE_CAPABLE(ha)(ha-tgt.atio_q_length)
 
/* HBA serial number */
uint8_t serial0;
diff --git a/drivers/scsi/qla2xxx/qla_target.c 
b/drivers/scsi/qla2xxx/qla_target.c
index cb8ea44..61b5d8c 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -4306,6 +4306,12 @@ int qlt_add_target(struct qla_hw_data *ha, struct 
scsi_qla_host *base_vha)
if (!QLA_TGT_MODE_ENABLED())
return 0;
 
+   if (!IS_TGT_MODE_CAPABLE(ha)) {
+   ql_log(ql_log_warn, base_vha, 0xe070,
+   This adapter does not support target mode.\n);
+   return 0;
+   }
+
ql_dbg(ql_dbg_tgt, base_vha, 0xe03b,
Registering target for host %ld(%p), base_vha-host_no, ha);
 
-- 
1.7.7

--
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 6/6] qla2xxx: Correction to the message ids.

2013-01-30 Thread Saurav Kashyap
Signed-off-by: Giridhar Malavali giridhar.malav...@qlogic.com
Signed-off-by: Saurav Kashyap saurav.kash...@qlogic.com
---
 drivers/scsi/qla2xxx/qla_dbg.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index f81e938..ba2d7a8 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -25,6 +25,7 @@
  * |  || 0x5047,0x5052  |
  * | Timer Routines   |   0x6011   ||
  * | User Space Interactions  |   0x70c3   | 0x7018,0x702e, |
+ * |  || 0x7020,0x7024, |
  * |  || 0x7039,0x7045, |
  * |  || 0x7073-0x7075, |
  * |  || 0x708c,|
-- 
1.7.7

--
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 0/3] target: Fix zero-length regressions in v3.8-rc1 code

2013-01-30 Thread Paolo Bonzini
Il 29/01/2013 23:26, Nicholas A. Bellinger ha scritto:
 From: Nicholas Bellinger n...@linux-iscsi.org
 
 Hi folks,
 
 The following are a handful of zero-length CDB regression bugfixes to address
 breakage introduced by the recent sense_reason_t conversion in v3.8-rc1 code,
 which incorrectly assumed CHECK_CONDITION status (in all CDB emulation cases)
 when NULL was returned by transport_kmap_data_sg().
 
 Please review, as I'd like to get these into v3.8-rc6.
 
 Thank you,
 
 --nab
 
 Nicholas Bellinger (3):
   target: Fix zero-length INQUIRY additional sense code regression
   target: Fix zero-length MODE_SENSE regression
   target: Fix zero-length READ_CAPACITY_16 regression
 
  drivers/target/target_core_sbc.c |   18 +++
  drivers/target/target_core_spc.c |   44 +
  2 files changed, 19 insertions(+), 43 deletions(-)
 

Looks good, but given the mess I made in my own zero-length patches,
don't really count it as a Reviewed-by.

Paolo
--
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 v8 0/4] block layer runtime pm

2013-01-30 Thread Aaron Lu
In August 2010, Jens and Alan discussed about Runtime PM and the block
layer. http://marc.info/?t=12825910841r=1w=2
And then Alan has given a detailed implementation guide:
http://marc.info/?l=linux-scsim=133727953625963w=2

To test:
# ls -l /sys/block/sda
/sys/devices/pci:00/:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda

# echo 1  
/sys/devices/pci:00/:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/power/autosuspend_delay_ms
# echo auto  
/sys/devices/pci:00/:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/power/control
Then you'll see sda is suspended after 10secs idle.

[ 1767.680192] sd 2:0:0:0: [sda] Synchronizing SCSI cache
[ 1767.680317] sd 2:0:0:0: [sda] Stopping disk

And if you do some IO, it will resume immediately.
[ 1791.052438] sd 2:0:0:0: [sda] Starting disk

For test, I often set the autosuspend time to 1 second. If you are using
a GUI, the 10 seconds delay may be too long that the disk can not enter
runtime suspended state.

Note that sd's runtime suspend callback will dump some kernel messages
and the syslog daemon will write kernel message to /var/log/messages,
making the disk instantly resume after suspended. So for test, the
syslog daemon should better be temporarily stopped.

A git repo for it, on top of libata/upstream, scsi/for-next and
block/for-next:
https://github.com/aaronlu/linux.git blockpm

v8:
- Set default autosuspend delay to -1 to avoid suspend till an updated
  value is set as suggested by Alan Stern;
- Always check the dev field of the queue structure, as it is incorrect
  and meaningless to do any operation on devices that do not use block
  layer runtime PM as reminded by Alan Stern;
- Update scsi bus level runtime PM callback to take care of scsi devices
  that use block layer runtime PM and that don't.

v7:
- Add kernel doc for block layer runtime PM API as suggested by
  Alan Stern;

- Add back check for q-dev, as that serves as a flag if driver
  is using block layer runtime PM;

- Do not auto suspend when last request is finished, as that's a hot
  path and auto suspend is not a trivial function. Instead, mark last
  busy in pre_suspend so that runtim PM core will retry suspend some
  time later to solve the 1st problem demostrated in v6, suggested by
  Alan Stern.

- Move block layer runtime PM strtegy functions to where they are
  needed instead of in include/linux/blkdev.h as suggested by Alan
  Stern since clients of block layer do not need to know those
  functions.

v6:
Take over from Lin Ming.

- Instead of put the device into autosuspend state in
  blk_post_runtime_suspend, do it when the last request is finished.
  This can also solve the problem illustrated below:

  thread Athread B
|suspend timer expired  |
|  ... ...  |a new request comes in,
|  ... ...  |blk_pm_add_request
|  ... ...  |skip request_resume due to
|  ... ...  |q-status is still RPM_ACTIVE
|  rpm_suspend  |  ... ...
|scsi_runtime_suspend   |  ... ...
|  blk_pre_runtime_suspend  |  ... ...
|  return -EBUSY due to nr_pending  |  ... ...
|  rpm_suspend done |  ... ...
|   |blk_pm_put_request, mark last busy

But no more trigger point, and the device will stay at RPM_ACTIVE state.
Run pm_runtime_autosuspend after the last request is finished solved
this problem.

- Requests which have the REQ_PM flag should not involve nr_pending
  counting, or we may lose the condition to resume the device:
  Suppose queue is active and nr_pending is 0. Then a REQ_PM request
  comes and nr_pending will be increased to 1, but since the request has
  REQ_PM flag, it will not cause resume. Before it is finished, a normal
  request comes in, and since nr_pending is 1 now, it will not trigger
  the resume of the device either. Bug.

- Do not quiesce the device in scsi bus level runtime suspend callback.
  Since the only reason the device is to be runtime suspended is due to
  no more requests pending for it, quiesce it is pointless.

- Remove scsi_autopm_* from sd_check_events as we are request driven.

- Call blk_pm_runtime_init in scsi_sysfs_initialize_dev, so that we do
  not need to check queue's device in blk_pm_add/put_request.

- Do not mark last busy and initiate an autosuspend for the device in
  blk_pm_runtime_init function.

- Do not mark last busy and initiate an autosuspend for the device in
  block_post_runtime_resume, as when the request that triggered the
  resume finished, the blk_pm_put_request will mark last busy and
  initiate an autosuspend.

v5:
- rename scsi_execute_req to scsi_execute_req_flags
  and wrap scsi_execute_req around it.
- add detail function descriptions in patch 2 log
- define static helper functions to do runtime pm work on block layer
  and put the definitions inside a #ifdef 

[PATCH v8 4/4] sd: change to auto suspend mode

2013-01-30 Thread Aaron Lu
From: Lin Ming ming.m@intel.com

Uses block layer runtime pm helper functions in
scsi_runtime_suspend/resume for devices that take advantage of it.

Remove scsi_autopm_* from sd open/release path and check_events path.

Signed-off-by: Lin Ming ming.m@intel.com
Signed-off-by: Aaron Lu aaron...@intel.com
---
 drivers/scsi/scsi_pm.c | 79 ++
 drivers/scsi/sd.c  | 13 +
 2 files changed, 68 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 8f6b12c..6ce00c5 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -144,18 +144,44 @@ static int scsi_bus_restore(struct device *dev)
 
 #ifdef CONFIG_PM_RUNTIME
 
+static int scsi_blk_runtime_suspend(struct device *dev)
+{
+   struct scsi_device *sdev = to_scsi_device(dev);
+   int err;
+
+   err = blk_pre_runtime_suspend(sdev-request_queue);
+   if (err)
+   return err;
+   err = pm_generic_runtime_suspend(dev);
+   blk_post_runtime_suspend(sdev-request_queue, err);
+
+   return err;
+}
+
+static int scsi_dev_runtime_suspend(struct device *dev)
+{
+   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
+   int err;
+
+   err = scsi_dev_type_suspend(dev, pm ? pm-runtime_suspend : NULL);
+   if (err == -EAGAIN)
+   pm_schedule_suspend(dev, jiffies_to_msecs(
+   round_jiffies_up_relative(HZ/10)));
+
+   return err;
+}
+
 static int scsi_runtime_suspend(struct device *dev)
 {
int err = 0;
-   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
 
dev_dbg(dev, scsi_runtime_suspend\n);
if (scsi_is_sdev_device(dev)) {
-   err = scsi_dev_type_suspend(dev,
-   pm ? pm-runtime_suspend : NULL);
-   if (err == -EAGAIN)
-   pm_schedule_suspend(dev, jiffies_to_msecs(
-   round_jiffies_up_relative(HZ/10)));
+   struct scsi_device *sdev = to_scsi_device(dev);
+   if (sdev-request_queue-dev)
+   err = scsi_blk_runtime_suspend(dev);
+   else
+   err = scsi_dev_runtime_suspend(dev);
}
 
/* Insert hooks here for targets, hosts, and transport classes */
@@ -163,14 +189,36 @@ static int scsi_runtime_suspend(struct device *dev)
return err;
 }
 
+static int scsi_blk_runtime_resume(struct device *dev)
+{
+   struct scsi_device *sdev = to_scsi_device(dev);
+   int err;
+
+   blk_pre_runtime_resume(sdev-request_queue);
+   err = pm_generic_runtime_resume(dev);
+   blk_post_runtime_resume(sdev-request_queue, err);
+
+   return err;
+}
+
+static int scsi_dev_runtime_resume(struct device *dev)
+{
+   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
+   return scsi_dev_type_resume(dev, pm ? pm-runtime_resume : NULL);
+}
+
 static int scsi_runtime_resume(struct device *dev)
 {
int err = 0;
-   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
 
dev_dbg(dev, scsi_runtime_resume\n);
-   if (scsi_is_sdev_device(dev))
-   err = scsi_dev_type_resume(dev, pm ? pm-runtime_resume : NULL);
+   if (scsi_is_sdev_device(dev)) {
+   struct scsi_device *sdev = to_scsi_device(dev);
+   if (sdev-request_queue-dev)
+   err = scsi_blk_runtime_resume(dev);
+   else
+   err = scsi_dev_runtime_resume(dev);
+   }
 
/* Insert hooks here for targets, hosts, and transport classes */
 
@@ -185,10 +233,17 @@ static int scsi_runtime_idle(struct device *dev)
 
/* Insert hooks here for targets, hosts, and transport classes */
 
-   if (scsi_is_sdev_device(dev))
-   err = pm_schedule_suspend(dev, 100);
-   else
+   if (scsi_is_sdev_device(dev)) {
+   struct scsi_device *sdev = to_scsi_device(dev);
+   if (sdev-request_queue-dev) {
+   pm_runtime_mark_last_busy(dev);
+   err = pm_runtime_autosuspend(dev);
+   } else {
+   err = pm_schedule_suspend(dev, 100);
+   }
+   } else {
err = pm_runtime_suspend(dev);
+   }
return err;
 }
 
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 698923f..bf04dbf 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1121,10 +1121,6 @@ static int sd_open(struct block_device *bdev, fmode_t 
mode)
 
sdev = sdkp-device;
 
-   retval = scsi_autopm_get_device(sdev);
-   if (retval)
-   goto error_autopm;
-
/*
 * If the device is in error recovery, wait until it is done.
 * If the device is offline, then disallow any access to it.
@@ -1169,8 +1165,6 @@ static int sd_open(struct block_device *bdev, fmode_t 

[PATCH v8 3/4] block: implement runtime pm strategy

2013-01-30 Thread Aaron Lu
From: Lin Ming ming.m@intel.com

When a request is added:
If device is suspended or is suspending and the request is not a
PM request, resume the device.

When the last request finishes:
Call pm_runtime_mark_last_busy().

When pick a request:
If device is resuming/suspending, then only PM request is allowed
to go.

Signed-off-by: Lin Ming ming.m@intel.com
Signed-off-by: Aaron Lu aaron...@intel.com
---
 block/blk-core.c | 39 +++
 block/elevator.c | 26 ++
 2 files changed, 65 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index 1ec09f6..82cf678 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1264,6 +1264,16 @@ void part_round_stats(int cpu, struct hd_struct *part)
 }
 EXPORT_SYMBOL_GPL(part_round_stats);
 
+#ifdef CONFIG_PM_RUNTIME
+static void blk_pm_put_request(struct request *rq)
+{
+   if (rq-q-dev  !(rq-cmd_flags  REQ_PM)  !--rq-q-nr_pending)
+   pm_runtime_mark_last_busy(rq-q-dev);
+}
+#else
+static inline void blk_pm_put_request(struct request *rq) {}
+#endif
+
 /*
  * queue lock must be held
  */
@@ -1274,6 +1284,8 @@ void __blk_put_request(struct request_queue *q, struct 
request *req)
if (unlikely(--req-ref_count))
return;
 
+   blk_pm_put_request(req);
+
elv_completed_request(q, req);
 
/* this is a bio leak */
@@ -2051,6 +2063,28 @@ static void blk_account_io_done(struct request *req)
}
 }
 
+#ifdef CONFIG_PM_RUNTIME
+/*
+ * Don't process normal requests when queue is suspended
+ * or in the process of suspending/resuming
+ */
+static struct request *blk_pm_peek_request(struct request_queue *q,
+  struct request *rq)
+{
+   if (q-dev  (q-rpm_status == RPM_SUSPENDED ||
+   (q-rpm_status != RPM_ACTIVE  !(rq-cmd_flags  REQ_PM
+   return NULL;
+   else
+   return rq;
+}
+#else
+static inline struct request *blk_pm_peek_request(struct request_queue *q,
+ struct request *rq)
+{
+   return rq;
+}
+#endif
+
 /**
  * blk_peek_request - peek at the top of a request queue
  * @q: request queue to peek at
@@ -2073,6 +2107,11 @@ struct request *blk_peek_request(struct request_queue *q)
int ret;
 
while ((rq = __elv_next_request(q)) != NULL) {
+
+   rq = blk_pm_peek_request(q, rq);
+   if (!rq)
+   break;
+
if (!(rq-cmd_flags  REQ_STARTED)) {
/*
 * This is the first time the device driver
diff --git a/block/elevator.c b/block/elevator.c
index 11683bb..29c5c7e 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -34,6 +34,7 @@
 #include linux/blktrace_api.h
 #include linux/hash.h
 #include linux/uaccess.h
+#include linux/pm_runtime.h
 
 #include trace/events/block.h
 
@@ -515,6 +516,27 @@ void elv_bio_merged(struct request_queue *q, struct 
request *rq,
e-type-ops.elevator_bio_merged_fn(q, rq, bio);
 }
 
+#ifdef CONFIG_PM_RUNTIME
+static void blk_pm_requeue_request(struct request *rq)
+{
+   if (rq-q-dev  !(rq-cmd_flags  REQ_PM))
+   rq-q-nr_pending--;
+}
+
+static void blk_pm_add_request(struct request_queue *q, struct request *rq)
+{
+   if (q-dev  !(rq-cmd_flags  REQ_PM)  q-nr_pending++ == 0 
+   (q-rpm_status == RPM_SUSPENDED || q-rpm_status == RPM_SUSPENDING))
+   pm_request_resume(q-dev);
+}
+#else
+static inline void blk_pm_requeue_request(struct request *rq) {}
+static inline void blk_pm_add_request(struct request_queue *q,
+ struct request *rq)
+{
+}
+#endif
+
 void elv_requeue_request(struct request_queue *q, struct request *rq)
 {
/*
@@ -529,6 +551,8 @@ void elv_requeue_request(struct request_queue *q, struct 
request *rq)
 
rq-cmd_flags = ~REQ_STARTED;
 
+   blk_pm_requeue_request(rq);
+
__elv_add_request(q, rq, ELEVATOR_INSERT_REQUEUE);
 }
 
@@ -551,6 +575,8 @@ void __elv_add_request(struct request_queue *q, struct 
request *rq, int where)
 {
trace_block_rq_insert(q, rq);
 
+   blk_pm_add_request(q, rq);
+
rq-q = q;
 
if (rq-cmd_flags  REQ_SOFTBARRIER) {
-- 
1.8.1

--
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 v8 2/4] block: add runtime pm helpers

2013-01-30 Thread Aaron Lu
From: Lin Ming ming.m@intel.com

Add runtime pm helper functions:

void blk_pm_runtime_init(struct request_queue *q, struct device *dev)
  - Initialization function for drivers to call.

int blk_pre_runtime_suspend(struct request_queue *q)
  - If any requests are in the queue, mark last busy and return -EBUSY.
Otherwise set q-rpm_status to RPM_SUSPENDING and return 0.

void blk_post_runtime_suspend(struct request_queue *q, int err)
  - If the suspend succeeded then set q-rpm_status to RPM_SUSPENDED.
Otherwise set it to RPM_ACTIVE.

void blk_pre_runtime_resume(struct request_queue *q)
  - Set q-rpm_status to RPM_RESUMING.

void blk_post_runtime_resume(struct request_queue *q, int err)
  - If the resume succeeded then set q-rpm_status to RPM_ACTIVE
and call __blk_run_queue, then mark last busy and autosuspend.
Otherwise set q-rpm_status to RPM_SUSPENDED.

Signed-off-by: Lin Ming ming.m@intel.com
Signed-off-by: Aaron Lu aaron...@intel.com
---
 block/blk-core.c   | 143 +
 include/linux/blkdev.h |  27 ++
 2 files changed, 170 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index 66d3168..1ec09f6 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -30,6 +30,7 @@
 #include linux/list_sort.h
 #include linux/delay.h
 #include linux/ratelimit.h
+#include linux/pm_runtime.h
 
 #define CREATE_TRACE_POINTS
 #include trace/events/block.h
@@ -3043,6 +3044,148 @@ void blk_finish_plug(struct blk_plug *plug)
 }
 EXPORT_SYMBOL(blk_finish_plug);
 
+#ifdef CONFIG_PM_RUNTIME
+/**
+ * blk_pm_runtime_init - Block layer runtime PM initialization routine
+ * @q: the queue of the device
+ * @dev: the device the queue belongs to
+ *
+ * Description:
+ *Initialize runtime-PM-related fields for @q and start auto suspend for
+ *@dev. Drivers that want to take advantage of request-based runtime PM
+ *should call this function after @dev has been initialized, and its
+ *request queue @q has been allocated, and runtime PM for it can not happen
+ *yet(either due to disabled/forbidden or its usage_count  0). In most
+ *cases, driver should call this function before any I/O has taken place.
+ *
+ *This function takes care of setting up using auto suspend for the device,
+ *the autosuspend delay is set to -1 to make runtime suspend impossible
+ *until an updated value is either set by user or by driver. Drivers do
+ *not need to touch other autosuspend settings.
+ *
+ *The block layer runtime PM is request based, so only works for drivers
+ *that use request as their IO unit instead of those directly use bio's.
+ */
+void blk_pm_runtime_init(struct request_queue *q, struct device *dev)
+{
+   q-dev = dev;
+   q-rpm_status = RPM_ACTIVE;
+   pm_runtime_set_autosuspend_delay(q-dev, -1);
+   pm_runtime_use_autosuspend(q-dev);
+   pm_runtime_mark_last_busy(q-dev);
+   pm_runtime_autosuspend(q-dev);
+}
+EXPORT_SYMBOL(blk_pm_runtime_init);
+
+/**
+ * blk_pre_runtime_suspend - Pre runtime suspend check
+ * @q: the queue of the device
+ *
+ * Description:
+ *This function will check if runtime suspend is allowed for the device
+ *by examining if there are any requests pending in the queue. If there
+ *are requests pending, the device can not be runtime suspended; otherwise,
+ *the queue's status will be updated to SUSPENDING and the driver can
+ *proceed to suspend the device.
+ *
+ *For the not allowed case, we mark last busy for the device so that
+ *runtime PM core will try to autosuspend it some time later.
+ *
+ *This function should be called near the start of the device's
+ *runtime_suspend callback.
+ *
+ * Return:
+ *0- OK to runtime suspend the device
+ *-EBUSY   - Device should not be runtime suspended
+ */
+int blk_pre_runtime_suspend(struct request_queue *q)
+{
+   int ret = 0;
+
+   spin_lock_irq(q-queue_lock);
+   if (q-nr_pending) {
+   ret = -EBUSY;
+   pm_runtime_mark_last_busy(q-dev);
+   } else {
+   q-rpm_status = RPM_SUSPENDING;
+   }
+   spin_unlock_irq(q-queue_lock);
+   return ret;
+}
+EXPORT_SYMBOL(blk_pre_runtime_suspend);
+
+/**
+ * blk_post_runtime_suspend - Post runtime suspend processing
+ * @q: the queue of the device
+ * @err: return value of the device's runtime_suspend function
+ *
+ * Description:
+ *Update the queue's runtime status according to the return value of the
+ *device's runtime suspend function.
+ *
+ *This function should be called near the end of the device's
+ *runtime_suspend callback.
+ */
+void blk_post_runtime_suspend(struct request_queue *q, int err)
+{
+   spin_lock_irq(q-queue_lock);
+   if (!err)
+   q-rpm_status = RPM_SUSPENDED;
+   else
+   q-rpm_status = RPM_ACTIVE;
+   spin_unlock_irq(q-queue_lock);
+}

Re: [patch] [SCSI] libosd: check for kzalloc() failure

2013-01-30 Thread Benny Halevy
On Wed, Jan 30, 2013 at 10:57 AM, walter harms wha...@bfs.de wrote:


 Am 30.01.2013 09:27, schrieb Dan Carpenter:
 On Wed, Jan 30, 2013 at 09:15:43AM +0100, walter harms wrote:


 Am 30.01.2013 08:06, schrieb Dan Carpenter:
 There wasn't any error handling for this kzalloc().

 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

 diff --git a/drivers/scsi/osd/osd_initiator.c 
 b/drivers/scsi/osd/osd_initiator.c
 index c06b8e5..d8293f2 100644
 --- a/drivers/scsi/osd/osd_initiator.c
 +++ b/drivers/scsi/osd/osd_initiator.c
 @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev 
 *od,
 odi-osdname_len = get_attrs[a].len;
 /* Avoid NULL for memcmp optimization 0-length is good enough */
 odi-osdname = kzalloc(odi-osdname_len + 1, GFP_KERNEL);
 +   if (!odi-osdname) {
 +   ret = -ENOMEM;
 +   goto out;
 +   }
 if (odi-osdname_len)
 memcpy(odi-osdname, get_attrs[a].val_ptr, odi-osdname_len);
 OSD_INFO(OSD_NAME   [%s]\n, odi-osdname);
 --

 this looks like strdup() ?


 Maybe?  It's a funny thing going on with the NUL terminator and I
 don't understand what the comment is about.

 It appears that normally get_attrs[a].val_ptr is a NUL terminated
 string but get_attrs[a].len does not count the terminator.

 Odd.

 i have no clue what the programmer was thinking. if i read this correct
 osdname is u8 *osdname; so a simple strdup() or strndup() would be ok
 the comment seems to indicate that get_attrs[a].val_ptr could be NULL
 but where is the check ?
 Perhaps they are not using ascii here ? then a memdup(get_attrs[a].len)
 would be better.

There are subtle differences between kstrdup or kmemdup and this implementation.

kmemdup is problematic as get_attrs[a].val_ptr does not contain the
NUL terminator

In the following case:
if get_attrs[a].len == 0
then get_attrs[a].val_ptr == NULL

The end result should be
odi-osdname_len = 0
odi-osdname = kzalloc(1, GFP_KERNEL);

while with kstrdup, odi-osdname will end up being NULL
as it's input arg s is NULL.

Benny


 re,
  wh


--
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: storvsc loops with No Sense messages

2013-01-30 Thread Olaf Hering
On Tue, Jan 29, Olaf Hering wrote:

 On Tue, Jan 29, KY Srinivasan wrote:
 
  Was the installation done on a vhd or VHDX. I checked with Hyper-V
  guys and they say they never supported WRITE_SAME. I am wondering how
  it worked on ws2008.
 
 Its a vhdx image, both fixed and dynamically size have the same issue.
 I will see what happens with ws2008, at least the install proceeds
 without issues. 

I see that WRITE_SAME is also used when running on ws2008, but
appearently it does not cause any issues.
This triggers only when the default ext4 is used, not when I force ext3
for the root filesystem.
So to me it looks like ws2012 has issues with WRITE_SAME handling.


Is there a way to not use WRITE_SAME at all? While browsing the code its
not clear if there is a conditional for this command.


Olaf
--
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] libosd: check for kzalloc() failure

2013-01-30 Thread Benny Halevy
On Wed, Jan 30, 2013 at 9:06 AM, Dan Carpenter dan.carpen...@oracle.com wrote:
 There wasn't any error handling for this kzalloc().

 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

 diff --git a/drivers/scsi/osd/osd_initiator.c 
 b/drivers/scsi/osd/osd_initiator.c
 index c06b8e5..d8293f2 100644
 --- a/drivers/scsi/osd/osd_initiator.c
 +++ b/drivers/scsi/osd/osd_initiator.c
 @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev *od,
 odi-osdname_len = get_attrs[a].len;
 /* Avoid NULL for memcmp optimization 0-length is good enough */
 odi-osdname = kzalloc(odi-osdname_len + 1, GFP_KERNEL);
 +   if (!odi-osdname) {
 +   ret = -ENOMEM;
 +   goto out;
 +   }

good catch!

Benny

 if (odi-osdname_len)
 memcpy(odi-osdname, get_attrs[a].val_ptr, odi-osdname_len);
 OSD_INFO(OSD_NAME   [%s]\n, odi-osdname);
--
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] qla4xxx: don't free NULL dma pool

2013-01-30 Thread Vikas Chaudhary

-Original Message-
From: Dan Carpenter dan.carpen...@oracle.com
Date: Wednesday 30 January 2013 12:37 PM
To: Ravi Anand ravi.an...@qlogic.com
Cc: Vikas vikas.chaudh...@qlogic.com, Dept-Eng iSCSI Driver
dept-iscsidri...@qlogic.com, James E.J. Bottomley
jbottom...@parallels.com, scsi linux-scsi@vger.kernel.org,
kernel-janit...@vger.kernel.org kernel-janit...@vger.kernel.org
Subject: [patch] [SCSI] qla4xxx: don't free NULL dma pool

The error path calls dma_pool_free() on this path but chap_table is
NULL and chap_dma is uninitialized.  It's cleaner to just return
directly.

Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

diff --git a/drivers/scsi/qla4xxx/ql4_mbx.c
b/drivers/scsi/qla4xxx/ql4_mbx.c
index 1c57c22..887e409 100644
--- a/drivers/scsi/qla4xxx/ql4_mbx.c
+++ b/drivers/scsi/qla4xxx/ql4_mbx.c
@@ -1404,10 +1404,8 @@ int qla4xxx_get_chap(struct scsi_qla_host *ha,
char *username, char *password,
   dma_addr_t chap_dma;

   chap_table = dma_pool_alloc(ha-chap_dma_pool, GFP_KERNEL, chap_dma);
-  if (chap_table == NULL) {
-  ret = -ENOMEM;
-  goto exit_get_chap;
-  }
+  if (chap_table == NULL)
+  return -ENOMEM;

   chap_size = sizeof(struct ql4_chap_table);
   memset(chap_table, 0, chap_size);

Thanks for a fix.

Acked-by: Vikas Chaudhary vikas.chaudh...@qlogic.com




This message and any attached documents contain information from QLogic 
Corporation or its wholly-owned subsidiaries that may be confidential. If you 
are not the intended recipient, you may not read, copy, distribute, or use this 
information. If you have received this transmission in error, please notify the 
sender immediately by reply e-mail and then delete this message.

--
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] libosd: check for kzalloc() failure

2013-01-30 Thread walter harms


Am 30.01.2013 10:51, schrieb Benny Halevy:
 On Wed, Jan 30, 2013 at 10:57 AM, walter harms wha...@bfs.de wrote:


 Am 30.01.2013 09:27, schrieb Dan Carpenter:
 On Wed, Jan 30, 2013 at 09:15:43AM +0100, walter harms wrote:


 Am 30.01.2013 08:06, schrieb Dan Carpenter:
 There wasn't any error handling for this kzalloc().

 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

 diff --git a/drivers/scsi/osd/osd_initiator.c 
 b/drivers/scsi/osd/osd_initiator.c
 index c06b8e5..d8293f2 100644
 --- a/drivers/scsi/osd/osd_initiator.c
 +++ b/drivers/scsi/osd/osd_initiator.c
 @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev 
 *od,
 odi-osdname_len = get_attrs[a].len;
 /* Avoid NULL for memcmp optimization 0-length is good enough */
 odi-osdname = kzalloc(odi-osdname_len + 1, GFP_KERNEL);
 +   if (!odi-osdname) {
 +   ret = -ENOMEM;
 +   goto out;
 +   }
 if (odi-osdname_len)
 memcpy(odi-osdname, get_attrs[a].val_ptr, odi-osdname_len);
 OSD_INFO(OSD_NAME   [%s]\n, odi-osdname);
 --

 this looks like strdup() ?


 Maybe?  It's a funny thing going on with the NUL terminator and I
 don't understand what the comment is about.

 It appears that normally get_attrs[a].val_ptr is a NUL terminated
 string but get_attrs[a].len does not count the terminator.

 Odd.

 i have no clue what the programmer was thinking. if i read this correct
 osdname is u8 *osdname; so a simple strdup() or strndup() would be ok
 the comment seems to indicate that get_attrs[a].val_ptr could be NULL
 but where is the check ?
 Perhaps they are not using ascii here ? then a memdup(get_attrs[a].len)
 would be better.
 
 There are subtle differences between kstrdup or kmemdup and this 
 implementation.
 
 kmemdup is problematic as get_attrs[a].val_ptr does not contain the
 NUL terminator

ok, i understand - but can we assume that we are talking ascii here ?

 In the following case:
 if get_attrs[a].len == 0
 then get_attrs[a].val_ptr == NULL
 
 The end result should be
 odi-osdname_len = 0
 odi-osdname = kzalloc(1, GFP_KERNEL);
 
 while with kstrdup, odi-osdname will end up being NULL
 as it's input arg s is NULL.
 

and you want the argument to be  ?
May i ask why ? kfree() can handle NULL and kprintf() (-family) also.

re,
 wh


 Benny
 

 re,
  wh


 
 
--
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: storvsc loops with No Sense messages

2013-01-30 Thread Olaf Hering
On Wed, Jan 30, Olaf Hering wrote:

 Is there a way to not use WRITE_SAME at all? While browsing the code its
 not clear if there is a conditional for this command.

It seems scsi_device-no_write_same may avoid this command, I will
test this patch:

# Subject: [PATCH] scsi: storvsc: avoid usage of WRITE_SAME

Set scsi_device-no_write_same because the host does not support it.
Also blacklist WRITE_SAME to avoid (and log) accident usage.

Signed-off-by: Olaf Hering o...@aepfle.de
---
 drivers/scsi/storvsc_drv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 0144078..21f788a 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1155,6 +1155,8 @@ static int storvsc_device_configure(struct scsi_device 
*sdevice)
 
blk_queue_bounce_limit(sdevice-request_queue, BLK_BOUNCE_ANY);
 
+   sdevice-no_write_same = 1;
+
return 0;
 }
 
@@ -1241,6 +1243,7 @@ static bool storvsc_scsi_cmd_ok(struct scsi_cmnd *scmnd)
 * smartd sends this command and the host does not handle
 * this. So, don't send it.
 */
+   case WRITE_SAME:
case SET_WINDOW:
scmnd-result = ILLEGAL_REQUEST  16;
allowed = false;
-- 
1.8.1.1

--
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] libosd: check for kzalloc() failure

2013-01-30 Thread Benny Halevy
On Wed, Jan 30, 2013 at 3:00 PM, walter harms wha...@bfs.de wrote:


 Am 30.01.2013 10:51, schrieb Benny Halevy:
 On Wed, Jan 30, 2013 at 10:57 AM, walter harms wha...@bfs.de wrote:


 Am 30.01.2013 09:27, schrieb Dan Carpenter:
 On Wed, Jan 30, 2013 at 09:15:43AM +0100, walter harms wrote:


 Am 30.01.2013 08:06, schrieb Dan Carpenter:
 There wasn't any error handling for this kzalloc().

 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

 diff --git a/drivers/scsi/osd/osd_initiator.c 
 b/drivers/scsi/osd/osd_initiator.c
 index c06b8e5..d8293f2 100644
 --- a/drivers/scsi/osd/osd_initiator.c
 +++ b/drivers/scsi/osd/osd_initiator.c
 @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct 
 osd_dev *od,
 odi-osdname_len = get_attrs[a].len;
 /* Avoid NULL for memcmp optimization 0-length is good enough */
 odi-osdname = kzalloc(odi-osdname_len + 1, GFP_KERNEL);
 +   if (!odi-osdname) {
 +   ret = -ENOMEM;
 +   goto out;
 +   }
 if (odi-osdname_len)
 memcpy(odi-osdname, get_attrs[a].val_ptr, odi-osdname_len);
 OSD_INFO(OSD_NAME   [%s]\n, odi-osdname);
 --

 this looks like strdup() ?


 Maybe?  It's a funny thing going on with the NUL terminator and I
 don't understand what the comment is about.

 It appears that normally get_attrs[a].val_ptr is a NUL terminated
 string but get_attrs[a].len does not count the terminator.

 Odd.

 i have no clue what the programmer was thinking. if i read this correct
 osdname is u8 *osdname; so a simple strdup() or strndup() would be ok
 the comment seems to indicate that get_attrs[a].val_ptr could be NULL
 but where is the check ?
 Perhaps they are not using ascii here ? then a memdup(get_attrs[a].len)
 would be better.

 There are subtle differences between kstrdup or kmemdup and this 
 implementation.

 kmemdup is problematic as get_attrs[a].val_ptr does not contain the
 NUL terminator

 ok, i understand - but can we assume that we are talking ascii here ?


No, it can be anything.  UTF-8 is more likely but not guaranteed either.

 In the following case:
 if get_attrs[a].len == 0
 then get_attrs[a].val_ptr == NULL

 The end result should be
 odi-osdname_len = 0
 odi-osdname = kzalloc(1, GFP_KERNEL);

 while with kstrdup, odi-osdname will end up being NULL
 as it's input arg s is NULL.


 and you want the argument to be  ?
 May i ask why ? kfree() can handle NULL and kprintf() (-family) also.

It was Boaz' decision at the time to simplify internal tests
like _the_same_or_null in drivers/scsi/osd/osd_uld.c
that doesn't check for NULL

Nothing spectacular :)

Benny


 re,
  wh


 Benny


 re,
  wh




--
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] libosd: check for kzalloc() failure

2013-01-30 Thread walter harms


Am 30.01.2013 14:40, schrieb Benny Halevy:
 On Wed, Jan 30, 2013 at 3:00 PM, walter harms wha...@bfs.de wrote:


 Am 30.01.2013 10:51, schrieb Benny Halevy:
 On Wed, Jan 30, 2013 at 10:57 AM, walter harms wha...@bfs.de wrote:


 Am 30.01.2013 09:27, schrieb Dan Carpenter:
 On Wed, Jan 30, 2013 at 09:15:43AM +0100, walter harms wrote:


 Am 30.01.2013 08:06, schrieb Dan Carpenter:
 There wasn't any error handling for this kzalloc().

 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

 diff --git a/drivers/scsi/osd/osd_initiator.c 
 b/drivers/scsi/osd/osd_initiator.c
 index c06b8e5..d8293f2 100644
 --- a/drivers/scsi/osd/osd_initiator.c
 +++ b/drivers/scsi/osd/osd_initiator.c
 @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct 
 osd_dev *od,
 odi-osdname_len = get_attrs[a].len;
 /* Avoid NULL for memcmp optimization 0-length is good enough */
 odi-osdname = kzalloc(odi-osdname_len + 1, GFP_KERNEL);
 +   if (!odi-osdname) {
 +   ret = -ENOMEM;
 +   goto out;
 +   }
 if (odi-osdname_len)
 memcpy(odi-osdname, get_attrs[a].val_ptr, 
 odi-osdname_len);
 OSD_INFO(OSD_NAME   [%s]\n, odi-osdname);
 --

 this looks like strdup() ?


 Maybe?  It's a funny thing going on with the NUL terminator and I
 don't understand what the comment is about.

 It appears that normally get_attrs[a].val_ptr is a NUL terminated
 string but get_attrs[a].len does not count the terminator.

 Odd.

 i have no clue what the programmer was thinking. if i read this correct
 osdname is u8 *osdname; so a simple strdup() or strndup() would be ok
 the comment seems to indicate that get_attrs[a].val_ptr could be NULL
 but where is the check ?
 Perhaps they are not using ascii here ? then a memdup(get_attrs[a].len)
 would be better.

 There are subtle differences between kstrdup or kmemdup and this 
 implementation.

 kmemdup is problematic as get_attrs[a].val_ptr does not contain the
 NUL terminator

 ok, i understand - but can we assume that we are talking ascii here ?

 
 No, it can be anything.  UTF-8 is more likely but not guaranteed either.
 

I start to see the complexity of the situation. Would you mind to add
the comment it can be anything.  UTF-8 is more likely but not guaranteed 
either ?
For now using a pascal-string seems the best solution but it should be warned
that get_attrs[a].val_ptr is NOT a c-string and therefore can not be used with
the printf-family (i guess the situation will become more clear in future)

I have no clue why you need that, using c-strings makes life more easy in the
last minute a sprintf(buf,%u) will save the day ;)

 In the following case:
 if get_attrs[a].len == 0
 then get_attrs[a].val_ptr == NULL

 The end result should be
 odi-osdname_len = 0
 odi-osdname = kzalloc(1, GFP_KERNEL);

 while with kstrdup, odi-osdname will end up being NULL
 as it's input arg s is NULL.


 and you want the argument to be  ?
 May i ask why ? kfree() can handle NULL and kprintf() (-family) also.
 
 It was Boaz' decision at the time to simplify internal tests
 like _the_same_or_null in drivers/scsi/osd/osd_uld.c
 that doesn't check for NULL
 
It look clever to add the NULL test here.
Noone reviewing the code will understand that.
(Rule of least surprise)

just my 2 cents,
re,
 wh


 Nothing spectacular :)
 
 Benny
 

 re,
  wh


 Benny


 re,
  wh




 
 
--
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] scsi: storvsc: avoid usage of WRITE_SAME

2013-01-30 Thread Olaf Hering
Set scsi_device-no_write_same because the host does not support it.
Also blacklist WRITE_SAME to avoid (and log) accident usage.

If the guest uses the ext4 filesystem, storvsc hangs while it prints
these messages in an endless loop:
...
[  161.459523] hv_storvsc vmbus_0_1: cmd 0x41 scsi status 0x2 srb status 0x6
[  161.462157] sd 2:0:0:0: [sda]
[  161.463135] Sense Key : No Sense [current]
[  161.464983] sd 2:0:0:0: [sda]
[  161.465899] Add. Sense: No additional sense information
[  161.468211] hv_storvsc vmbus_0_1: cmd 0x41 scsi status 0x2 srb status 0x6
[  161.475766] sd 2:0:0:0: [sda]
[  161.476728] Sense Key : No Sense [current]
[  161.478284] sd 2:0:0:0: [sda]
[  161.479441] Add. Sense: No additional sense information
...

This happens with a guest running on Windows Server 2012, but happens to
work while running on Windows Server 2008. WRITE_SAME isnt really
supported by both versions, so disable the command usage globally.

Signed-off-by: Olaf Hering o...@aepfle.de
Cc: KY Srinivasan k...@microsoft.com
---
 drivers/scsi/storvsc_drv.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 0144078..314586c 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1155,6 +1155,8 @@ static int storvsc_device_configure(struct scsi_device 
*sdevice)
 
blk_queue_bounce_limit(sdevice-request_queue, BLK_BOUNCE_ANY);
 
+   sdevice-no_write_same = 1;
+
return 0;
 }
 
@@ -1237,6 +1239,8 @@ static bool storvsc_scsi_cmd_ok(struct scsi_cmnd *scmnd)
u8 scsi_op = scmnd-cmnd[0];
 
switch (scsi_op) {
+   /* the host does not handle WRITE_SAME, log accident usage */
+   case WRITE_SAME:
/*
 * smartd sends this command and the host does not handle
 * this. So, don't send it.
-- 
1.8.1.1

--
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 v8 4/4] sd: change to auto suspend mode

2013-01-30 Thread Alan Stern
On Wed, 30 Jan 2013, Aaron Lu wrote:

 From: Lin Ming ming.m@intel.com
 
 Uses block layer runtime pm helper functions in
 scsi_runtime_suspend/resume for devices that take advantage of it.
 
 Remove scsi_autopm_* from sd open/release path and check_events path.
 
 Signed-off-by: Lin Ming ming.m@intel.com
 Signed-off-by: Aaron Lu aaron...@intel.com

A couple of very minor suggestions...

 --- a/drivers/scsi/scsi_pm.c
 +++ b/drivers/scsi/scsi_pm.c

 @@ -144,18 +144,44 @@ static int scsi_bus_restore(struct device *dev)
  
  #ifdef CONFIG_PM_RUNTIME
  
 +static int scsi_blk_runtime_suspend(struct device *dev)
 +{
 + struct scsi_device *sdev = to_scsi_device(dev);

For this routine and the other new ones, it may be slightly more
efficient to pass both dev and sdev as arguments (this depends on how
smart the compiler's optimizer is).  The caller already knows both of
them, after all.

 + int err;
 +
 + err = blk_pre_runtime_suspend(sdev-request_queue);
 + if (err)
 + return err;
 + err = pm_generic_runtime_suspend(dev);
 + blk_post_runtime_suspend(sdev-request_queue, err);
 +
 + return err;
 +}
 +
 +static int scsi_dev_runtime_suspend(struct device *dev)
 +{
 + const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
 + int err;
 +
 + err = scsi_dev_type_suspend(dev, pm ? pm-runtime_suspend : NULL);
 + if (err == -EAGAIN)
 + pm_schedule_suspend(dev, jiffies_to_msecs(
 + round_jiffies_up_relative(HZ/10)));
 +
 + return err;
 +}
 +
  static int scsi_runtime_suspend(struct device *dev)
  {
   int err = 0;
 - const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
  
   dev_dbg(dev, scsi_runtime_suspend\n);
   if (scsi_is_sdev_device(dev)) {
 - err = scsi_dev_type_suspend(dev,
 - pm ? pm-runtime_suspend : NULL);
 - if (err == -EAGAIN)
 - pm_schedule_suspend(dev, jiffies_to_msecs(
 - round_jiffies_up_relative(HZ/10)));
 + struct scsi_device *sdev = to_scsi_device(dev);

There should be a blank line between the declaration and the
executable code.

 + if (sdev-request_queue-dev)
 + err = scsi_blk_runtime_suspend(dev);
 + else
 + err = scsi_dev_runtime_suspend(dev);
   }
  
   /* Insert hooks here for targets, hosts, and transport classes */
 @@ -163,14 +189,36 @@ static int scsi_runtime_suspend(struct device *dev)
   return err;
  }
  
 +static int scsi_blk_runtime_resume(struct device *dev)
 +{
 + struct scsi_device *sdev = to_scsi_device(dev);
 + int err;
 +
 + blk_pre_runtime_resume(sdev-request_queue);
 + err = pm_generic_runtime_resume(dev);
 + blk_post_runtime_resume(sdev-request_queue, err);
 +
 + return err;
 +}
 +
 +static int scsi_dev_runtime_resume(struct device *dev)
 +{
 + const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;

Blank line.

 + return scsi_dev_type_resume(dev, pm ? pm-runtime_resume : NULL);
 +}
 +
  static int scsi_runtime_resume(struct device *dev)
  {
   int err = 0;
 - const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
  
   dev_dbg(dev, scsi_runtime_resume\n);
 - if (scsi_is_sdev_device(dev))
 - err = scsi_dev_type_resume(dev, pm ? pm-runtime_resume : NULL);
 + if (scsi_is_sdev_device(dev)) {
 + struct scsi_device *sdev = to_scsi_device(dev);

Blank line.

 + if (sdev-request_queue-dev)
 + err = scsi_blk_runtime_resume(dev);
 + else
 + err = scsi_dev_runtime_resume(dev);
 + }
  
   /* Insert hooks here for targets, hosts, and transport classes */
  
 @@ -185,10 +233,17 @@ static int scsi_runtime_idle(struct device *dev)
  
   /* Insert hooks here for targets, hosts, and transport classes */
  
 - if (scsi_is_sdev_device(dev))
 - err = pm_schedule_suspend(dev, 100);
 - else
 + if (scsi_is_sdev_device(dev)) {
 + struct scsi_device *sdev = to_scsi_device(dev);

Blank line.

 + if (sdev-request_queue-dev) {
 + pm_runtime_mark_last_busy(dev);
 + err = pm_runtime_autosuspend(dev);
 + } else {
 + err = pm_schedule_suspend(dev, 100);
 + }
 + } else {
   err = pm_runtime_suspend(dev);
 + }
   return err;
  }

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] [SCSI] libosd: check for kzalloc() failure

2013-01-30 Thread Boaz Harrosh
On 01/30/2013 04:34 PM, walter harms wrote:
 

 I start to see the complexity of the situation. Would you mind to add
 the comment it can be anything.  UTF-8 is more likely but not guaranteed 
 either ?
 For now using a pascal-string seems the best solution but it should be warned
 that get_attrs[a].val_ptr is NOT a c-string and therefore can not be used with
 the printf-family (i guess the situation will become more clear in future)
 

OK, So... The long story

The STD says that osdname can be *anything* binary or otherwise, and of *any*
length. And is used to uniquely identify the OSD-device/partition in a cluster.

We decided that if so we would stick an 40 char UUID in it, generated by a 
uuidgen
and all the external utilities and surrounding code forces it, and treat it as
a c-string. But this code here in the core cannot make that assumption and still
support the STD.

On the other hand we did want the osdname to be printable in traces and messages
because it is such an important identifier. So I have decided to sacrifice an
extra char in-memory to carry a \NUL and safely stick it into printk(s). Those
(none existent) OSD devices that will put unprintable characters in here will
still function fine, but will look real scary in printk(s).

Please note that the one that sets policy is the osd-target vendor. (And they
all currently use my code base)

So save the kzalloc check this code (tested) is safe and will show strings when
present, but will gracefully show ugly things but still work when not.

 I have no clue why you need that, using c-strings makes life more easy in the
 last minute a sprintf(buf,%u) will save the day ;)
 

Actually it is very funny, because just recently (last week) I have discovered
something that eliminates all those funny business.

printf(%*s, odi-osdname, odi-osdname_len);

The * will instruct c to expect an extra variable following %s which is the
max_length of %s. This is exactly for pascal strings and the such like above.

So I added a TODO to clean that a bit by always printing with %*s


 It look clever to add the NULL test here.
 Noone reviewing the code will understand that.
 (Rule of least surprise)
 

Thanks for looking, I agree it needs a fat comment, I'll do that when I'll
convert to above system. Thanks for looking, That code is really old and never
had any extra eyes on it.

 just my 2 cents,
 re,
  wh
 

Cheers
Boaz

--
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 v8 2/4] block: add runtime pm helpers

2013-01-30 Thread Alan Stern
On Wed, 30 Jan 2013, Aaron Lu wrote:

 From: Lin Ming ming.m@intel.com
 
 Add runtime pm helper functions:
 
 void blk_pm_runtime_init(struct request_queue *q, struct device *dev)
   - Initialization function for drivers to call.
 
 int blk_pre_runtime_suspend(struct request_queue *q)
   - If any requests are in the queue, mark last busy and return -EBUSY.
 Otherwise set q-rpm_status to RPM_SUSPENDING and return 0.
 
 void blk_post_runtime_suspend(struct request_queue *q, int err)
   - If the suspend succeeded then set q-rpm_status to RPM_SUSPENDED.
 Otherwise set it to RPM_ACTIVE.
 
 void blk_pre_runtime_resume(struct request_queue *q)
   - Set q-rpm_status to RPM_RESUMING.
 
 void blk_post_runtime_resume(struct request_queue *q, int err)
   - If the resume succeeded then set q-rpm_status to RPM_ACTIVE
 and call __blk_run_queue, then mark last busy and autosuspend.
 Otherwise set q-rpm_status to RPM_SUSPENDED.
 
 Signed-off-by: Lin Ming ming.m@intel.com
 Signed-off-by: Aaron Lu aaron...@intel.com

 +void blk_pm_runtime_init(struct request_queue *q, struct device *dev)
 +{
 + q-dev = dev;
 + q-rpm_status = RPM_ACTIVE;
 + pm_runtime_set_autosuspend_delay(q-dev, -1);
 + pm_runtime_use_autosuspend(q-dev);
 + pm_runtime_mark_last_busy(q-dev);
 + pm_runtime_autosuspend(q-dev);

This last line is no longer needed.  It can't do anything useful, since 
autosuspends are disabled (the delay is -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


Re: [patch] [SCSI] libosd: check for kzalloc() failure

2013-01-30 Thread Boaz Harrosh
On 01/30/2013 09:06 AM, Dan Carpenter wrote:
 There wasn't any error handling for this kzalloc().
 

ACK-by: Boaz Harrosh bharr...@panasas.com

James please queue for inclusion

 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

Thanks Dan

 
 diff --git a/drivers/scsi/osd/osd_initiator.c 
 b/drivers/scsi/osd/osd_initiator.c
 index c06b8e5..d8293f2 100644
 --- a/drivers/scsi/osd/osd_initiator.c
 +++ b/drivers/scsi/osd/osd_initiator.c
 @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev *od,
   odi-osdname_len = get_attrs[a].len;
   /* Avoid NULL for memcmp optimization 0-length is good enough */
   odi-osdname = kzalloc(odi-osdname_len + 1, GFP_KERNEL);
 + if (!odi-osdname) {
 + ret = -ENOMEM;
 + goto out;
 + }
   if (odi-osdname_len)
   memcpy(odi-osdname, get_attrs[a].val_ptr, odi-osdname_len);
   OSD_INFO(OSD_NAME   [%s]\n, odi-osdname);
 

--
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: storvsc loops with No Sense messages

2013-01-30 Thread KY Srinivasan


 -Original Message-
 From: Olaf Hering [mailto:o...@aepfle.de]
 Sent: Wednesday, January 30, 2013 5:00 AM
 To: KY Srinivasan
 Cc: linux-scsi@vger.kernel.org
 Subject: Re: storvsc loops with No Sense messages
 
 On Tue, Jan 29, Olaf Hering wrote:
 
  On Tue, Jan 29, KY Srinivasan wrote:
 
   Was the installation done on a vhd or VHDX. I checked with Hyper-V
   guys and they say they never supported WRITE_SAME. I am wondering how
   it worked on ws2008.
 
  Its a vhdx image, both fixed and dynamically size have the same issue.
  I will see what happens with ws2008, at least the install proceeds
  without issues.
 
 I see that WRITE_SAME is also used when running on ws2008, but
 appearently it does not cause any issues.
 This triggers only when the default ext4 is used, not when I force ext3
 for the root filesystem.

Talking to the MSFT storage guys, WRITE_SAME has never been supported - not
on ws2008 or on ws2012. So I am not sure why or how this works on ws2008.

K. Y
 So to me it looks like ws2012 has issues with WRITE_SAME handling.
 
 
 Is there a way to not use WRITE_SAME at all? While browsing the code its
 not clear if there is a conditional for this command.
 
 
 Olaf
 



RE: storvsc loops with No Sense messages

2013-01-30 Thread KY Srinivasan


 -Original Message-
 From: Olaf Hering [mailto:o...@aepfle.de]
 Sent: Wednesday, January 30, 2013 8:35 AM
 To: KY Srinivasan
 Cc: linux-scsi@vger.kernel.org
 Subject: Re: storvsc loops with No Sense messages
 
 On Wed, Jan 30, Olaf Hering wrote:
 
  Is there a way to not use WRITE_SAME at all? While browsing the code its
  not clear if there is a conditional for this command.
 
 It seems scsi_device-no_write_same may avoid this command, I will
 test this patch:
 
 # Subject: [PATCH] scsi: storvsc: avoid usage of WRITE_SAME
 
 Set scsi_device-no_write_same because the host does not support it.
 Also blacklist WRITE_SAME to avoid (and log) accident usage.

Olaf,

Thanks for looking into this.

Acked-by:  K. Y. Srinivasan k...@microsoft.com
 
 Signed-off-by: Olaf Hering o...@aepfle.de
 ---
  drivers/scsi/storvsc_drv.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
 index 0144078..21f788a 100644
 --- a/drivers/scsi/storvsc_drv.c
 +++ b/drivers/scsi/storvsc_drv.c
 @@ -1155,6 +1155,8 @@ static int storvsc_device_configure(struct scsi_device
 *sdevice)
 
   blk_queue_bounce_limit(sdevice-request_queue, BLK_BOUNCE_ANY);
 
 + sdevice-no_write_same = 1;
 +
   return 0;
  }
 
 @@ -1241,6 +1243,7 @@ static bool storvsc_scsi_cmd_ok(struct scsi_cmnd
 *scmnd)
* smartd sends this command and the host does not handle
* this. So, don't send it.
*/
 + case WRITE_SAME:
   case SET_WINDOW:
   scmnd-result = ILLEGAL_REQUEST  16;
   allowed = false;
 --
 1.8.1.1
 
 



Re: [PATCH 1/6] qla2xxx: Determine the number of outstanding commands based on available resources.

2013-01-30 Thread Chad Dupuis

FYI, the fix the smatch warning:

+ drivers/scsi/qla2xxx/qla_os.c:364 qla2x00_free_req_que() info:
+ redundant null check on req-outstanding_cmds calling kfree()

has been merged with this patch.

On Wed, 30 Jan 2013, Saurav Kashyap wrote:


From: Chad Dupuis chad.dup...@qlogic.com

Base the number of outstanding requests the driver will keep track of on the
available resources instead of being hard-coded.

Signed-off-by: Chad Dupuis chad.dup...@qlogic.com
Signed-off-by: Saurav Kashyap saurav.kash...@qlogic.com
---
drivers/scsi/qla2xxx/qla_bsg.c|2 +-
drivers/scsi/qla2xxx/qla_dbg.c|2 +-
drivers/scsi/qla2xxx/qla_def.h|   12 
drivers/scsi/qla2xxx/qla_gbl.h|3 ++
drivers/scsi/qla2xxx/qla_init.c   |   54 +++-
drivers/scsi/qla2xxx/qla_iocb.c   |   37 -
drivers/scsi/qla2xxx/qla_isr.c|   10 +++---
drivers/scsi/qla2xxx/qla_mbx.c|8 +++---
drivers/scsi/qla2xxx/qla_mid.c|7 -
drivers/scsi/qla2xxx/qla_nx.c |2 +-
drivers/scsi/qla2xxx/qla_os.c |   11 +--
drivers/scsi/qla2xxx/qla_target.c |4 +-
drivers/scsi/qla2xxx/qla_target.h |5 ++-
13 files changed, 110 insertions(+), 47 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index 9f34ded..f7cb6a3 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -1950,7 +1950,7 @@ qla24xx_bsg_timeout(struct fc_bsg_job *bsg_job)
  if (!req)
  continue;

- for (cnt = 1; cnt  MAX_OUTSTANDING_COMMANDS; cnt++) {
+ for (cnt = 1; cnt  req-num_outstanding_cmds; cnt++) {
  sp = req-outstanding_cmds[cnt];
  if (sp) {
  if (((sp-type == SRB_CT_CMD) ||
diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index 53f9e49..2f3e765 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -11,7 +11,7 @@
 * --
 * | Level|   Last Value Used  | Holes|
 * --
- * | Module Init and Probe|   0x0125   | 0x4b,0xba,0xfa |
+ * | Module Init and Probe|   0x0126   | 0x4b,0xba,0xfa |
 * | Mailbox commands |   0x114f   | 0x111a-0x111b  |
 * |  || 0x112c-0x112e  |
 * |  || 0x113a |
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 6e7727f..a84bb8d 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -253,8 +253,8 @@
#define LOOP_DOWN_TIME255 /* 240 */
#define   LOOP_DOWN_RESET (LOOP_DOWN_TIME - 30)

-/* Maximum outstanding commands in ISP queues (1-65535) */
-#define MAX_OUTSTANDING_COMMANDS 1024
+#define DEFAULT_OUTSTANDING_COMMANDS 1024
+#define MIN_OUTSTANDING_COMMANDS 128

/* ISP request and response entry counts (37-65535) */
#define REQUEST_ENTRY_CNT_2100128 /* Number of request 
entries. */
@@ -2533,8 +2533,9 @@ struct req_que {
  uint16_t  qos;
  uint16_t  vp_idx;
  struct rsp_que *rsp;
- srb_t *outstanding_cmds[MAX_OUTSTANDING_COMMANDS];
+ srb_t **outstanding_cmds;
  uint32_t current_outstanding_cmd;
+ uint16_t num_outstanding_cmds;
  int max_q_depth;
};

@@ -2561,7 +2562,7 @@ struct qlt_hw_data {
  void *target_lport_ptr;
  struct qla_tgt_func_tmpl *tgt_ops;
  struct qla_tgt *qla_tgt;
- struct qla_tgt_cmd *cmds[MAX_OUTSTANDING_COMMANDS];
+ struct qla_tgt_cmd *cmds[DEFAULT_OUTSTANDING_COMMANDS];
  uint16_t current_handle;

  struct qla_tgt_vp_map *tgt_vp_map;
@@ -2887,6 +2888,7 @@ struct qla_hw_data {
#define RISC_START_ADDRESS_2300 0x800
#define RISC_START_ADDRESS_2400 0x10
  uint16_tfw_xcb_count;
+ uint16_tfw_iocb_count;

  uint16_tfw_options[16]; /* slots: 1,2,3,10,11 */
  uint8_t fw_seriallink_options[4];
@@ -3248,8 +3250,6 @@ struct qla_tgt_vp_map {

#define NVRAM_DELAY() udelay(10)

-#define INVALID_HANDLE   (MAX_OUTSTANDING_COMMANDS+1)
-
/*
 * Flash support definitions
 */
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index 2411d1a..fba0651 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -84,6 +84,9 @@ extern int qla83xx_nic_core_reset(scsi_qla_host_t *);
extern void qla83xx_reset_ownership(scsi_qla_host_t *);
extern int qla2xxx_mctp_dump(scsi_qla_host_t *);

+extern int
+qla2x00_alloc_outstanding_cmds(struct qla_hw_data *, struct req_que *);
+
/*
 * Global Data in qla_os.c source file.
 */
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 

Re: [PATCH v8 2/4] block: add runtime pm helpers

2013-01-30 Thread Aaron Lu
On Wed, Jan 30, 2013 at 10:54:53AM -0500, Alan Stern wrote:
 On Wed, 30 Jan 2013, Aaron Lu wrote:
 
  From: Lin Ming ming.m@intel.com
  
  Add runtime pm helper functions:
  
  void blk_pm_runtime_init(struct request_queue *q, struct device *dev)
- Initialization function for drivers to call.
  
  int blk_pre_runtime_suspend(struct request_queue *q)
- If any requests are in the queue, mark last busy and return -EBUSY.
  Otherwise set q-rpm_status to RPM_SUSPENDING and return 0.
  
  void blk_post_runtime_suspend(struct request_queue *q, int err)
- If the suspend succeeded then set q-rpm_status to RPM_SUSPENDED.
  Otherwise set it to RPM_ACTIVE.
  
  void blk_pre_runtime_resume(struct request_queue *q)
- Set q-rpm_status to RPM_RESUMING.
  
  void blk_post_runtime_resume(struct request_queue *q, int err)
- If the resume succeeded then set q-rpm_status to RPM_ACTIVE
  and call __blk_run_queue, then mark last busy and autosuspend.
  Otherwise set q-rpm_status to RPM_SUSPENDED.
  
  Signed-off-by: Lin Ming ming.m@intel.com
  Signed-off-by: Aaron Lu aaron...@intel.com
 
  +void blk_pm_runtime_init(struct request_queue *q, struct device *dev)
  +{
  +   q-dev = dev;
  +   q-rpm_status = RPM_ACTIVE;
  +   pm_runtime_set_autosuspend_delay(q-dev, -1);
  +   pm_runtime_use_autosuspend(q-dev);
  +   pm_runtime_mark_last_busy(q-dev);
  +   pm_runtime_autosuspend(q-dev);
 
 This last line is no longer needed.  It can't do anything useful, since 
 autosuspends are disabled (the delay is -1).

Right, thanks.
And the mark_last_busy can probably be removed too, it didn't make much
sense here and we can add driver should call pm_runtime_mark_last_busy
and pm_runtime_autosuspend in its runtime idle callback to the kernel
doc. What do you think?

Thanks,
Aaron

--
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 v8 4/4] sd: change to auto suspend mode

2013-01-30 Thread Aaron Lu
On Wed, Jan 30, 2013 at 10:38:26AM -0500, Alan Stern wrote:
 On Wed, 30 Jan 2013, Aaron Lu wrote:
 
  From: Lin Ming ming.m@intel.com
  
  Uses block layer runtime pm helper functions in
  scsi_runtime_suspend/resume for devices that take advantage of it.
  
  Remove scsi_autopm_* from sd open/release path and check_events path.
  
  Signed-off-by: Lin Ming ming.m@intel.com
  Signed-off-by: Aaron Lu aaron...@intel.com
 
 A couple of very minor suggestions...
 
  --- a/drivers/scsi/scsi_pm.c
  +++ b/drivers/scsi/scsi_pm.c
 
  @@ -144,18 +144,44 @@ static int scsi_bus_restore(struct device *dev)
   
   #ifdef CONFIG_PM_RUNTIME
   
  +static int scsi_blk_runtime_suspend(struct device *dev)
  +{
  +   struct scsi_device *sdev = to_scsi_device(dev);
 
 For this routine and the other new ones, it may be slightly more
 efficient to pass both dev and sdev as arguments (this depends on how
 smart the compiler's optimizer is).  The caller already knows both of
 them, after all.

What about passing only scsi_device? When device is needed, I can use
sdev-sdev_gendev. Is this equally efficient?

   static int scsi_runtime_suspend(struct device *dev)
   {
  int err = 0;
  -   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
   
  dev_dbg(dev, scsi_runtime_suspend\n);
  if (scsi_is_sdev_device(dev)) {
  -   err = scsi_dev_type_suspend(dev,
  -   pm ? pm-runtime_suspend : NULL);
  -   if (err == -EAGAIN)
  -   pm_schedule_suspend(dev, jiffies_to_msecs(
  -   round_jiffies_up_relative(HZ/10)));
  +   struct scsi_device *sdev = to_scsi_device(dev);
 
 There should be a blank line between the declaration and the
 executable code.

OK, will change this.

  @@ -185,10 +233,17 @@ static int scsi_runtime_idle(struct device *dev)
   
  /* Insert hooks here for targets, hosts, and transport classes */
   
  -   if (scsi_is_sdev_device(dev))
  -   err = pm_schedule_suspend(dev, 100);
  -   else
  +   if (scsi_is_sdev_device(dev)) {
  +   struct scsi_device *sdev = to_scsi_device(dev);
 
 Blank line.
 
  +   if (sdev-request_queue-dev) {
  +   pm_runtime_mark_last_busy(dev);
  +   err = pm_runtime_autosuspend(dev);
  +   } else {
  +   err = pm_schedule_suspend(dev, 100);
  +   }
  +   } else {
  err = pm_runtime_suspend(dev);
  +   }
  return err;

Shall we ignore the return value for these pm_xxx_suspend functions?
I mean we do not need to record the return value for them and return it,
since pm core doesn't care the return value of idle callback.

Thanks,
Aaron

--
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 0/1] Fix oops while resetting an ipr adapter

2013-01-30 Thread wenxiong
When hotplug removing an adapter, we've seen the issues with
scsi_unblock_requests running after the adapter's memory has been
freed. This patch fixes the oops while resetting an ipr adapter.

Thanks!
Wendy

-- 
--
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 1/1] ipr: Fix oops while resetting an ipr adapter

2013-01-30 Thread wenxiong
From: Brian King brk...@linux.vnet.ibm.com

When resetting an ipr adapter, we use scsi_block_requests to
block any new commands from scsi core, and then unblock after
the reset. When hotplug removing an adapter, we shut it down
and go through this same code, but we've seen issues with
scsi_unblock_requests running after the adapter's memory has
been freed. There is really no need to block/unblock when
the adapter is being removed, so this patch skips the
block/unblock and will immediately fail any commands that
happen to make it to queuecommand while the adapter is
being shutdown.

Signed-off-by: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Wen Xiong wenxi...@linux.vnet.ibm.com
---

 drivers/scsi/ipr.c |   33 +++--
 drivers/scsi/ipr.h |1 +
 2 files changed, 24 insertions(+), 10 deletions(-)

Index: b/drivers/scsi/ipr.c
===
--- a/drivers/scsi/ipr.c2013-01-21 13:45:10.0 -0600
+++ b/drivers/scsi/ipr.c2013-01-22 14:21:00.394286989 -0600
@@ -6112,7 +6112,7 @@ static int ipr_queuecommand(struct Scsi_
 * We have told the host to stop giving us new requests, but
 * ERP ops don't count. FIXME
 */
-   if (unlikely(!hrrq-allow_cmds  !hrrq-ioa_is_dead)) {
+   if (unlikely(!hrrq-allow_cmds  !hrrq-ioa_is_dead  
!hrrq-removing_ioa)) {
spin_unlock_irqrestore(hrrq-lock, hrrq_flags);
return SCSI_MLQUEUE_HOST_BUSY;
}
@@ -6121,7 +6121,7 @@ static int ipr_queuecommand(struct Scsi_
 * FIXME - Create scsi_set_host_offline interface
 *  and the ioa_is_dead check can be removed
 */
-   if (unlikely(hrrq-ioa_is_dead || !res)) {
+   if (unlikely(hrrq-ioa_is_dead || hrrq-removing_ioa || !res)) {
spin_unlock_irqrestore(hrrq-lock, hrrq_flags);
goto err_nodev;
}
@@ -6741,14 +6741,17 @@ static int ipr_ioa_bringdown_done(struct
struct ipr_ioa_cfg *ioa_cfg = ipr_cmd-ioa_cfg;
 
ENTER;
+   if (!ioa_cfg-hrrq[IPR_INIT_HRRQ].removing_ioa) {
+   ipr_trace;
+   spin_unlock_irq(ioa_cfg-host-host_lock);
+   scsi_unblock_requests(ioa_cfg-host);
+   spin_lock_irq(ioa_cfg-host-host_lock);
+   }
+
ioa_cfg-in_reset_reload = 0;
ioa_cfg-reset_retries = 0;
list_add_tail(ipr_cmd-queue, ipr_cmd-hrrq-hrrq_free_q);
wake_up_all(ioa_cfg-reset_wait_q);
-
-   spin_unlock_irq(ioa_cfg-host-host_lock);
-   scsi_unblock_requests(ioa_cfg-host);
-   spin_lock_irq(ioa_cfg-host-host_lock);
LEAVE;
 
return IPR_RC_JOB_RETURN;
@@ -8494,7 +8497,8 @@ static void _ipr_initiate_ioa_reset(stru
spin_unlock(ioa_cfg-hrrq[i]._lock);
}
wmb();
-   scsi_block_requests(ioa_cfg-host);
+   if (!ioa_cfg-hrrq[IPR_INIT_HRRQ].removing_ioa)
+   scsi_block_requests(ioa_cfg-host);
 
ipr_cmd = ipr_get_free_ipr_cmnd(ioa_cfg);
ioa_cfg-reset_cmd = ipr_cmd;
@@ -8549,9 +8553,11 @@ static void ipr_initiate_ioa_reset(struc
ipr_fail_all_ops(ioa_cfg);
wake_up_all(ioa_cfg-reset_wait_q);
 
-   spin_unlock_irq(ioa_cfg-host-host_lock);
-   scsi_unblock_requests(ioa_cfg-host);
-   spin_lock_irq(ioa_cfg-host-host_lock);
+   if (!ioa_cfg-hrrq[IPR_INIT_HRRQ].removing_ioa) {
+   spin_unlock_irq(ioa_cfg-host-host_lock);
+   scsi_unblock_requests(ioa_cfg-host);
+   spin_lock_irq(ioa_cfg-host-host_lock);
+   }
return;
} else {
ioa_cfg-in_ioa_bringdown = 1;
@@ -9695,6 +9701,7 @@ static void __ipr_remove(struct pci_dev 
 {
unsigned long host_lock_flags = 0;
struct ipr_ioa_cfg *ioa_cfg = pci_get_drvdata(pdev);
+   int i;
ENTER;
 
spin_lock_irqsave(ioa_cfg-host-host_lock, host_lock_flags);
@@ -9704,6 +9711,12 @@ static void __ipr_remove(struct pci_dev 
spin_lock_irqsave(ioa_cfg-host-host_lock, host_lock_flags);
}
 
+   for (i = 0; i  ioa_cfg-hrrq_num; i++) {
+   spin_lock(ioa_cfg-hrrq[i]._lock);
+   ioa_cfg-hrrq[i].removing_ioa = 1;
+   spin_unlock(ioa_cfg-hrrq[i]._lock);
+   }
+   wmb();
ipr_initiate_ioa_bringdown(ioa_cfg, IPR_SHUTDOWN_NORMAL);
 
spin_unlock_irqrestore(ioa_cfg-host-host_lock, host_lock_flags);
Index: b/drivers/scsi/ipr.h
===
--- a/drivers/scsi/ipr.h2013-01-21 13:45:10.0 -0600
+++ b/drivers/scsi/ipr.h2013-01-22 11:10:23.414280773 -0600
@@ -493,6 +493,7 @@ struct ipr_hrr_queue {
u8 allow_interrupts:1;
u8 ioa_is_dead:1;
u8