Re: [PATCH RESEND 0/1] AHCI: Optimize interrupt processing

2013-07-19 Thread Nicholas A. Bellinger
On Thu, 2013-07-18 at 18:03 -0700, Nicholas A. Bellinger wrote:
 On Thu, 2013-07-18 at 18:30 -0600, Jens Axboe wrote:
  On Thu, Jul 18 2013, Nicholas A. Bellinger wrote:
   On Thu, 2013-07-18 at 13:12 -0600, Mike Christie wrote:
On 07/18/2013 12:51 PM, Nicholas A. Bellinger wrote:

SNIP

   nod, just noticed the blk_queue_bounce() in blk_rq_map_kern().  
   
   Not sure why this doesn't seem to be doing what it's supposed to for
   libata just yet..
  
  How are you make the request from the bio? It'd be pretty trivial to
  ensure that it gets bounced properly... blk_mq_execute_rq() assumes a
  fully complete request, so it wont bounce anything.
  
 
 From what I gather for REQ_TYPE_BLOCK_PC, scsi_execute() -
 blk_rq_map_kern() - blk_rq_append_bio() - blk_rq_bio_prep() is what
 does the request setup from the bios returned by bio_[copy,map]_kern()
 in blk_rq_map_kern() code.
 
 blk_queue_bounce() is called immediately after blk_rq_append_bio() here,
 which AFAICT looks like it's doing the correct thing for scsi-mq..
 
 What is strange here is that libata-scsi.c CDB emulation code is doing
 the same stuff for both INQUIRY (that seems to be OK) and READ_CAPACITY
 (that is returning zeros), which makes me think that something else is
 going on..
 
 Alexander, where you able to re-test using sdev-sdev_mq_reg.queue_depth
 = 1 in scsi_mq_alloc_queue()..?
 

So after a bit more hacking tonight it appears the explicit setting of
dma_alignment by libata (sdev-sector_size - 1) is what is making
blk_rq_map_kern() invoke blk_copy_kern() instead of blk_map_kern(), and
triggering this scsi-mq specific bug with libata.  I'm able to confirm
with QEMU IDE emulation the bug is occurring only after INQUIRY and
before READ_CAPACITY, as reported by Alexander.

Below is a quick hack to skip this setting in ata_scsi_dev_config() for
blk-mq, and leaves the default dma_alignment=0x03 for REQ_TYPE_BLOCK_PC
requests as initially set by scsi-core in scsi_init_request_queue().

Also included is the change for using queue_depth = min(SHT-can_queue,
SHT-cmd_per_lun) during scsi-mq request_queue initialization, along
with a very basic ata_piix conversion for testing purposes.

With these three changes in place, I'm able to register a single 1GB
ata_piix LUN using QEMU IDE emulation, and successfully run simple fio
writeverify tests with blocksize=4k @ queue_depth=1.

Obviously this is not a proper bugfix for unaligned blk_copy_kern() with
scsi-mq + REQ_TYPE_BLOCK_PC case, but should be enough to at least get
libata LUN scanning to work.  Need to take a deeper look at the problem
here..

Alexander, care to give this work-around a shot on your bare-metal setup
using ata_piix..?

--nab

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 9a8a674..ac05cd6 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -1066,6 +1066,8 @@ static u8 piix_vmw_bmdma_status(struct ata_port *ap)
 
 static struct scsi_host_template piix_sht = {
ATA_BMDMA_SHT(DRV_NAME),
+   .scsi_mq= true,
+   .queuecommand_mq = ata_scsi_queuecmd,
 };
 
 static struct ata_port_operations piix_sata_ops = {
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 0101af5..191bc15 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1144,7 +1144,11 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
sector_size=%u  PAGE_SIZE, PIO may malfunction\n,
sdev-sector_size);
 
-   blk_queue_update_dma_alignment(q, sdev-sector_size - 1);
+   if (!q-mq_ops) {
+   blk_queue_update_dma_alignment(q, sdev-sector_size - 1);
+   } else {
+   printk(Skipping dma_alignment for libata w/ scsi-mq\n);
+   }
 
if (dev-flags  ATA_DFLAG_AN)
set_bit(SDEV_EVT_MEDIA_CHANGE, sdev-supported_events);
diff --git a/drivers/scsi/scsi-mq.c b/drivers/scsi/scsi-mq.c
index ca6ff67..81b2633 100644
--- a/drivers/scsi/scsi-mq.c
+++ b/drivers/scsi/scsi-mq.c
@@ -199,11 +199,11 @@ int scsi_mq_alloc_queue(struct Scsi_Host *sh, struct 
scsi_device *sdev)
int i, j;
 
sdev-sdev_mq_reg.ops = scsi_mq_ops;
-   sdev-sdev_mq_reg.queue_depth = sdev-queue_depth;
+   sdev-sdev_mq_reg.queue_depth = min((short)sh-hostt-can_queue,
+   sh-hostt-cmd_per_lun);
sdev-sdev_mq_reg.cmd_size = sizeof(struct scsi_cmnd) + 
sh-hostt-cmd_size;
sdev-sdev_mq_reg.numa_node = NUMA_NO_NODE;
sdev-sdev_mq_reg.nr_hw_queues = 1;
-   sdev-sdev_mq_reg.queue_depth = 64;
sdev-sdev_mq_reg.flags = BLK_MQ_F_SHOULD_MERGE;
 
printk(Calling blk_mq_init_queue: scsi_mq_ops: %p, queue_depth: %d,

--
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: replace strict_strtoul() with kstrtoul()

2013-07-19 Thread Jingoo Han
The usage of strict_strtoul() is not preferred, because
strict_strtoul() is obsolete. Thus, kstrtoul() should be
used.

Signed-off-by: Jingoo Han jg1@samsung.com
---
 drivers/scsi/pmcraid.c|6 --
 drivers/scsi/scsi_sysfs.c |6 --
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index 1eb7b028..8cd98e6 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -4203,9 +4203,11 @@ static ssize_t pmcraid_store_log_level(
struct Scsi_Host *shost;
struct pmcraid_instance *pinstance;
unsigned long val;
+   int ret;
 
-   if (strict_strtoul(buf, 10, val))
-   return -EINVAL;
+   ret = kstrtoul(buf, 10, val);
+   if (ret)
+   return ret;
/* log-level should be from 0 to 2 */
if (val  2)
return -EINVAL;
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 7e50061..a876e9b 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -819,9 +819,11 @@ sdev_store_queue_ramp_up_period(struct device *dev,
 {
struct scsi_device *sdev = to_scsi_device(dev);
unsigned long period;
+   int ret;
 
-   if (strict_strtoul(buf, 10, period))
-   return -EINVAL;
+   ret = kstrtoul(buf, 10, period);
+   if (ret)
+   return ret;
 
sdev-queue_ramp_up_period = msecs_to_jiffies(period);
return period;
-- 
1.7.10.4


--
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] target: replace strict_strto*() with kstrto*()

2013-07-19 Thread Jingoo Han
The usage of strict_strtoul() and strict_strtoull() is not preferred,
because strict_strtoul() and strict_strtoull() are obsolete. Thus,
kstrtoul() and kstrtoull() should be used.

Signed-off-by: Jingoo Han jg1@samsung.com
---
 drivers/target/iscsi/iscsi_target_configfs.c   |8 ++---
 drivers/target/iscsi/iscsi_target_login.c  |2 +-
 drivers/target/iscsi/iscsi_target_parameters.c |2 +-
 drivers/target/target_core_alua.c  |   32 +-
 drivers/target/target_core_configfs.c  |   42 
 drivers/target/target_core_fabric_configfs.c   |   16 ++---
 drivers/target/target_core_file.c  |4 +--
 drivers/target/target_core_iblock.c|4 +--
 drivers/target/tcm_fc/tfc_conf.c   |6 +++-
 9 files changed, 64 insertions(+), 52 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_configfs.c 
b/drivers/target/iscsi/iscsi_target_configfs.c
index bbfd288..ddfa32c 100644
--- a/drivers/target/iscsi/iscsi_target_configfs.c
+++ b/drivers/target/iscsi/iscsi_target_configfs.c
@@ -265,9 +265,9 @@ static struct se_tpg_np *lio_target_call_addnptotpg(
*port_str = '\0'; /* Terminate string for IP */
port_str++; /* Skip over : */
 
-   ret = strict_strtoul(port_str, 0, port);
+   ret = kstrtoul(port_str, 0, port);
if (ret  0) {
-   pr_err(strict_strtoul() failed for port_str: %d\n, 
ret);
+   pr_err(kstrtoul() failed for port_str: %d\n, ret);
return ERR_PTR(ret);
}
sock_in6 = (struct sockaddr_in6 *)sockaddr;
@@ -290,9 +290,9 @@ static struct se_tpg_np *lio_target_call_addnptotpg(
*port_str = '\0'; /* Terminate string for IP */
port_str++; /* Skip over : */
 
-   ret = strict_strtoul(port_str, 0, port);
+   ret = kstrtoul(port_str, 0, port);
if (ret  0) {
-   pr_err(strict_strtoul() failed for port_str: %d\n, 
ret);
+   pr_err(kstrtoul() failed for port_str: %d\n, ret);
return ERR_PTR(ret);
}
sock_in = (struct sockaddr_in *)sockaddr;
diff --git a/drivers/target/iscsi/iscsi_target_login.c 
b/drivers/target/iscsi/iscsi_target_login.c
index 3402241..76cf1cd 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -428,7 +428,7 @@ static int iscsi_login_zero_tsih_s2(
ISCSI_LOGIN_STATUS_NO_RESOURCES);
return -1;
}
-   rc = strict_strtoul(param-value, 0, mrdsl);
+   rc = kstrtoul(param-value, 0, mrdsl);
if (rc  0) {
iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
ISCSI_LOGIN_STATUS_NO_RESOURCES);
diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index 35fd643..034bd6d 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -1182,7 +1182,7 @@ static int iscsi_check_acceptor_state(struct iscsi_param 
*param, char *value,
unsigned long long tmp;
int rc;
 
-   rc = strict_strtoull(param-value, 0, tmp);
+   rc = kstrtoull(param-value, 0, tmp);
if (rc  0)
return -1;
 
diff --git a/drivers/target/target_core_alua.c 
b/drivers/target/target_core_alua.c
index cbe48ab..5403186 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -1756,10 +1756,10 @@ ssize_t core_alua_store_access_type(
unsigned long tmp;
int ret;
 
-   ret = strict_strtoul(page, 0, tmp);
+   ret = kstrtoul(page, 0, tmp);
if (ret  0) {
pr_err(Unable to extract alua_access_type\n);
-   return -EINVAL;
+   return ret;
}
if ((tmp != 0)  (tmp != 1)  (tmp != 2)  (tmp != 3)) {
pr_err(Illegal value for alua_access_type:
@@ -1794,10 +1794,10 @@ ssize_t core_alua_store_nonop_delay_msecs(
unsigned long tmp;
int ret;
 
-   ret = strict_strtoul(page, 0, tmp);
+   ret = kstrtoul(page, 0, tmp);
if (ret  0) {
pr_err(Unable to extract nonop_delay_msecs\n);
-   return -EINVAL;
+   return ret;
}
if (tmp  ALUA_MAX_NONOP_DELAY_MSECS) {
pr_err(Passed nonop_delay_msecs: %lu, exceeds
@@ -1825,10 +1825,10 @@ ssize_t core_alua_store_trans_delay_msecs(
unsigned long tmp;
int ret;
 
-   ret = strict_strtoul(page, 0, tmp);
+   ret = kstrtoul(page, 0, tmp);
if (ret  0) {
pr_err(Unable to 

[Bug 59601] commit 97dec564fd4948e0e560869c80b76e166ca2a83e breaks communication with XYRATEX disk shelves

2013-07-19 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=59601

--- Comment #11 from Saurav Kashyap saurav.kash...@qlogic.com ---
Hi Jack,
This patch http://marc.info/?l=linux-scsim=137365649318663w=2 is submitted to
upstream. Please close this BZ.

thanks,
~Saurav

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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 2/2] qla2xxx: Properly set the tagging for commands.

2013-07-19 Thread Saurav Kashyap
This should go to stable also, added in to list. This fixes a BZ 
https://bugzilla.kernel.org/show_bug.cgi?id=59601

Thanks,
~Saurav


-Original Message-
From: Saurav Kashyap [mailto:saurav.kash...@qlogic.com] 
Sent: Saturday, July 13, 2013 12:18 AM
To: jbottom...@parallels.com
Cc: Giridhar Malavali; Saurav Kashyap; Andrew Vasquez; linux-scsi
Subject: [PATCH 2/2] qla2xxx: Properly set the tagging for commands.

Reported-by: Jack Hill jackh...@jackhill.us
Signed-off-by: Saurav Kashyap saurav.kash...@qlogic.com
Signed-off-by: Giridhar Malavali giridhar.malav...@qlogic.com
---
 drivers/scsi/qla2xxx/qla_iocb.c |   11 +--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c 
index 42ef481..ef0a548 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -419,6 +419,8 @@ qla2x00_start_scsi(srb_t *sp)
__constant_cpu_to_le16(CF_SIMPLE_TAG);
break;
}
+   } else {
+   cmd_pkt-control_flags = __constant_cpu_to_le16(CF_SIMPLE_TAG);
}
 
/* Load SCSI command packet. */
@@ -1307,11 +1309,11 @@ qla24xx_build_scsi_crc_2_iocbs(srb_t *sp, struct 
cmd_type_crc_2 *cmd_pkt,
fcp_cmnd-task_attribute = TSK_ORDERED;
break;
default:
-   fcp_cmnd-task_attribute = 0;
+   fcp_cmnd-task_attribute = TSK_SIMPLE;
break;
}
} else {
-   fcp_cmnd-task_attribute = 0;
+   fcp_cmnd-task_attribute = TSK_SIMPLE;
}
 
cmd_pkt-fcp_rsp_dseg_len = 0; /* Let response come in status iocb */ 
@@ -1525,7 +1527,12 @@ qla24xx_start_scsi(srb_t *sp)
case ORDERED_QUEUE_TAG:
cmd_pkt-task = TSK_ORDERED;
break;
+   default:
+   cmd_pkt-task = TSK_SIMPLE;
+   break;
}
+   } else {
+   cmd_pkt-task = TSK_SIMPLE;
}
 
/* Load SCSI command packet. */
--
1.7.7

attachment: winmail.dat

RE: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU

2013-07-19 Thread Seungwon Jeon
On Thu, July 18, 2013, Sujit Reddy Thumma wrote:
  + * ufshcd_wait_for_register - wait for register value to change
  + * @hba - per-adapter interface
  + * @reg - mmio register offset
  + * @mask - mask to apply to read register value
  + * @val - wait condition
  + * @interval_us - polling interval in microsecs
  + * @timeout_ms - timeout in millisecs
  + *
  + * Returns final register value after iteration
  + */
  +static u32 ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 
  mask,
  +u32 val, unsigned long interval_us, unsigned long 
  timeout_ms)
  I feel like this function's role is to wait for clearing register 
  (specially, doorbells).
  If you really don't intend to apply other all register, I think it would 
  better to change the
  function name.
  ex ufshcd_wait_for_clear_doorbell or ufshcd_wait_for_clear_reg?
 
  Although, this API is introduced in this patch and used only for
  clearing the doorbell, I have intentionally made it generic to avoid
  duplication of wait condition code in future (not just clearing but
  also for setting a bit). For example, when we write to HCE.ENABLE we
  wait for it turn to 1.
 
 
  And if you like it, it could be more simple like below
 
  static bool ufshcd_wait_for_clear_reg(struct ufs_hba *hba, u32 reg, u32 
  mask,
 unsigned long interval_us,
 unsigned int timeout_ms)
  {
unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
 
  Jiffies on 100Hz systems have minimum granularity of 10ms. So we really
  wait for more 10ms even though the timeout_ms  10ms.
  Yes. Real timeout depends on system.
  But normally actual wait will be maintained until 'ufshcd_readl' is done.
  Timeout is valid for failure case.  If we don't need to apply a strict 
  timeout value, it's not bad.
 
 Hmm.. make sense. Will take care in the next patchset.
 
 
 
/* wakeup within 50us of expiry */
const unsigned int expiry = 50;
 
while (ufshcd_readl(hba, reg)  mask) {
usleep_range(interval_us, interval_us + expiry);
if (time_after(jiffies, timeout)) {
if (ufshcd_readl(hba, reg)  mask)
return false;
 
  I really want the caller to decide on what to do after the timeout.
  This helps in error handling cases where we try to clear off the entire
  door-bell register but only a few of the bits are cleared after timeout.
  I don't know what we can do with the report of the partial success on 
  clearing bit.
  It's just failure case. Isn't enough with false/true?
 
 The point is, if a bit can't be cleared it can be regarded as a
 permanent failure (only for that request), otherwise, it can be
 retried with the same tag value.
 
 
 
else
return true;
}
}
 
return true;
  }
  +{
  +u32 tmp;
  +ktime_t start;
  +unsigned long diff;
  +
  +tmp = ufshcd_readl(hba, reg);
  +
  +if ((val  mask) != val) {
  +dev_err(hba-dev, %s: Invalid wait condition 0x%x\n,
  +__func__, val);
  +goto out;
  +}
  +
  +start = ktime_get();
  +while ((tmp  mask) != val) {
  +/* wakeup within 50us of expiry */
  +usleep_range(interval_us, interval_us + 50);
  +tmp = ufshcd_readl(hba, reg);
  +diff = ktime_to_ms(ktime_sub(ktime_get(), start));
  +if (diff  timeout_ms) {
  +tmp = ufshcd_readl(hba, reg);
  +break;
  +}
  +}
  +out:
  +return tmp;
  +}
  +
 ..
  +static int
  +ufshcd_clear_cmd(struct ufs_hba *hba, int tag)
  +{
  +int err = 0;
  +unsigned long flags;
  +u32 reg;
  +u32 mask = 1  tag;
  +
  +/* clear outstanding transaction before retry */
  +spin_lock_irqsave(hba-host-host_lock, flags);
  +ufshcd_utrl_clear(hba, tag);
  +spin_unlock_irqrestore(hba-host-host_lock, flags);
  +
  +/*
  + * wait for for h/w to clear corresponding bit in door-bell.
  + * max. wait is 1 sec.
  + */
  +reg = ufshcd_wait_for_register(hba,
  +REG_UTP_TRANSFER_REQ_DOOR_BELL,
  +mask, 0, 1000, 1000);
  4th argument should be (~mask) instead of '0', right?
 
  True, but not really for this implementation. It breaks the check in
  in wait_for_register -
  if ((val  mask) != val)
   dev_err(...);
  Hmm, it seems complicated to use.
  Ok. Is right the following about val as 4th argument?
  - clear: val  should be '0' regardless corresponding bit.
  - set: val should be same with mask.
  If the related comment is 

RE: [PATCH V3 2/2] scsi: ufs: Set fDeviceInit flag to initiate device initialization

2013-07-19 Thread Seungwon Jeon
On Thursday, July 18, 2013, Dolev Raviv wrote:
  Hi,
 
  Sorry for the late response. I had a few days off.
 
  On Thu, July 11, 2013, Dolev Raviv wrote:
   On Tuesday, July 09, 2013, Sujit Reddy Thumma wrote:
   From: Dolev Raviv dra...@codeaurora.org
   Allow UFS device to complete its initialization and accept
   SCSI commands by setting fDeviceInit flag. The device may take
   time for this operation and hence the host should poll until
   fDeviceInit flag is toggled to zero. This step is mandated by
   UFS device specification for device initialization completion.
   Signed-off-by: Dolev Raviv dra...@codeaurora.org
   Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
   ---
drivers/scsi/ufs/ufs.h|   88 +-
drivers/scsi/ufs/ufshcd.c |  292
   -
drivers/scsi/ufs/ufshcd.h |   14 ++
drivers/scsi/ufs/ufshci.h |2 +-
4 files changed, 390 insertions(+), 6 deletions(-)
   diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
   index 14c0a4e..db5bde4 100644
   --- a/drivers/scsi/ufs/ufs.h
   +++ b/drivers/scsi/ufs/ufs.h
   @@ -43,6 +43,8 @@
#define GENERAL_UPIU_REQUEST_SIZE 32
#define UPIU_HEADER_DATA_SEGMENT_MAX_SIZE  ((ALIGNED_UPIU_SIZE) - \
   (GENERAL_UPIU_REQUEST_SIZE))
   +#define QUERY_OSF_SIZE ((GENERAL_UPIU_REQUEST_SIZE) - \
   +   (sizeof(struct 
   utp_upiu_header)))
#define UPIU_HEADER_DWORD(byte3, byte2, byte1, byte0)\
   cpu_to_be32((byte3  24) | (byte2  16) |\
   @@ -68,7 +70,7 @@ enum {
   UPIU_TRANSACTION_COMMAND= 0x01,
   UPIU_TRANSACTION_DATA_OUT   = 0x02,
   UPIU_TRANSACTION_TASK_REQ   = 0x04,
   -   UPIU_TRANSACTION_QUERY_REQ  = 0x26,
   +   UPIU_TRANSACTION_QUERY_REQ  = 0x16,
};
/* UTP UPIU Transaction Codes Target to Initiator */
   @@ -97,8 +99,19 @@ enum {
   UPIU_TASK_ATTR_ACA  = 0x03,
};
   -/* UTP QUERY Transaction Specific Fields OpCode */
   +/* UPIU Query request function */
enum {
   +   UPIU_QUERY_FUNC_STANDARD_READ_REQUEST = 0x01,
   +   UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST = 0x81,
   +};
   +
   +/* Flag idn for Query Requests*/
   +enum flag_idn {
   +   QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
   +};
   +
   +/* UTP QUERY Transaction Specific Fields OpCode */
   +enum query_opcode {
   UPIU_QUERY_OPCODE_NOP   = 0x0,
   UPIU_QUERY_OPCODE_READ_DESC = 0x1,
   UPIU_QUERY_OPCODE_WRITE_DESC= 0x2,
   @@ -110,6 +123,21 @@ enum {
   UPIU_QUERY_OPCODE_TOGGLE_FLAG   = 0x8,
};
   +/* Query response result code */
   +enum {
   +   QUERY_RESULT_SUCCESS= 0x00,
   +   QUERY_RESULT_NOT_READABLE   = 0xF6,
   +   QUERY_RESULT_NOT_WRITEABLE  = 0xF7,
   +   QUERY_RESULT_ALREADY_WRITTEN= 0xF8,
   +   QUERY_RESULT_INVALID_LENGTH = 0xF9,
   +   QUERY_RESULT_INVALID_VALUE  = 0xFA,
   +   QUERY_RESULT_INVALID_SELECTOR   = 0xFB,
   +   QUERY_RESULT_INVALID_INDEX  = 0xFC,
   +   QUERY_RESULT_INVALID_IDN= 0xFD,
   +   QUERY_RESULT_INVALID_OPCODE = 0xFE,
   +   QUERY_RESULT_GENERAL_FAILURE= 0xFF,
   +};
   +
/* UTP Transfer Request Command Type (CT) */
enum {
   UPIU_COMMAND_SET_TYPE_SCSI  = 0x0,
   @@ -127,6 +155,7 @@ enum {
   MASK_SCSI_STATUS= 0xFF,
   MASK_TASK_RESPONSE  = 0xFF00,
   MASK_RSP_UPIU_RESULT= 0x,
   +   MASK_QUERY_DATA_SEG_LEN = 0x,
};
/* Task management service response */
   @@ -160,13 +189,40 @@ struct utp_upiu_cmd {
};
/**
   + * struct utp_upiu_query - upiu request buffer structure for
   + * query request.
   + * @opcode: command to perform B-0
   + * @idn: a value that indicates the particular type of data B-1 + *
  @index: Index to further identify data B-2
   + * @selector: Index to further identify data B-3
   + * @reserved_osf: spec reserved field B-4,5
   + * @length: number of descriptor bytes to read/write B-6,7
   + * @value: Attribute value to be written DW-5
   + * @reserved: spec reserved DW-6,7
   + */
   +struct utp_upiu_query {
   +   u8 opcode;
   +   u8 idn;
   +   u8 index;
   +   u8 selector;
   +   u16 reserved_osf;
   +   u16 length;
   +   u32 value;
   +   u32 reserved[2];
   +};
   +
   +/**
 * struct utp_upiu_req - general upiu request structure
 * @header:UPIU header structure DW-0 to DW-2
 * @sc: fields structure for scsi command DW-3 to DW-7
   + * @qr: fields structure for query request DW-3 to DW-7
 */
struct utp_upiu_req {
   struct utp_upiu_header header;
   -   struct utp_upiu_cmd sc;
   +   union {
   +   struct utp_upiu_cmd sc;
   +  

RE: [PATCH V3 1/2] scsi: ufs: Add support for host assisted background operations

2013-07-19 Thread Seungwon Jeon
On Thu, July 18, 2013, Sujit Reddy Thumma wrote:
   I'm not sure that BKOPS with runtime-pm associates.
   Do you think it's helpful for power management?
   How about hibernation scheme for runtime-pm?
   I'm testing and I can introduce soon.
  
  Well, I am thinking on following approach when we introduce
  power management.
  
  ufshcd_runtime_suspend() {
if (bkops_status = NON_CRITICAL) { /* 0x1 */
ufshcd_enable_auto_bkops();
hibernate(); /* only the link and the device
should be able to cary out bkops */
} else {
hibernate(); /* Link and the device for more savings */
}
  }
  
  Let me know if this is okay.
  I still consider whether BKOPS is proper behavior with runtime-pm or not.
 
 The BKOPS is something that host allows the card to carry out
 when the host knows it is idle and not expecting back to back requests.
 Runtime PM idle is the only way to know whether the device is
 idle (unless we want to reinvent the wheel to detect the idleness of
 host and trigger bkops). There was a discussion on this in MMC mailing
 list as well, and folks have agreed to move idle time bkops to runtime
 PM (http://thread.gmane.org/gmane.linux.kernel.mmc/19444/)
It looks like different.
eMMC cannot execute BKOPS itself unlike UFS.
That's the way eMMC's host should trigger the BKOPS manually.

 
  How about this scenario? It seems more simple.
  If we concern a response latency of transfer requests, BKOPS can be 
disabled by default.
  And then BKOPS can be enabled whenever device requests in some exception.
  If you have any idea, let me know.
 
 Exceptions are raised only when the device is in critical need for
 bkops. Also the spec. recommends, host should ensure that the device
 doesn't go into such states.
 
 With your suggestion, when we disable bkops, the exception is raised and
 we enable bkops after which there is no way to disable it again?
Yes, it's difficult to find proper time. 
Maybe, BKOPS can be disabled when request comes up.

Thanks,
Seungwon Jeon
 
 --
 Regards,
 Sujit
 --
 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

--
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 V3 1/4] scsi: ufs: Fix broken task management command implementation

2013-07-19 Thread Seungwon Jeon
On Tue, July 09, 2013 Sujit Reddy Thumma wrote:
 Currently, sending Task Management (TM) command to the card might
 be broken in some scenarios as listed below:
 
 Problem: If there are more than 8 TM commands the implementation
  returns error to the caller.
 Fix: Wait for one of the slots to be emptied and send the command.
 
 Problem: Sometimes it is necessary for the caller to know the TM service
  response code to determine the task status.
 Fix: Propogate the service response to the caller.
 
 Problem: If the TM command times out no proper error recovery is
  implemented.
 Fix: Clear the command in the controller door-bell register, so that
  further commands for the same slot don't fail.
 
 Problem: While preparing the TM command descriptor, the task tag used
  should be unique across SCSI/NOP/QUERY/TM commands and not the
task tag of the command which the TM command is trying to manage.
 Fix: Use a unique task tag instead of task tag of SCSI command.
 
 Problem: Since the TM command involves H/W communication, abruptly ending
  the request on kill interrupt signal might cause h/w malfunction.
 Fix: Wait for hardware completion interrupt with TASK_UNINTERRUPTIBLE
  set.
 
 Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
 ---
  drivers/scsi/ufs/ufshcd.c |  177 
 ++---
  drivers/scsi/ufs/ufshcd.h |8 ++-
  2 files changed, 126 insertions(+), 59 deletions(-)
 
 diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
 index af7d01d..a176421 100644
 --- a/drivers/scsi/ufs/ufshcd.c
 +++ b/drivers/scsi/ufs/ufshcd.c
 @@ -53,6 +53,9 @@
  /* Query request timeout */
  #define QUERY_REQ_TIMEOUT 30 /* msec */
 
 +/* Task management command timeout */
 +#define TM_CMD_TIMEOUT   100 /* msecs */
 +
  /* Expose the flag value from utp_upiu_query.value */
  #define MASK_QUERY_UPIU_FLAG_LOC 0xFF
 
 @@ -190,13 +193,35 @@ ufshcd_get_tmr_ocs(struct utp_task_req_desc 
 *task_req_descp)
  /**
   * ufshcd_get_tm_free_slot - get a free slot for task management request
   * @hba: per adapter instance
 + * @free_slot: pointer to variable with available slot value
   *
 - * Returns maximum number of task management request slots in case of
 - * task management queue full or returns the free slot number
 + * Get a free tag and lock it until ufshcd_put_tm_slot() is called.
 + * Returns 0 if free slot is not available, else return 1 with tag value
 + * in @free_slot.
   */
 -static inline int ufshcd_get_tm_free_slot(struct ufs_hba *hba)
 +static bool ufshcd_get_tm_free_slot(struct ufs_hba *hba, int *free_slot)
 +{
 + int tag;
 + bool ret = false;
 +
 + if (!free_slot)
 + goto out;
 +
 + do {
 + tag = find_first_zero_bit(hba-tm_slots_in_use, hba-nutmrs);
 + if (tag = hba-nutmrs)
 + goto out;
 + } while (test_and_set_bit_lock(tag, hba-tm_slots_in_use));
 +
 + *free_slot = tag;
 + ret = true;
 +out:
 + return ret;
 +}
 +
 +static inline void ufshcd_put_tm_slot(struct ufs_hba *hba, int slot)
  {
 - return find_first_zero_bit(hba-outstanding_tasks, hba-nutmrs);
 + clear_bit_unlock(slot, hba-tm_slots_in_use);
  }
 
  /**
 @@ -1778,10 +1803,11 @@ static void ufshcd_slave_destroy(struct scsi_device 
 *sdev)
   * ufshcd_task_req_compl - handle task management request completion
   * @hba: per adapter instance
   * @index: index of the completed request
 + * @resp: task management service response
   *
 - * Returns SUCCESS/FAILED
 + * Returns non-zero value on error, zero on success
   */
 -static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index)
 +static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index, u8 *resp)
  {
   struct utp_task_req_desc *task_req_descp;
   struct utp_upiu_task_rsp *task_rsp_upiup;
 @@ -1802,19 +1828,15 @@ static int ufshcd_task_req_compl(struct ufs_hba *hba, 
 u32 index)
   task_req_descp[index].task_rsp_upiu;
   task_result = be32_to_cpu(task_rsp_upiup-header.dword_1);
   task_result = ((task_result  MASK_TASK_RESPONSE)  8);
 -
 - if (task_result != UPIU_TASK_MANAGEMENT_FUNC_COMPL 
 - task_result != UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED)
 - task_result = FAILED;
 - else
 - task_result = SUCCESS;
 + if (resp)
 + *resp = (u8)task_result;
   } else {
 - task_result = FAILED;
 - dev_err(hba-dev,
 - trc: Invalid ocs = %x\n, ocs_value);
 + dev_err(hba-dev, %s: failed, ocs = 0x%x\n,
 + __func__, ocs_value);
   }
   spin_unlock_irqrestore(hba-host-host_lock, flags);
 - return task_result;
 +
 + return ocs_value;
  }
 
  /**
 @@ -2298,7 +2320,7 @@ static void ufshcd_tmc_handler(struct ufs_hba 

RE: [PATCH V3 3/4] scsi: ufs: Fix device and host reset methods

2013-07-19 Thread Seungwon Jeon
On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
 As of now SCSI initiated error handling is broken because,
 the reset APIs don't try to bring back the device initialized and
 ready for further transfers.
 
 In case of timeouts, the scsi error handler takes care of handling aborts
 and resets. Improve the error handling in such scenario by resetting the
 device and host and re-initializing them in proper manner.
 
 Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
 ---
  drivers/scsi/ufs/ufshcd.c |  467 
 +++--
  drivers/scsi/ufs/ufshcd.h |2 +
  2 files changed, 411 insertions(+), 58 deletions(-)
 
 diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
 index 51ce096..b4c9910 100644
 --- a/drivers/scsi/ufs/ufshcd.c
 +++ b/drivers/scsi/ufs/ufshcd.c
 @@ -69,9 +69,15 @@ enum {
 
  /* UFSHCD states */
  enum {
 - UFSHCD_STATE_OPERATIONAL,
   UFSHCD_STATE_RESET,
   UFSHCD_STATE_ERROR,
 + UFSHCD_STATE_OPERATIONAL,
 +};
 +
 +/* UFSHCD error handling flags */
 +enum {
 + UFSHCD_EH_HOST_RESET_PENDING = (1  0),
 + UFSHCD_EH_DEVICE_RESET_PENDING = (1  1),
  };
 
  /* Interrupt configuration options */
 @@ -87,6 +93,22 @@ enum {
   INT_AGGR_CONFIG,
  };
 
 +#define ufshcd_set_device_reset_pending(h) \
 + (h-eh_flags |= UFSHCD_EH_DEVICE_RESET_PENDING)
 +#define ufshcd_set_host_reset_pending(h) \
 + (h-eh_flags |= UFSHCD_EH_HOST_RESET_PENDING)
 +#define ufshcd_device_reset_pending(h) \
 + (h-eh_flags  UFSHCD_EH_DEVICE_RESET_PENDING)
 +#define ufshcd_host_reset_pending(h) \
 + (h-eh_flags  UFSHCD_EH_HOST_RESET_PENDING)
 +#define ufshcd_clear_device_reset_pending(h) \
 + (h-eh_flags = ~UFSHCD_EH_DEVICE_RESET_PENDING)
 +#define ufshcd_clear_host_reset_pending(h) \
 + (h-eh_flags = ~UFSHCD_EH_HOST_RESET_PENDING)
 +
 +static void ufshcd_tmc_handler(struct ufs_hba *hba);
 +static void ufshcd_async_scan(void *data, async_cookie_t cookie);
 +
  /*
   * ufshcd_wait_for_register - wait for register value to change
   * @hba - per-adapter interface
 @@ -851,9 +873,22 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, 
 struct scsi_cmnd *cmd)
 
   tag = cmd-request-tag;
 
 - if (hba-ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
 + switch (hba-ufshcd_state) {
Lock is no needed for ufshcd_state?

 + case UFSHCD_STATE_OPERATIONAL:
 + break;
 + case UFSHCD_STATE_RESET:
   err = SCSI_MLQUEUE_HOST_BUSY;
   goto out;
 + case UFSHCD_STATE_ERROR:
 + set_host_byte(cmd, DID_ERROR);
 + cmd-scsi_done(cmd);
 + goto out;
 + default:
 + dev_WARN_ONCE(hba-dev, 1, %s: invalid state %d\n,
 + __func__, hba-ufshcd_state);
 + set_host_byte(cmd, DID_BAD_TARGET);
 + cmd-scsi_done(cmd);
 + goto out;
   }
 
   /* acquire the tag to make sure device cmds don't use it */
 @@ -1573,8 +1608,6 @@ static int ufshcd_make_hba_operational(struct ufs_hba 
 *hba)
   if (hba-ufshcd_state == UFSHCD_STATE_RESET)
   scsi_unblock_requests(hba-host);
 
 - hba-ufshcd_state = UFSHCD_STATE_OPERATIONAL;
 -
  out:
   return err;
  }
 @@ -2273,6 +2306,106 @@ out:
  }
 
  /**
 + * ufshcd_utrl_is_rsr_enabled - check if run-stop register is enabled
 + * @hba: per-adapter instance
 + */
 +static bool ufshcd_utrl_is_rsr_enabled(struct ufs_hba *hba)
 +{
 + return ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_LIST_RUN_STOP)  0x1;
 +}
 +
 +/**
 + * ufshcd_utmrl_is_rsr_enabled - check if run-stop register is enabled
 + * @hba: per-adapter instance
 + */
 +static bool ufshcd_utmrl_is_rsr_enabled(struct ufs_hba *hba)
 +{
 + return ufshcd_readl(hba, REG_UTP_TASK_REQ_LIST_RUN_STOP)  0x1;
 +}
 +
 +/**
 + * ufshcd_complete_pending_tasks - complete outstanding tasks
 + * @hba: per adapter instance
 + *
 + * Abort in-progress task management commands and wakeup
 + * waiting threads.
 + *
 + * Returns non-zero error value when failed to clear all the commands.
 + */
 +static int ufshcd_complete_pending_tasks(struct ufs_hba *hba)
 +{
 + u32 reg;
 + int err = 0;
 + unsigned long flags;
 +
 + if (!hba-outstanding_tasks)
 + goto out;
 +
 + /* Clear UTMRL only when run-stop is enabled */
 + if (ufshcd_utmrl_is_rsr_enabled(hba))
 + ufshcd_writel(hba, ~hba-outstanding_tasks,
 + REG_UTP_TASK_REQ_LIST_CLEAR);
 +
 + /* poll for max. 1 sec to clear door bell register by h/w */
 + reg = ufshcd_wait_for_register(hba,
 + REG_UTP_TASK_REQ_DOOR_BELL,
 + hba-outstanding_tasks, 0, 1000, 1000);
 + if (reg  hba-outstanding_tasks)
 + err = -ETIMEDOUT;
 +
 + spin_lock_irqsave(hba-host-host_lock, flags);
 + /* complete commands that were cleared out */
 + ufshcd_tmc_handler(hba);
 + spin_unlock_irqrestore(hba-host-host_lock, flags);
 +out:
 

RE: [PATCH V3 2/4] scsi: ufs: Fix hardware race conditions while aborting a command

2013-07-19 Thread Seungwon Jeon
On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
 There is a possible race condition in the hardware when the abort
 command is issued to terminate the ongoing SCSI command as described
 below:
 
 - A bit in the door-bell register is set in the controller for a
   new SCSI command.
 - In some rare situations, before controller get a chance to issue
   the command to the device, the software issued an abort command.
It's interesting.
I wonder when we can meet this situation.
Is it possible if SCSI mid layer should send abort command as soon as the 
transfer command is issued?
AFAIK abort command is followed if one command has timed out.
That means command have been already issued and no response?
If you had some problem, could you share?

 - If the device recieves abort command first then it returns success
receives

   because the command itself is not present.
 - Now if the controller commits the command to device it will be
   processed.
 - Software thinks that command is aborted and proceed while still
   the device is processing it.
 - The software, controller and device may go out of sync because of
   this race condition.
 
 To avoid this, query task presence in the device before sending abort
 task command so that after the abort operation, the command is guaranteed
 to be non-existent in both controller and the device.
 
 Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
 ---
  drivers/scsi/ufs/ufshcd.c |   70 +++-
  1 files changed, 55 insertions(+), 15 deletions(-)
 
 diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
 index a176421..51ce096 100644
 --- a/drivers/scsi/ufs/ufshcd.c
 +++ b/drivers/scsi/ufs/ufshcd.c
 @@ -2550,6 +2550,12 @@ static int ufshcd_host_reset(struct scsi_cmnd *cmd)
   * ufshcd_abort - abort a specific command
   * @cmd: SCSI command pointer
   *
 + * Abort the pending command in device by sending UFS_ABORT_TASK task 
 management
 + * command, and in host controller by clearing the door-bell register. There 
 can
 + * be race between controller sending the command to the device while abort 
 is
 + * issued. To avoid that, first issue UFS_QUERY_TASK to check if the command 
 is
 + * really issued and then try to abort it.
 + *
   * Returns SUCCESS/FAILED
   */
  static int ufshcd_abort(struct scsi_cmnd *cmd)
 @@ -2558,7 +2564,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
   struct ufs_hba *hba;
   unsigned long flags;
   unsigned int tag;
 - int err;
 + int err = 0;
 + int poll_cnt;
   u8 resp;
   struct ufshcd_lrb *lrbp;
 
 @@ -2566,33 +2573,59 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
   hba = shost_priv(host);
   tag = cmd-request-tag;
 
 - spin_lock_irqsave(host-host_lock, flags);
 + /* If command is already aborted/completed, return SUCCESS */
 + if (!(test_bit(tag, hba-outstanding_reqs)))
 + goto out;
 
 - /* check if command is still pending */
 - if (!(test_bit(tag, hba-outstanding_reqs))) {
 - err = FAILED;
 - spin_unlock_irqrestore(host-host_lock, flags);
 + lrbp = hba-lrb[tag];
 + for (poll_cnt = 100; poll_cnt; poll_cnt--) {
 + err = ufshcd_issue_tm_cmd(hba, lrbp-lun, lrbp-task_tag,
 + UFS_QUERY_TASK, resp);
 + if (!err  resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
 + /* cmd pending in the device */
 + break;
 + } else if (!err  resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
 + u32 reg;
 +
 + /*
 +  * cmd not pending in the device, check if it is
 +  * in transition.
 +  */
 + reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 + if (reg  (1  tag)) {
 + /* sleep for max. 2ms to stabilize */
 + usleep_range(1000, 2000);
 + continue;
 + }
 + /* command completed already */
 + goto out;
 + } else {
 + if (!err)
 + err = resp; /* service response error */
 + goto out;
 + }
 + }
 +
 + if (!poll_cnt) {
 + err = -EBUSY;
   goto out;
   }
 - spin_unlock_irqrestore(host-host_lock, flags);
 
 - lrbp = hba-lrb[tag];
   err = ufshcd_issue_tm_cmd(hba, lrbp-lun, lrbp-task_tag,
   UFS_ABORT_TASK, resp);
   if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
 - err = FAILED;
 + if (!err)
 + err = resp; /* service response error */
   goto out;
 - } else {
 - err = SUCCESS;
   }
 
 + err = ufshcd_clear_cmd(hba, tag);
 + if (err)
 + goto out;
 +
   

RE: [PATCH V3 4/4] scsi: ufs: Improve UFS fatal error handling

2013-07-19 Thread Seungwon Jeon
On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
 Error handling in UFS driver is broken and resets the host controller
 for fatal errors without re-initialization. Correct the fatal error
 handling sequence according to UFS Host Controller Interface (HCI)
 v1.1 specification.
 
 o Upon determining fatal error condition the host controller may hang
   forever until a reset is applied, so just retrying the command doesn't
   work without a reset. So, the reset is applied in the driver context
   in a separate work and SCSI mid-layer isn't informed until reset is
   applied.
 
 o Processed requests which are completed without error are reported to
   SCSI layer as successful and any pending commands that are not started
   yet or are not cause of the error are re-queued into scsi midlayer queue.
   For the command that caused error, host controller or device is reset
   and DID_ERROR is returned for command retry after applying reset.
 
 o SCSI is informed about the expected Unit-Attentioni exception from the
Attention'i',  typo.

   device for the immediate command after a reset so that the SCSI layer
   take necessary steps to establish communication with the device.
 
 Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
 ---
  drivers/scsi/ufs/ufshcd.c |  349 +++-
  drivers/scsi/ufs/ufshcd.h |2 +
  drivers/scsi/ufs/ufshci.h |   19 ++-
  3 files changed, 295 insertions(+), 75 deletions(-)
 
 diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
 index b4c9910..2a3874f 100644
 --- a/drivers/scsi/ufs/ufshcd.c
 +++ b/drivers/scsi/ufs/ufshcd.c
 @@ -80,6 +80,14 @@ enum {
   UFSHCD_EH_DEVICE_RESET_PENDING = (1  1),
  };
 
 +/* UFSHCD UIC layer error flags */
 +enum {
 + UFSHCD_UIC_DL_PA_INIT_ERROR = (1  0), /* Data link layer error */
 + UFSHCD_UIC_NL_ERROR = (1  1), /* Network layer error */
 + UFSHCD_UIC_TL_ERROR = (1  2), /* Transport Layer error */
 + UFSHCD_UIC_DME_ERROR = (1  3), /* DME error */
 +};
 +
  /* Interrupt configuration options */
  enum {
   UFSHCD_INT_DISABLE,
 @@ -108,6 +116,7 @@ enum {
 
  static void ufshcd_tmc_handler(struct ufs_hba *hba);
  static void ufshcd_async_scan(void *data, async_cookie_t cookie);
 +static int ufshcd_reset_and_restore(struct ufs_hba *hba);
 
  /*
   * ufshcd_wait_for_register - wait for register value to change
 @@ -1605,9 +1614,6 @@ static int ufshcd_make_hba_operational(struct ufs_hba 
 *hba)
   goto out;
   }
 
 - if (hba-ufshcd_state == UFSHCD_STATE_RESET)
 - scsi_unblock_requests(hba-host);
 -
  out:
   return err;
  }
 @@ -1733,66 +1739,6 @@ static int ufshcd_validate_dev_connection(struct 
 ufs_hba *hba)
  }
 
  /**
 - * ufshcd_do_reset - reset the host controller
 - * @hba: per adapter instance
 - *
 - * Returns SUCCESS/FAILED
 - */
 -static int ufshcd_do_reset(struct ufs_hba *hba)
 -{
 - struct ufshcd_lrb *lrbp;
 - unsigned long flags;
 - int tag;
 -
 - /* block commands from midlayer */
 - scsi_block_requests(hba-host);
 -
 - spin_lock_irqsave(hba-host-host_lock, flags);
 - hba-ufshcd_state = UFSHCD_STATE_RESET;
 -
 - /* send controller to reset state */
 - ufshcd_hba_stop(hba);
 - spin_unlock_irqrestore(hba-host-host_lock, flags);
 -
 - /* abort outstanding commands */
 - for (tag = 0; tag  hba-nutrs; tag++) {
 - if (test_bit(tag, hba-outstanding_reqs)) {
 - lrbp = hba-lrb[tag];
 - if (lrbp-cmd) {
 - scsi_dma_unmap(lrbp-cmd);
 - lrbp-cmd-result = DID_RESET  16;
 - lrbp-cmd-scsi_done(lrbp-cmd);
 - lrbp-cmd = NULL;
 - clear_bit_unlock(tag, hba-lrb_in_use);
 - }
 - }
 - }
 -
 - /* complete device management command */
 - if (hba-dev_cmd.complete)
 - complete(hba-dev_cmd.complete);
 -
 - /* clear outstanding request/task bit maps */
 - hba-outstanding_reqs = 0;
 - hba-outstanding_tasks = 0;
 -
 - /* Host controller enable */
 - if (ufshcd_hba_enable(hba)) {
 - dev_err(hba-dev,
 - Reset: Controller initialization failed\n);
 - return FAILED;
 - }
 -
 - if (ufshcd_link_startup(hba)) {
 - dev_err(hba-dev,
 - Reset: Link start-up failed\n);
 - return FAILED;
 - }
 -
 - return SUCCESS;
 -}
 -
 -/**
   * ufshcd_slave_alloc - handle initial SCSI device configurations
   * @sdev: pointer to SCSI device
   *
 @@ -1809,6 +1755,9 @@ static int ufshcd_slave_alloc(struct scsi_device *sdev)
   sdev-use_10_for_ms = 1;
   scsi_set_tag_type(sdev, MSG_SIMPLE_TAG);
 
 + /* allow SCSI layer to restart the device in case of errors */
 + sdev-allow_restart = 1;
 +
   /*
* Inform SCSI Midlayer that the LUN queue depth 

Re: [Ksummit-2013-discuss] [ATTEND] scsi-mq prototype discussion

2013-07-19 Thread Ric Wheeler

On 07/17/2013 12:52 AM, James Bottomley wrote:

On Tue, 2013-07-16 at 15:15 -0600, Jens Axboe wrote:

On Tue, Jul 16 2013, Nicholas A. Bellinger wrote:

On Sat, 2013-07-13 at 06:53 +, James Bottomley wrote:

On Fri, 2013-07-12 at 12:52 +0200, Hannes Reinecke wrote:

On 07/12/2013 03:33 AM, Nicholas A. Bellinger wrote:

On Thu, 2013-07-11 at 18:02 -0700, Greg KH wrote:

On Thu, Jul 11, 2013 at 05:23:32PM -0700, Nicholas A. Bellinger wrote:

Drilling down the work items ahead of a real mainline push is high on
priority list for discussion.

The parties to be included in such a discussion are:

   - Jens Axboe (blk-mq author)
   - James Bottomley (scsi maintainer)
   - Christoph Hellwig (scsi)
   - Martin Petersen (scsi)
   - Tejun Heo (block + libata)
   - Hannes Reinecke (scsi error recovery)
   - Kent Overstreet (block, per-cpu ida)
   - Stephen Cameron (scsi-over-pcie driver)
   - Andrew Vasquez (qla2xxx LLD)
   - James Smart (lpfc LLD)

Isn't this something that should have been discussed at the storage
mini-summit a few months ago?

The scsi-mq prototype, along with blk-mq (in it's current form) did not
exist a few short months ago.  ;)


  It seems very specific to one subsystem to be a kernel summit topic,
don't you think?

It's no more subsystem specific than half of the other proposals so far,
and given it's reach across multiple subsystems (block, scsi, target),
and the amount of off-list interest on the topic, I think it would make
a good candidate for discussion.


And it'll open up new approaches which previously were dismissed,
like re-implementing multipathing on top of scsi-mq, giving us the
single scsi device like other UNIX systems.

Also I do think there's quite some synergy to be had, as with blk-mq
we could nail each queue to a processor, which would eliminate the
need for locking.
Which could be useful for other subsystems, too.

Lets start with discussing this on the list, please, and then see where
we go from there ...


Yes, the discussion is beginning to make it's way to the list.  I've
mostly been waiting for blk-mq to get a wider review before taking the
early scsi-mq prototype driver to a larger public audience.

Primarily, I'm now reaching out to the people most effected by existing
scsi_request_fn() based performance limitations.  Most of them have
abandoned existing scsi_request_fn() based logic in favor of raw block
make_request() based drivers, and are now estimating the amount of
effort to move to an scsi-mq based approach.

Regardless, as the prototype progresses over the next months, having a
face-to-face discussion with the key parties in the room would be very
helpful given the large amount of effort involved to actually make this
type of generational shift in SCSI actually happen.

There's a certain amount of overlap with the aio/O_DIRECT work as well.
But if it's not a general session, could always be a BOF or something.

I'll second the argument that most technical topics probably DO belong
in a topic related workshop. But that leaves us with basically only
process related topics at KS, I don't think it hurts to have a bit of
tech meat on the bone too. At least I personally miss that part of KS
from years gone by.

Heh well, given that most of the block mq discussions at LSF have been
you saying you really should get around to cleaning up and posting the
code, you'll understand my wanting to see that happen first ...

I suppose we could try to run a storage workshop within KS, but I think
most of the mini summit slots have already gone.  There's also plumbers
if all slots are gone (I would say that, being biased and on the
programme committee) Ric is running the storage and Filesystems MC

http://www.linuxplumbersconf.org/2013/ocw/events/LPC2013/tracks/159

James



And we still are looking for suggested topics - it would be great to have the 
multi-queue work at plumbers.


You can post a proposal for it (or other topics) here:

http://www.linuxplumbersconf.org/2013/ocw/events/LPC2013/proposals

Ric


--
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 1/2] qla2xxx: Fix sparse warnings in qlafx00_fxdisc_iocb function.

2013-07-19 Thread James Bottomley
On Fri, 2013-07-12 at 14:47 -0400, Saurav Kashyap wrote:
 Signed-off-by: Saurav Kashyap saurav.kash...@qlogic.com
 Signed-off-by: Giridhar Malavali giridhar.malav...@qlogic.com

This does not fix a bug ... therefore it's not really -rc fixes
material.  In early -rc this rule can be relaxed a bit ... but what's
the reason to get this through the -rc tree?

James

--
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 RESEND 0/1] AHCI: Optimize interrupt processing

2013-07-19 Thread James Bottomley
On Thu, 2013-07-18 at 23:34 -0700, Nicholas A. Bellinger wrote:
 diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
 index 0101af5..191bc15 100644
 --- a/drivers/ata/libata-scsi.c
 +++ b/drivers/ata/libata-scsi.c
 @@ -1144,7 +1144,11 @@ static int ata_scsi_dev_config(struct scsi_device 
 *sdev,
 sector_size=%u  PAGE_SIZE, PIO may malfunction\n,
 sdev-sector_size);
  
 -   blk_queue_update_dma_alignment(q, sdev-sector_size - 1);
 +   if (!q-mq_ops) {
 +   blk_queue_update_dma_alignment(q, sdev-sector_size - 1);
 +   } else {
 +   printk(Skipping dma_alignment for libata w/ scsi-mq\n);
 +   }

Amazingly enough there is a reason for the dma alignment, and it wasn't
just to annoy you, so you can't blindly do this.

The email thread is probably lost in the mists of time, but if I
remember correctly the problem is that some ahci DMA controllers barf if
the sector they're doing DMA on crosses a page boundary.  Some are
annoying enough to actually cause silent data corruption.  You won't
find every ahci DMA controller doing this, so the change will work for
some, but it will be hard to identify those it won't work for until
people start losing data.

The correct fix, obviously, is to do the bio copy on the kernel path for
unaligned data.  It is OK to assume that REQ_TYPE_FS data is correctly
aligned (because of the block to page alignment).

James


--
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/7] qla4xxx: Updates for scsi misc branch

2013-07-19 Thread Mike Christie
On 07/08/2013 06:33 AM, adheer.chandravan...@qlogic.com wrote:
 From: Adheer Chandravanshi adheer.chandravan...@qlogic.com
 
 James,
 
 Please apply the following patches to the scsi tree at your earliest
 convenience.
 
 Adheer Chandravanshi (7):
   qla4xxx: Allow removal of failed session using logout.
   qla4xxx: Use discovery_parent_idx instead of discovery_parent_type
   qla4xxx: Set IPv6 traffic class if device type is IPv6.
   qla4xxx: discovery_parent_idx can be shown without any check.
   qla4xxx: Only BIOS boot target entries should be at index 0 and 1.
   qla4xxx: Export more firmware info in sysfs
   qla4xxx: Update driver version to 5.03.00-k11
 


Ignore my comment on that one patch. Patches look ok.

Reviewed-by: Mike Christie micha...@cs.wisc.edu

--
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 6/7] qla4xxx: Export more firmware info in sysfs

2013-07-19 Thread Mike Christie
On 07/17/2013 11:44 AM, Mike Christie wrote:
 On 07/08/2013 06:33 AM, adheer.chandravan...@qlogic.com wrote:
  static ssize_t
 @@ -181,8 +179,8 @@ qla4xxx_iscsi_version_show(struct device *dev, struct 
 device_attribute *attr,
 char *buf)
  {
  struct scsi_qla_host *ha = to_qla_host(class_to_shost(dev));
 -return snprintf(buf, PAGE_SIZE, %d.%02d\n, ha-iscsi_major,
 -ha-iscsi_minor);
 +return snprintf(buf, PAGE_SIZE, %d.%02d\n, ha-fw_info.iscsi_major,
 +ha-fw_info.iscsi_minor);
  }
 
 Is this the same as the iscsi max and min from the iscsi spec? If so, it
 should be a iscsi host setting.
 

Ignore this comment.

--
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 RESEND 0/1] AHCI: Optimize interrupt processing

2013-07-19 Thread Mike Christie
On 07/18/2013 06:23 PM, Nicholas A. Bellinger wrote:
 Just saw this while trying out iscsi with the scsi-mq stuff :)
  
 Took at stab at this a while back, but ended getting distracted on other
 items.  Do you have an initial conversion running yet..?

Not running well :) Have a patch but I am debugging it now. I messed up
something with the cmd_size stuff.
--
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


[Bug 59601] commit 97dec564fd4948e0e560869c80b76e166ca2a83e breaks communication with XYRATEX disk shelves

2013-07-19 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=59601

Jack Hill jackh...@jackhill.us changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |CODE_FIX

--- Comment #12 from Jack Hill jackh...@jackhill.us ---
Closing this bug since Saurav has submitted the patch upstream.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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 2/2] qla2xxx: Properly set the tagging for commands.

2013-07-19 Thread Greg KH
On Fri, Jul 19, 2013 at 09:16:51AM +, Saurav Kashyap wrote:
 This should go to stable also, added in to list. This fixes a BZ 
 https://bugzilla.kernel.org/show_bug.cgi?id=59601
 
 Thanks,
 ~Saurav
 
 
 -Original Message-
 From: Saurav Kashyap [mailto:saurav.kash...@qlogic.com]
 Sent: Saturday, July 13, 2013 12:18 AM
 To: jbottom...@parallels.com
 Cc: Giridhar Malavali; Saurav Kashyap; Andrew Vasquez; linux-scsi
 Subject: [PATCH 2/2] qla2xxx: Properly set the tagging for commands.
 
 Reported-by: Jack Hill jackh...@jackhill.us
 Signed-off-by: Saurav Kashyap saurav.kash...@qlogic.com
 Signed-off-by: Giridhar Malavali giridhar.malav...@qlogic.com
 ---
  drivers/scsi/qla2xxx/qla_iocb.c |   11 +--
  1 files changed, 9 insertions(+), 2 deletions(-)

formletter

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

/formletter
--
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 V3 1/2] scsi: ufs: Add support for host assisted background operations

2013-07-19 Thread Sujit Reddy Thumma



I'm not sure that BKOPS with runtime-pm associates.
Do you think it's helpful for power management?
How about hibernation scheme for runtime-pm?
I'm testing and I can introduce soon.


Well, I am thinking on following approach when we introduce
power management.

ufshcd_runtime_suspend() {
if (bkops_status = NON_CRITICAL) { /* 0x1 */
ufshcd_enable_auto_bkops();
hibernate(); /* only the link and the device
should be able to cary out bkops */
} else {
hibernate(); /* Link and the device for more savings */
}
}

Let me know if this is okay.

I still consider whether BKOPS is proper behavior with runtime-pm or not.


The BKOPS is something that host allows the card to carry out
when the host knows it is idle and not expecting back to back requests.
Runtime PM idle is the only way to know whether the device is
idle (unless we want to reinvent the wheel to detect the idleness of
host and trigger bkops). There was a discussion on this in MMC mailing
list as well, and folks have agreed to move idle time bkops to runtime
PM (http://thread.gmane.org/gmane.linux.kernel.mmc/19444/)

It looks like different.
eMMC cannot execute BKOPS itself unlike UFS.
That's the way eMMC's host should trigger the BKOPS manually.



I guess it is not much of a difference for UFS as far as we concern
only about idle time bkops. In UFS one can still disallow the device to
not carry out BKOPS and hence the case where UFS device also cannot
execute BKOPS itself and a idle timer is needed to allow BKOPS
sporadically so that device doesn't go into URGENT_BKOPS state.




How about this scenario? It seems more simple.

  If we concern a response latency of transfer requests, BKOPS can be 
disabled by default.

And then BKOPS can be enabled whenever device requests in some exception.
If you have any idea, let me know.


Exceptions are raised only when the device is in critical need for
bkops. Also the spec. recommends, host should ensure that the device
doesn't go into such states.

With your suggestion, when we disable bkops, the exception is raised and
we enable bkops after which there is no way to disable it again?

Yes, it's difficult to find proper time.
Maybe, BKOPS can be disabled when request comes up.


In cases where there are back-to-back heavy data write requests then it
is not proper to enable/disable BKOPS as soon as the new request comes
up. It may happen that the card will then move from PERFORMANCE_IMPACT
state to CRITICAL and ultimately start failing the requests. The point
is that we should never get into state where we need URGENT_BKOPS.

--
Regards,
Sujit
--
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 V3 2/4] scsi: ufs: Fix hardware race conditions while aborting a command

2013-07-19 Thread Sujit Reddy Thumma
On 7/19/2013 7:26 PM, Seungwon Jeon wrote:
 On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
 There is a possible race condition in the hardware when the abort
 command is issued to terminate the ongoing SCSI command as described
 below:

 - A bit in the door-bell register is set in the controller for a
new SCSI command.
 - In some rare situations, before controller get a chance to issue
the command to the device, the software issued an abort command.
 It's interesting.
 I wonder when we can meet this situation.
 Is it possible if SCSI mid layer should send abort command as soon as the 
 transfer command is issued?
 AFAIK abort command is followed if one command has timed out.
 That means command have been already issued and no response?
 If you had some problem, could you share?

You are right. This is a very rare case and probably don't happen at all
until and unless the SCSI error handling is changed. We found it just by
static analysis. Probably, at some point I may push a patch that tries
to reduce the read latencies while aborting a large write transfer when
I come across a UFS device that has command per LU as 1 :-)

I guess this is good to have change. The path chosen is according to
SCSI SAM-5 architecture specification, so I don't expect any issues
here.

 
 - If the device recieves abort command first then it returns success
 receives
 
because the command itself is not present.
 - Now if the controller commits the command to device it will be
processed.
 - Software thinks that command is aborted and proceed while still
the device is processing it.
 - The software, controller and device may go out of sync because of
this race condition.

 To avoid this, query task presence in the device before sending abort
 task command so that after the abort operation, the command is guaranteed
 to be non-existent in both controller and the device.

 Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
 ---
   drivers/scsi/ufs/ufshcd.c |   70 
 +++-
   1 files changed, 55 insertions(+), 15 deletions(-)

 diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
 index a176421..51ce096 100644
 --- a/drivers/scsi/ufs/ufshcd.c
 +++ b/drivers/scsi/ufs/ufshcd.c
 @@ -2550,6 +2550,12 @@ static int ufshcd_host_reset(struct scsi_cmnd *cmd)
* ufshcd_abort - abort a specific command
* @cmd: SCSI command pointer
*
 + * Abort the pending command in device by sending UFS_ABORT_TASK task 
 management
 + * command, and in host controller by clearing the door-bell register. 
 There can
 + * be race between controller sending the command to the device while abort 
 is
 + * issued. To avoid that, first issue UFS_QUERY_TASK to check if the 
 command is
 + * really issued and then try to abort it.
 + *
* Returns SUCCESS/FAILED
*/
   static int ufshcd_abort(struct scsi_cmnd *cmd)
 @@ -2558,7 +2564,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
  struct ufs_hba *hba;
  unsigned long flags;
  unsigned int tag;
 -int err;
 +int err = 0;
 +int poll_cnt;
  u8 resp;
  struct ufshcd_lrb *lrbp;

 @@ -2566,33 +2573,59 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
  hba = shost_priv(host);
  tag = cmd-request-tag;

 -spin_lock_irqsave(host-host_lock, flags);
 +/* If command is already aborted/completed, return SUCCESS */
 +if (!(test_bit(tag, hba-outstanding_reqs)))
 +goto out;

 -/* check if command is still pending */
 -if (!(test_bit(tag, hba-outstanding_reqs))) {
 -err = FAILED;
 -spin_unlock_irqrestore(host-host_lock, flags);
 +lrbp = hba-lrb[tag];
 +for (poll_cnt = 100; poll_cnt; poll_cnt--) {
 +err = ufshcd_issue_tm_cmd(hba, lrbp-lun, lrbp-task_tag,
 +UFS_QUERY_TASK, resp);
 +if (!err  resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
 +/* cmd pending in the device */
 +break;
 +} else if (!err  resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
 +u32 reg;
 +
 +/*
 + * cmd not pending in the device, check if it is
 + * in transition.
 + */
 +reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 +if (reg  (1  tag)) {
 +/* sleep for max. 2ms to stabilize */
 +usleep_range(1000, 2000);
 +continue;
 +}
 +/* command completed already */
 +goto out;
 +} else {
 +if (!err)
 +err = resp; /* service response error */
 +goto out;
 +}
 +}
 +
 +if (!poll_cnt) {
 +err = -EBUSY;
  goto out;
  }
 -spin_unlock_irqrestore(host-host_lock, 

Re: [PATCH V3 1/4] scsi: ufs: Fix broken task management command implementation

2013-07-19 Thread Sujit Reddy Thumma
On 7/19/2013 7:26 PM, Seungwon Jeon wrote:
 On Tue, July 09, 2013 Sujit Reddy Thumma wrote:
 Currently, sending Task Management (TM) command to the card might
 be broken in some scenarios as listed below:

 Problem: If there are more than 8 TM commands the implementation
   returns error to the caller.
 Fix: Wait for one of the slots to be emptied and send the command.

 Problem: Sometimes it is necessary for the caller to know the TM service
   response code to determine the task status.
 Fix: Propogate the service response to the caller.

 Problem: If the TM command times out no proper error recovery is
   implemented.
 Fix: Clear the command in the controller door-bell register, so that
   further commands for the same slot don't fail.

 Problem: While preparing the TM command descriptor, the task tag used
   should be unique across SCSI/NOP/QUERY/TM commands and not the
   task tag of the command which the TM command is trying to manage.
 Fix: Use a unique task tag instead of task tag of SCSI command.

 Problem: Since the TM command involves H/W communication, abruptly ending
   the request on kill interrupt signal might cause h/w malfunction.
 Fix: Wait for hardware completion interrupt with TASK_UNINTERRUPTIBLE
   set.

 Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
 ---
   drivers/scsi/ufs/ufshcd.c |  177 
 ++---
   drivers/scsi/ufs/ufshcd.h |8 ++-
   2 files changed, 126 insertions(+), 59 deletions(-)

 diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
 index af7d01d..a176421 100644
 --- a/drivers/scsi/ufs/ufshcd.c
 +++ b/drivers/scsi/ufs/ufshcd.c
 @@ -53,6 +53,9 @@
   /* Query request timeout */
   #define QUERY_REQ_TIMEOUT 30 /* msec */

 +/* Task management command timeout */
 +#define TM_CMD_TIMEOUT  100 /* msecs */
 +
   /* Expose the flag value from utp_upiu_query.value */
   #define MASK_QUERY_UPIU_FLAG_LOC 0xFF

 @@ -190,13 +193,35 @@ ufshcd_get_tmr_ocs(struct utp_task_req_desc 
 *task_req_descp)
   /**
* ufshcd_get_tm_free_slot - get a free slot for task management request
* @hba: per adapter instance
 + * @free_slot: pointer to variable with available slot value
*
 - * Returns maximum number of task management request slots in case of
 - * task management queue full or returns the free slot number
 + * Get a free tag and lock it until ufshcd_put_tm_slot() is called.
 + * Returns 0 if free slot is not available, else return 1 with tag value
 + * in @free_slot.
*/
 -static inline int ufshcd_get_tm_free_slot(struct ufs_hba *hba)
 +static bool ufshcd_get_tm_free_slot(struct ufs_hba *hba, int *free_slot)
 +{
 +int tag;
 +bool ret = false;
 +
 +if (!free_slot)
 +goto out;
 +
 +do {
 +tag = find_first_zero_bit(hba-tm_slots_in_use, hba-nutmrs);
 +if (tag = hba-nutmrs)
 +goto out;
 +} while (test_and_set_bit_lock(tag, hba-tm_slots_in_use));
 +
 +*free_slot = tag;
 +ret = true;
 +out:
 +return ret;
 +}
 +
 +static inline void ufshcd_put_tm_slot(struct ufs_hba *hba, int slot)
   {
 -return find_first_zero_bit(hba-outstanding_tasks, hba-nutmrs);
 +clear_bit_unlock(slot, hba-tm_slots_in_use);
   }

   /**
 @@ -1778,10 +1803,11 @@ static void ufshcd_slave_destroy(struct scsi_device 
 *sdev)
* ufshcd_task_req_compl - handle task management request completion
* @hba: per adapter instance
* @index: index of the completed request
 + * @resp: task management service response
*
 - * Returns SUCCESS/FAILED
 + * Returns non-zero value on error, zero on success
*/
 -static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index)
 +static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index, u8 *resp)
   {
  struct utp_task_req_desc *task_req_descp;
  struct utp_upiu_task_rsp *task_rsp_upiup;
 @@ -1802,19 +1828,15 @@ static int ufshcd_task_req_compl(struct ufs_hba 
 *hba, u32 index)
  task_req_descp[index].task_rsp_upiu;
  task_result = be32_to_cpu(task_rsp_upiup-header.dword_1);
  task_result = ((task_result  MASK_TASK_RESPONSE)  8);
 -
 -if (task_result != UPIU_TASK_MANAGEMENT_FUNC_COMPL 
 -task_result != UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED)
 -task_result = FAILED;
 -else
 -task_result = SUCCESS;
 +if (resp)
 +*resp = (u8)task_result;
  } else {
 -task_result = FAILED;
 -dev_err(hba-dev,
 -trc: Invalid ocs = %x\n, ocs_value);
 +dev_err(hba-dev, %s: failed, ocs = 0x%x\n,
 +__func__, ocs_value);
  }
  spin_unlock_irqrestore(hba-host-host_lock, flags);
 -return task_result;
 +
 +return ocs_value;
   }

   /**
 @@ -2298,7 +2320,7 @@ static void 

Re: [PATCH V3 3/4] scsi: ufs: Fix device and host reset methods

2013-07-19 Thread Sujit Reddy Thumma
On 7/19/2013 7:27 PM, Seungwon Jeon wrote:
 On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
 As of now SCSI initiated error handling is broken because,
 the reset APIs don't try to bring back the device initialized and
 ready for further transfers.

 In case of timeouts, the scsi error handler takes care of handling aborts
 and resets. Improve the error handling in such scenario by resetting the
 device and host and re-initializing them in proper manner.

 Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
 ---
   drivers/scsi/ufs/ufshcd.c |  467 
 +++--
   drivers/scsi/ufs/ufshcd.h |2 +
   2 files changed, 411 insertions(+), 58 deletions(-)

 diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
 index 51ce096..b4c9910 100644
 --- a/drivers/scsi/ufs/ufshcd.c
 +++ b/drivers/scsi/ufs/ufshcd.c
 @@ -69,9 +69,15 @@ enum {

   /* UFSHCD states */
   enum {
 -UFSHCD_STATE_OPERATIONAL,
  UFSHCD_STATE_RESET,
  UFSHCD_STATE_ERROR,
 +UFSHCD_STATE_OPERATIONAL,
 +};
 +
 +/* UFSHCD error handling flags */
 +enum {
 +UFSHCD_EH_HOST_RESET_PENDING = (1  0),
 +UFSHCD_EH_DEVICE_RESET_PENDING = (1  1),
   };

   /* Interrupt configuration options */
 @@ -87,6 +93,22 @@ enum {
  INT_AGGR_CONFIG,
   };

 +#define ufshcd_set_device_reset_pending(h) \
 +(h-eh_flags |= UFSHCD_EH_DEVICE_RESET_PENDING)
 +#define ufshcd_set_host_reset_pending(h) \
 +(h-eh_flags |= UFSHCD_EH_HOST_RESET_PENDING)
 +#define ufshcd_device_reset_pending(h) \
 +(h-eh_flags  UFSHCD_EH_DEVICE_RESET_PENDING)
 +#define ufshcd_host_reset_pending(h) \
 +(h-eh_flags  UFSHCD_EH_HOST_RESET_PENDING)
 +#define ufshcd_clear_device_reset_pending(h) \
 +(h-eh_flags = ~UFSHCD_EH_DEVICE_RESET_PENDING)
 +#define ufshcd_clear_host_reset_pending(h) \
 +(h-eh_flags = ~UFSHCD_EH_HOST_RESET_PENDING)
 +
 +static void ufshcd_tmc_handler(struct ufs_hba *hba);
 +static void ufshcd_async_scan(void *data, async_cookie_t cookie);
 +
   /*
* ufshcd_wait_for_register - wait for register value to change
* @hba - per-adapter interface
 @@ -851,9 +873,22 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, 
 struct scsi_cmnd *cmd)

  tag = cmd-request-tag;

 -if (hba-ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
 +switch (hba-ufshcd_state) {
 Lock is no needed for ufshcd_state?
 
 +case UFSHCD_STATE_OPERATIONAL:
 +break;
 +case UFSHCD_STATE_RESET:
  err = SCSI_MLQUEUE_HOST_BUSY;
  goto out;
 +case UFSHCD_STATE_ERROR:
 +set_host_byte(cmd, DID_ERROR);
 +cmd-scsi_done(cmd);
 +goto out;
 +default:
 +dev_WARN_ONCE(hba-dev, 1, %s: invalid state %d\n,
 +__func__, hba-ufshcd_state);
 +set_host_byte(cmd, DID_BAD_TARGET);
 +cmd-scsi_done(cmd);
 +goto out;
  }

  /* acquire the tag to make sure device cmds don't use it */
 @@ -1573,8 +1608,6 @@ static int ufshcd_make_hba_operational(struct ufs_hba 
 *hba)
  if (hba-ufshcd_state == UFSHCD_STATE_RESET)
  scsi_unblock_requests(hba-host);

 -hba-ufshcd_state = UFSHCD_STATE_OPERATIONAL;
 -
   out:
  return err;
   }
 @@ -2273,6 +2306,106 @@ out:
   }

   /**
 + * ufshcd_utrl_is_rsr_enabled - check if run-stop register is enabled
 + * @hba: per-adapter instance
 + */
 +static bool ufshcd_utrl_is_rsr_enabled(struct ufs_hba *hba)
 +{
 +return ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_LIST_RUN_STOP)  0x1;
 +}
 +
 +/**
 + * ufshcd_utmrl_is_rsr_enabled - check if run-stop register is enabled
 + * @hba: per-adapter instance
 + */
 +static bool ufshcd_utmrl_is_rsr_enabled(struct ufs_hba *hba)
 +{
 +return ufshcd_readl(hba, REG_UTP_TASK_REQ_LIST_RUN_STOP)  0x1;
 +}
 +
 +/**
 + * ufshcd_complete_pending_tasks - complete outstanding tasks
 + * @hba: per adapter instance
 + *
 + * Abort in-progress task management commands and wakeup
 + * waiting threads.
 + *
 + * Returns non-zero error value when failed to clear all the commands.
 + */
 +static int ufshcd_complete_pending_tasks(struct ufs_hba *hba)
 +{
 +u32 reg;
 +int err = 0;
 +unsigned long flags;
 +
 +if (!hba-outstanding_tasks)
 +goto out;
 +
 +/* Clear UTMRL only when run-stop is enabled */
 +if (ufshcd_utmrl_is_rsr_enabled(hba))
 +ufshcd_writel(hba, ~hba-outstanding_tasks,
 +REG_UTP_TASK_REQ_LIST_CLEAR);
 +
 +/* poll for max. 1 sec to clear door bell register by h/w */
 +reg = ufshcd_wait_for_register(hba,
 +REG_UTP_TASK_REQ_DOOR_BELL,
 +hba-outstanding_tasks, 0, 1000, 1000);
 +if (reg  hba-outstanding_tasks)
 +err = -ETIMEDOUT;
 +
 +spin_lock_irqsave(hba-host-host_lock, flags);
 +/* complete commands that were cleared out */
 +ufshcd_tmc_handler(hba);
 +spin_unlock_irqrestore(hba-host-host_lock, flags);
 +out:
 +if 

Re: [PATCH V3 4/4] scsi: ufs: Improve UFS fatal error handling

2013-07-19 Thread Sujit Reddy Thumma
On 7/19/2013 7:28 PM, Seungwon Jeon wrote:
 On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
 Error handling in UFS driver is broken and resets the host controller
 for fatal errors without re-initialization. Correct the fatal error
 handling sequence according to UFS Host Controller Interface (HCI)
 v1.1 specification.

 o Upon determining fatal error condition the host controller may hang
forever until a reset is applied, so just retrying the command doesn't
work without a reset. So, the reset is applied in the driver context
in a separate work and SCSI mid-layer isn't informed until reset is
applied.

 o Processed requests which are completed without error are reported to
SCSI layer as successful and any pending commands that are not started
yet or are not cause of the error are re-queued into scsi midlayer queue.
For the command that caused error, host controller or device is reset
and DID_ERROR is returned for command retry after applying reset.

 o SCSI is informed about the expected Unit-Attentioni exception from the
 Attention'i',  typo.
Okay.

 
device for the immediate command after a reset so that the SCSI layer
take necessary steps to establish communication with the device.

 Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
 ---
   drivers/scsi/ufs/ufshcd.c |  349 
 +++-
   drivers/scsi/ufs/ufshcd.h |2 +
   drivers/scsi/ufs/ufshci.h |   19 ++-
   3 files changed, 295 insertions(+), 75 deletions(-)

 diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
 index b4c9910..2a3874f 100644
 --- a/drivers/scsi/ufs/ufshcd.c
 +++ b/drivers/scsi/ufs/ufshcd.c
 @@ -80,6 +80,14 @@ enum {
  UFSHCD_EH_DEVICE_RESET_PENDING = (1  1),
   };

 +/* UFSHCD UIC layer error flags */
 +enum {
 +UFSHCD_UIC_DL_PA_INIT_ERROR = (1  0), /* Data link layer error */
 +UFSHCD_UIC_NL_ERROR = (1  1), /* Network layer error */
 +UFSHCD_UIC_TL_ERROR = (1  2), /* Transport Layer error */
 +UFSHCD_UIC_DME_ERROR = (1  3), /* DME error */
 +};
 +
   /* Interrupt configuration options */
   enum {
  UFSHCD_INT_DISABLE,
 @@ -108,6 +116,7 @@ enum {

   static void ufshcd_tmc_handler(struct ufs_hba *hba);
   static void ufshcd_async_scan(void *data, async_cookie_t cookie);
 +static int ufshcd_reset_and_restore(struct ufs_hba *hba);

   /*
* ufshcd_wait_for_register - wait for register value to change
 @@ -1605,9 +1614,6 @@ static int ufshcd_make_hba_operational(struct ufs_hba 
 *hba)
  goto out;
  }

 -if (hba-ufshcd_state == UFSHCD_STATE_RESET)
 -scsi_unblock_requests(hba-host);
 -
   out:
  return err;
   }
 @@ -1733,66 +1739,6 @@ static int ufshcd_validate_dev_connection(struct 
 ufs_hba *hba)
   }

   /**
 - * ufshcd_do_reset - reset the host controller
 - * @hba: per adapter instance
 - *
 - * Returns SUCCESS/FAILED
 - */
 -static int ufshcd_do_reset(struct ufs_hba *hba)
 -{
 -struct ufshcd_lrb *lrbp;
 -unsigned long flags;
 -int tag;
 -
 -/* block commands from midlayer */
 -scsi_block_requests(hba-host);
 -
 -spin_lock_irqsave(hba-host-host_lock, flags);
 -hba-ufshcd_state = UFSHCD_STATE_RESET;
 -
 -/* send controller to reset state */
 -ufshcd_hba_stop(hba);
 -spin_unlock_irqrestore(hba-host-host_lock, flags);
 -
 -/* abort outstanding commands */
 -for (tag = 0; tag  hba-nutrs; tag++) {
 -if (test_bit(tag, hba-outstanding_reqs)) {
 -lrbp = hba-lrb[tag];
 -if (lrbp-cmd) {
 -scsi_dma_unmap(lrbp-cmd);
 -lrbp-cmd-result = DID_RESET  16;
 -lrbp-cmd-scsi_done(lrbp-cmd);
 -lrbp-cmd = NULL;
 -clear_bit_unlock(tag, hba-lrb_in_use);
 -}
 -}
 -}
 -
 -/* complete device management command */
 -if (hba-dev_cmd.complete)
 -complete(hba-dev_cmd.complete);
 -
 -/* clear outstanding request/task bit maps */
 -hba-outstanding_reqs = 0;
 -hba-outstanding_tasks = 0;
 -
 -/* Host controller enable */
 -if (ufshcd_hba_enable(hba)) {
 -dev_err(hba-dev,
 -Reset: Controller initialization failed\n);
 -return FAILED;
 -}
 -
 -if (ufshcd_link_startup(hba)) {
 -dev_err(hba-dev,
 -Reset: Link start-up failed\n);
 -return FAILED;
 -}
 -
 -return SUCCESS;
 -}
 -
 -/**
* ufshcd_slave_alloc - handle initial SCSI device configurations
* @sdev: pointer to SCSI device
*
 @@ -1809,6 +1755,9 @@ static int ufshcd_slave_alloc(struct scsi_device *sdev)
  sdev-use_10_for_ms = 1;
  scsi_set_tag_type(sdev, MSG_SIMPLE_TAG);

 +/* allow SCSI layer to restart the device in case of errors */
 +sdev-allow_restart = 1;
 +
  /*
   * Inform SCSI Midlayer that the 

Re: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU

2013-07-19 Thread Sujit Reddy Thumma

On 7/19/2013 7:17 PM, Seungwon Jeon wrote:

On Thu, July 18, 2013, Sujit Reddy Thumma wrote:

+ * ufshcd_wait_for_register - wait for register value to change
+ * @hba - per-adapter interface
+ * @reg - mmio register offset
+ * @mask - mask to apply to read register value
+ * @val - wait condition
+ * @interval_us - polling interval in microsecs
+ * @timeout_ms - timeout in millisecs
+ *
+ * Returns final register value after iteration
+ */
+static u32 ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
+   u32 val, unsigned long interval_us, unsigned long timeout_ms)

I feel like this function's role is to wait for clearing register (specially, 
doorbells).
If you really don't intend to apply other all register, I think it would better 
to change the

function name.

ex ufshcd_wait_for_clear_doorbell or ufshcd_wait_for_clear_reg?


Although, this API is introduced in this patch and used only for
clearing the doorbell, I have intentionally made it generic to avoid
duplication of wait condition code in future (not just clearing but
also for setting a bit). For example, when we write to HCE.ENABLE we
wait for it turn to 1.



And if you like it, it could be more simple like below

static bool ufshcd_wait_for_clear_reg(struct ufs_hba *hba, u32 reg, u32 mask,
unsigned long interval_us,
unsigned int timeout_ms)
{
   unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);


Jiffies on 100Hz systems have minimum granularity of 10ms. So we really
wait for more 10ms even though the timeout_ms  10ms.

Yes. Real timeout depends on system.
But normally actual wait will be maintained until 'ufshcd_readl' is done.
Timeout is valid for failure case.  If we don't need to apply a strict timeout 
value, it's not bad.


Hmm.. make sense. Will take care in the next patchset.






   /* wakeup within 50us of expiry */
   const unsigned int expiry = 50;

   while (ufshcd_readl(hba, reg)  mask) {
   usleep_range(interval_us, interval_us + expiry);
   if (time_after(jiffies, timeout)) {
   if (ufshcd_readl(hba, reg)  mask)
   return false;


I really want the caller to decide on what to do after the timeout.
This helps in error handling cases where we try to clear off the entire
door-bell register but only a few of the bits are cleared after timeout.

I don't know what we can do with the report of the partial success on clearing 
bit.
It's just failure case. Isn't enough with false/true?


The point is, if a bit can't be cleared it can be regarded as a
permanent failure (only for that request), otherwise, it can be
retried with the same tag value.






   else
   return true;
   }
   }

   return true;
}

+{
+   u32 tmp;
+   ktime_t start;
+   unsigned long diff;
+
+   tmp = ufshcd_readl(hba, reg);
+
+   if ((val  mask) != val) {
+   dev_err(hba-dev, %s: Invalid wait condition 0x%x\n,
+   __func__, val);
+   goto out;
+   }
+
+   start = ktime_get();
+   while ((tmp  mask) != val) {
+   /* wakeup within 50us of expiry */
+   usleep_range(interval_us, interval_us + 50);
+   tmp = ufshcd_readl(hba, reg);
+   diff = ktime_to_ms(ktime_sub(ktime_get(), start));
+   if (diff  timeout_ms) {
+   tmp = ufshcd_readl(hba, reg);
+   break;
+   }
+   }
+out:
+   return tmp;
+}
+

..

+static int
+ufshcd_clear_cmd(struct ufs_hba *hba, int tag)
+{
+   int err = 0;
+   unsigned long flags;
+   u32 reg;
+   u32 mask = 1  tag;
+
+   /* clear outstanding transaction before retry */
+   spin_lock_irqsave(hba-host-host_lock, flags);
+   ufshcd_utrl_clear(hba, tag);
+   spin_unlock_irqrestore(hba-host-host_lock, flags);
+
+   /*
+* wait for for h/w to clear corresponding bit in door-bell.
+* max. wait is 1 sec.
+*/
+   reg = ufshcd_wait_for_register(hba,
+   REG_UTP_TRANSFER_REQ_DOOR_BELL,
+   mask, 0, 1000, 1000);

4th argument should be (~mask) instead of '0', right?


True, but not really for this implementation. It breaks the check in
in wait_for_register -
if ((val  mask) != val)
  dev_err(...);

Hmm, it seems complicated to use.
Ok. Is right the following about val as 4th argument?
- clear: val  should be '0' regardless corresponding bit.
- set: val should be same with mask.
If the related comment is added, it will be helpful.


Thinking again it looks like it is complicated. How about changing
the check to -

val = val  mask; /* ignore the bits we don't intend to wait on */
while (ufshcd_readl()  mask != 

Re: [PATCH RESEND 0/1] AHCI: Optimize interrupt processing

2013-07-19 Thread Nicholas A. Bellinger
On Fri, 2013-07-19 at 08:33 -0700, James Bottomley wrote:
 On Thu, 2013-07-18 at 23:34 -0700, Nicholas A. Bellinger wrote:
  diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
  index 0101af5..191bc15 100644
  --- a/drivers/ata/libata-scsi.c
  +++ b/drivers/ata/libata-scsi.c
  @@ -1144,7 +1144,11 @@ static int ata_scsi_dev_config(struct scsi_device 
  *sdev,
  sector_size=%u  PAGE_SIZE, PIO may malfunction\n,
  sdev-sector_size);
   
  -   blk_queue_update_dma_alignment(q, sdev-sector_size - 1);
  +   if (!q-mq_ops) {
  +   blk_queue_update_dma_alignment(q, sdev-sector_size - 1);
  +   } else {
  +   printk(Skipping dma_alignment for libata w/ scsi-mq\n);
  +   }
 
 Amazingly enough there is a reason for the dma alignment, and it wasn't
 just to annoy you, so you can't blindly do this.
 
 The email thread is probably lost in the mists of time, but if I
 remember correctly the problem is that some ahci DMA controllers barf if
 the sector they're doing DMA on crosses a page boundary.  Some are
 annoying enough to actually cause silent data corruption.  You won't
 find every ahci DMA controller doing this, so the change will work for
 some, but it will be hard to identify those it won't work for until
 people start losing data.

Thanks for the extra background.

So at least from what I gather thus far this shouldn't be an issue for
initial testing with scsi-mq - libata w/ ata_piix.

 
 The correct fix, obviously, is to do the bio copy on the kernel path for
 unaligned data.  It is OK to assume that REQ_TYPE_FS data is correctly
 aligned (because of the block to page alignment).
 

Indeed.  Looking into the bio_copy_kern() breakage next..

--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: [PATCH RESEND 0/1] AHCI: Optimize interrupt processing

2013-07-19 Thread Nicholas A. Bellinger
On Fri, 2013-07-19 at 09:58 -0600, Mike Christie wrote:
 On 07/18/2013 06:23 PM, Nicholas A. Bellinger wrote:
  Just saw this while trying out iscsi with the scsi-mq stuff :)
   
  Took at stab at this a while back, but ended getting distracted on other
  items.  Do you have an initial conversion running yet..?
 
 Not running well :) Have a patch but I am debugging it now. I messed up
 something with the cmd_size stuff.

nod, looking forward to see ib_iser move beyond the existing
scsi_request_fn() bottleneck.   ;)

Feel free to send me the WIP off-list if you'd like another pair of
eyes.

--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: [Ksummit-2013-discuss] [ATTEND] scsi-mq prototype discussion

2013-07-19 Thread Nicholas A. Bellinger
On Wed, 2013-07-17 at 04:52 +, James Bottomley wrote:
 On Tue, 2013-07-16 at 15:15 -0600, Jens Axboe wrote:
  On Tue, Jul 16 2013, Nicholas A. Bellinger wrote:
   On Sat, 2013-07-13 at 06:53 +, James Bottomley wrote:

SNIP

Lets start with discussing this on the list, please, and then see where
we go from there ...

   
   Yes, the discussion is beginning to make it's way to the list.  I've
   mostly been waiting for blk-mq to get a wider review before taking the
   early scsi-mq prototype driver to a larger public audience.
   
   Primarily, I'm now reaching out to the people most effected by existing
   scsi_request_fn() based performance limitations.  Most of them have
   abandoned existing scsi_request_fn() based logic in favor of raw block
   make_request() based drivers, and are now estimating the amount of
   effort to move to an scsi-mq based approach.
   
   Regardless, as the prototype progresses over the next months, having a
   face-to-face discussion with the key parties in the room would be very
   helpful given the large amount of effort involved to actually make this
   type of generational shift in SCSI actually happen.
  
  There's a certain amount of overlap with the aio/O_DIRECT work as well.
  But if it's not a general session, could always be a BOF or something.
  
  I'll second the argument that most technical topics probably DO belong
  in a topic related workshop. But that leaves us with basically only
  process related topics at KS, I don't think it hurts to have a bit of
  tech meat on the bone too. At least I personally miss that part of KS
  from years gone by.
 
 Heh well, given that most of the block mq discussions at LSF have been
 you saying you really should get around to cleaning up and posting the
 code, you'll understand my wanting to see that happen first ...
 
 I suppose we could try to run a storage workshop within KS, but I think
 most of the mini summit slots have already gone.

That would be great, given there is a reasonable level of interest from
various parities, and the pain threshold for existing scsi small block
random I/O performance is high..

When will we know if there is a workshop / mini summit slot available..?

(CC'ing Mike Christie as well for open-iscsi/scsi-mq bits)

 There's also plumbers
 if all slots are gone (I would say that, being biased and on the
 programme committee) Ric is running the storage and Filesystems MC
 
 http://www.linuxplumbersconf.org/2013/ocw/events/LPC2013/tracks/159

FYI, I'm not trying to 'sell' scsi-mq to the larger community yet, but
rather interested in getting the right scsi/block/LLD people in the same
room at KS for an hour or two to discuss implementation details, given
the scope of the effort involved.

--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: [Ksummit-2013-discuss] [ATTEND] scsi-mq prototype discussion

2013-07-19 Thread James Bottomley
On Fri, 2013-07-19 at 14:22 -0700, Nicholas A. Bellinger wrote:
 On Wed, 2013-07-17 at 04:52 +, James Bottomley wrote:
  On Tue, 2013-07-16 at 15:15 -0600, Jens Axboe wrote:
   On Tue, Jul 16 2013, Nicholas A. Bellinger wrote:
On Sat, 2013-07-13 at 06:53 +, James Bottomley wrote:
 
 SNIP
 
 Lets start with discussing this on the list, please, and then see 
 where
 we go from there ...
 

Yes, the discussion is beginning to make it's way to the list.  I've
mostly been waiting for blk-mq to get a wider review before taking the
early scsi-mq prototype driver to a larger public audience.

Primarily, I'm now reaching out to the people most effected by existing
scsi_request_fn() based performance limitations.  Most of them have
abandoned existing scsi_request_fn() based logic in favor of raw block
make_request() based drivers, and are now estimating the amount of
effort to move to an scsi-mq based approach.

Regardless, as the prototype progresses over the next months, having a
face-to-face discussion with the key parties in the room would be very
helpful given the large amount of effort involved to actually make this
type of generational shift in SCSI actually happen.
   
   There's a certain amount of overlap with the aio/O_DIRECT work as well.
   But if it's not a general session, could always be a BOF or something.
   
   I'll second the argument that most technical topics probably DO belong
   in a topic related workshop. But that leaves us with basically only
   process related topics at KS, I don't think it hurts to have a bit of
   tech meat on the bone too. At least I personally miss that part of KS
   from years gone by.
  
  Heh well, given that most of the block mq discussions at LSF have been
  you saying you really should get around to cleaning up and posting the
  code, you'll understand my wanting to see that happen first ...
  
  I suppose we could try to run a storage workshop within KS, but I think
  most of the mini summit slots have already gone.
 
 That would be great, given there is a reasonable level of interest from
 various parities, and the pain threshold for existing scsi small block
 random I/O performance is high..
 
 When will we know if there is a workshop / mini summit slot available..?
 
 (CC'ing Mike Christie as well for open-iscsi/scsi-mq bits)
 
  There's also plumbers
  if all slots are gone (I would say that, being biased and on the
  programme committee) Ric is running the storage and Filesystems MC
  
  http://www.linuxplumbersconf.org/2013/ocw/events/LPC2013/tracks/159
 
 FYI, I'm not trying to 'sell' scsi-mq to the larger community yet, but
 rather interested in getting the right scsi/block/LLD people in the same
 room at KS for an hour or two to discuss implementation details, given
 the scope of the effort involved.

Well, so that's why I think plumbers might be a better venue: we'll have
more of the actual storage people there.  Usually we get at most 2-3
storage people to KS compared to the 25 or so we usually have at LSF ...
that makes KS not a very good venue for storage centric discussions.

James

--
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: [Ksummit-2013-discuss] [ATTEND] scsi-mq prototype discussion

2013-07-19 Thread Nicholas A. Bellinger
On Fri, 2013-07-19 at 21:46 +, James Bottomley wrote:
 On Fri, 2013-07-19 at 14:22 -0700, Nicholas A. Bellinger wrote:
  On Wed, 2013-07-17 at 04:52 +, James Bottomley wrote:
   On Tue, 2013-07-16 at 15:15 -0600, Jens Axboe wrote:
On Tue, Jul 16 2013, Nicholas A. Bellinger wrote:
 On Sat, 2013-07-13 at 06:53 +, James Bottomley wrote:
  
  SNIP
  
  Lets start with discussing this on the list, please, and then see 
  where
  we go from there ...
  
 
 Yes, the discussion is beginning to make it's way to the list.  I've
 mostly been waiting for blk-mq to get a wider review before taking the
 early scsi-mq prototype driver to a larger public audience.
 
 Primarily, I'm now reaching out to the people most effected by 
 existing
 scsi_request_fn() based performance limitations.  Most of them have
 abandoned existing scsi_request_fn() based logic in favor of raw block
 make_request() based drivers, and are now estimating the amount of
 effort to move to an scsi-mq based approach.
 
 Regardless, as the prototype progresses over the next months, having a
 face-to-face discussion with the key parties in the room would be very
 helpful given the large amount of effort involved to actually make 
 this
 type of generational shift in SCSI actually happen.

There's a certain amount of overlap with the aio/O_DIRECT work as well.
But if it's not a general session, could always be a BOF or something.

I'll second the argument that most technical topics probably DO belong
in a topic related workshop. But that leaves us with basically only
process related topics at KS, I don't think it hurts to have a bit of
tech meat on the bone too. At least I personally miss that part of KS
from years gone by.
   
   Heh well, given that most of the block mq discussions at LSF have been
   you saying you really should get around to cleaning up and posting the
   code, you'll understand my wanting to see that happen first ...
   
   I suppose we could try to run a storage workshop within KS, but I think
   most of the mini summit slots have already gone.
  
  That would be great, given there is a reasonable level of interest from
  various parities, and the pain threshold for existing scsi small block
  random I/O performance is high..
  
  When will we know if there is a workshop / mini summit slot available..?
  
  (CC'ing Mike Christie as well for open-iscsi/scsi-mq bits)
  
   There's also plumbers
   if all slots are gone (I would say that, being biased and on the
   programme committee) Ric is running the storage and Filesystems MC
   
   http://www.linuxplumbersconf.org/2013/ocw/events/LPC2013/tracks/159
  
  FYI, I'm not trying to 'sell' scsi-mq to the larger community yet, but
  rather interested in getting the right scsi/block/LLD people in the same
  room at KS for an hour or two to discuss implementation details, given
  the scope of the effort involved.
 
 Well, so that's why I think plumbers might be a better venue: we'll have
 more of the actual storage people there.  Usually we get at most 2-3
 storage people to KS compared to the 25 or so we usually have at LSF ...
 that makes KS not a very good venue for storage centric discussions.
 

The most relevant people for the discussion are Jens, Hannes, Christoph,
Tejun, Martin, Mike, and you.

I know these folks are regular attendees for KS, but typically not for
plumbers, which is why I made this KS topic proposal in the first place.

--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: [PATCH v4 1/4] [SCSI] sg: use rwsem to solve race during exclusive open

2013-07-19 Thread Jörn Engel
On Wed, 17 July 2013 23:34:03 +0800, Vaughan Cao wrote:
 Date: Wed, 17 Jul 2013 23:34:03 +0800
 From: Vaughan Cao vaughan@oracle.com
 To: jo...@logfs.org
 Cc: dgilb...@interlog.com, jbottom...@parallels.com,
   linux-scsi@vger.kernel.org, linux-ker...@vger.kernel.org,
   vaughan@oracle.com
 Subject: [PATCH v4 1/4] [SCSI] sg: use rwsem to solve race during
  exclusive open
 X-Mailer: git-send-email 1.7.11.7
 
 A race condition may happen if two threads are both trying to open the same sg
 with O_EXCL simultaneously. It's possible that they both find fsds list is
 empty and get_exclude(sdp) returns 0, then they both call set_exclude() and
 break out from wait_event_interruptible and resume open.
 
 Now use rwsem to protect this process. Exclusive open gets write lock and
 others get read lock. The lock will be held until file descriptor is closed.
 This also leads 'exclude' only a status rather than a check mark.
 
 Signed-off-by: Vaughan Cao vaughan@oracle.com
Reviewed-by: Joern Engel jo...@logfs.org

Jörn

--
When you close your hand, you own nothing. When you open it up, you
own the whole world.
-- Li Mu Bai in Tiger  Dragon
--
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 4/4] [SCSI] sg: push file descriptor list locking down to per-device locking

2013-07-19 Thread Jörn Engel
On Wed, 17 July 2013 23:34:06 +0800, Vaughan Cao wrote:
 Date: Wed, 17 Jul 2013 23:34:06 +0800
 From: Vaughan Cao vaughan@oracle.com
 To: jo...@logfs.org
 Cc: dgilb...@interlog.com, jbottom...@parallels.com,
   linux-scsi@vger.kernel.org, linux-ker...@vger.kernel.org,
   vaughan@oracle.com
 Subject: [PATCH v4 4/4] [SCSI] sg: push file descriptor list locking down
  to per-device locking
 X-Mailer: git-send-email 1.7.11.7
 
 Push file descriptor list locking down to per-device locking. Let 
 sg_index_lock
 only protect device lookup.
 sdp-detached is also set and checked with this lock held.
 
 Signed-off-by: Vaughan Cao vaughan@oracle.com

Reviewed-by: Joern Engel jo...@logfs.org

Jörn

--
What one programmer can do in one month, two programmers can do in two months.
-- Fred Brooks
--
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 2/4] [SCSI] sg: no need sg_open_exclusive_lock

2013-07-19 Thread Jörn Engel
On Wed, 17 July 2013 23:34:04 +0800, Vaughan Cao wrote:
 Date: Wed, 17 Jul 2013 23:34:04 +0800
 From: Vaughan Cao vaughan@oracle.com
 To: jo...@logfs.org
 Cc: dgilb...@interlog.com, jbottom...@parallels.com,
   linux-scsi@vger.kernel.org, linux-ker...@vger.kernel.org,
   vaughan@oracle.com
 Subject: [PATCH v4 2/4] [SCSI] sg: no need sg_open_exclusive_lock
 X-Mailer: git-send-email 1.7.11.7
 
 Open exclusive check is protected by o_sem, no need sg_open_exclusive_lock.
 @exclude is used to record which type of rwsem we are holding.
 
 Signed-off-by: Vaughan Cao vaughan@oracle.com
Reviewed-by: Joern Engel jo...@logfs.org

Jörn

--
Error protection by error detection and correction.
-- from a university class
--
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 3/4] [SCSI] sg: checking sdp-detached isn't protected when open

2013-07-19 Thread Jörn Engel
On Wed, 17 July 2013 23:34:05 +0800, Vaughan Cao wrote:

 -static Sg_fd *sg_add_sfp(Sg_device * sdp, int dev);
 +static Sg_fd *sg_add_sfp(Sg_device * sdp, int dev, int * reason);

You can use ERR_PTR and friends instead of adding another parameter.

  static void sg_remove_sfp(struct kref *);
  static Sg_request *sg_get_rq_mark(Sg_fd * sfp, int pack_id);
  static Sg_request *sg_add_request(Sg_fd * sfp);
 @@ -295,21 +295,14 @@ sg_open(struct inode *inode, struct file *filp)
   if (flags  O_EXCL)
   sdp-exclude = 1;   /* used by release lock */
  
 - if (sdp-detached) {
 - retval = -ENODEV;
 - goto sem_out;
 - }
   if (sfds_list_empty(sdp)) { /* no existing opens on this device */
   sdp-sgdebug = 0;
   q = sdp-device-request_queue;
   sdp-sg_tablesize = queue_max_segments(q);
   }
 - if ((sfp = sg_add_sfp(sdp, dev)))
 - filp-private_data = sfp;
 - else {
 - retval = -ENOMEM;
 + if (!(sfp = sg_add_sfp(sdp, dev, retval)))
   goto sem_out;
 - }

sfp = sg_add_sfp(sdp, dev);
if (IS_ERR(sfp)) {
retval = PTR_ERR(sfp);
goto sem_out;
}

 + filp-private_data = sfp;
   retval = 0;
  
   if (retval) {
 @@ -2047,15 +2040,18 @@ sg_remove_request(Sg_fd * sfp, Sg_request * srp)
  }
  
  static Sg_fd *
 -sg_add_sfp(Sg_device * sdp, int dev)
 +sg_add_sfp(Sg_device * sdp, int dev, int * reason)
  {
   Sg_fd *sfp;
   unsigned long iflags;
   int bufflen;
  
   sfp = kzalloc(sizeof(*sfp), GFP_ATOMIC | __GFP_NOWARN);
 - if (!sfp)
 + if (!sfp) {
 + if (reason)
 + *reason = -ENOMEM;
   return NULL;
 + }

if (!sfp)
return ERR_PTR(-ENOMEM);

Jörn

--
Luck is when opportunity meets good preparation.
-- Chinese proverb
--
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] mpt2sas: don't handle broadcast primitives

2013-07-19 Thread Jörn Engel
The handling of broadcast primitives involves
_scsih_block_io_all_device(), which does what the name implies.  I have
observed cases with 60s of blocking io on all devices, caused by a
single bad device.  The downsides of this code are obvious, while the
upsides are more elusive.

Signed-off-by: Joern Engel jo...@logfs.org
---
 drivers/scsi/mpt2sas/mpt2sas_base.c  |4 -
 drivers/scsi/mpt2sas/mpt2sas_scsih.c |  238 --
 2 files changed, 242 deletions(-)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c 
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 21c0a27..bba2209 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -586,9 +586,6 @@ _base_display_event_data(struct MPT2SAS_ADAPTER *ioc,
printk(\n);
return;
}
-   case MPI2_EVENT_SAS_BROADCAST_PRIMITIVE:
-   desc = SAS Broadcast Primitive;
-   break;
case MPI2_EVENT_SAS_INIT_DEVICE_STATUS_CHANGE:
desc = SAS Init Device Status Change;
break;
@@ -4370,7 +4367,6 @@ mpt2sas_base_attach(struct MPT2SAS_ADAPTER *ioc)
 
/* here we enable the events we care about */
_base_unmask_events(ioc, MPI2_EVENT_SAS_DISCOVERY);
-   _base_unmask_events(ioc, MPI2_EVENT_SAS_BROADCAST_PRIMITIVE);
_base_unmask_events(ioc, MPI2_EVENT_SAS_TOPOLOGY_CHANGE_LIST);
_base_unmask_events(ioc, MPI2_EVENT_SAS_DEVICE_STATUS_CHANGE);
_base_unmask_events(ioc, MPI2_EVENT_SAS_ENCL_DEVICE_STATUS_CHANGE);
diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c 
b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
index 3fffb95..856a502 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
@@ -2915,31 +2915,6 @@ _scsih_fw_event_cleanup_queue(struct MPT2SAS_ADAPTER 
*ioc)
 }
 
 /**
- * _scsih_ublock_io_all_device - unblock every device
- * @ioc: per adapter object
- *
- * change the device state from block to running
- */
-static void
-_scsih_ublock_io_all_device(struct MPT2SAS_ADAPTER *ioc)
-{
-   struct MPT2SAS_DEVICE *sas_device_priv_data;
-   struct scsi_device *sdev;
-
-   shost_for_each_device(sdev, ioc-shost) {
-   sas_device_priv_data = sdev-hostdata;
-   if (!sas_device_priv_data)
-   continue;
-   if (!sas_device_priv_data-block)
-   continue;
-   sas_device_priv_data-block = 0;
-   dewtprintk(ioc, sdev_printk(KERN_INFO, sdev, device_running, 
-   handle(0x%04x)\n,
-   sas_device_priv_data-sas_target-handle));
-   scsi_internal_device_unblock(sdev, SDEV_RUNNING);
-   }
-}
-/**
  * _scsih_ublock_io_device - set the device state to SDEV_RUNNING
  * @ioc: per adapter object
  * @handle: device handle
@@ -2971,34 +2946,6 @@ _scsih_ublock_io_device(struct MPT2SAS_ADAPTER *ioc, u64 
sas_address)
 }
 
 /**
- * _scsih_block_io_all_device - set the device state to SDEV_BLOCK
- * @ioc: per adapter object
- * @handle: device handle
- *
- * During device pull we need to appropiately set the sdev state.
- */
-static void
-_scsih_block_io_all_device(struct MPT2SAS_ADAPTER *ioc)
-{
-   struct MPT2SAS_DEVICE *sas_device_priv_data;
-   struct scsi_device *sdev;
-
-   shost_for_each_device(sdev, ioc-shost) {
-   sas_device_priv_data = sdev-hostdata;
-   if (!sas_device_priv_data)
-   continue;
-   if (sas_device_priv_data-block)
-   continue;
-   sas_device_priv_data-block = 1;
-   dewtprintk(ioc, sdev_printk(KERN_INFO, sdev, device_blocked, 
-   handle(0x%04x)\n,
-   sas_device_priv_data-sas_target-handle));
-   scsi_internal_device_block(sdev);
-   }
-}
-
-
-/**
  * _scsih_block_io_device - set the device state to SDEV_BLOCK
  * @ioc: per adapter object
  * @handle: device handle
@@ -5829,166 +5776,6 @@ _scsih_sas_enclosure_dev_status_change_event(struct 
MPT2SAS_ADAPTER *ioc,
 }
 
 /**
- * _scsih_sas_broadcast_primitive_event - handle broadcast events
- * @ioc: per adapter object
- * @fw_event: The fw_event_work object
- * Context: user.
- *
- * Return nothing.
- */
-static void
-_scsih_sas_broadcast_primitive_event(struct MPT2SAS_ADAPTER *ioc,
-struct fw_event_work *fw_event)
-{
-   struct scsi_cmnd *scmd;
-   unsigned long ser_no;
-   struct scsi_device *sdev;
-   u16 smid, handle;
-   u32 lun;
-   struct MPT2SAS_DEVICE *sas_device_priv_data;
-   u32 termination_count;
-   u32 query_count;
-   Mpi2SCSITaskManagementReply_t *mpi_reply;
-   Mpi2EventDataSasBroadcastPrimitive_t *event_data = fw_event-event_data;
-   u16 ioc_status;
-   unsigned long flags;
-   int r;
-   u8 max_retries = 0;
-   u8 task_abort_retries;
-
-   mutex_lock(ioc-tm_cmds.mutex);
-   dewtprintk(ioc, 

Re: [PATCH] mpt2sas: don't handle broadcast primitives

2013-07-19 Thread Jörn Engel
On Fri, 19 July 2013 18:06:59 -0400, Jörn Engel wrote:
 
 The handling of broadcast primitives involves
 _scsih_block_io_all_device(), which does what the name implies.  I have
 observed cases with 60s of blocking io on all devices, caused by a
 single bad device.  The downsides of this code are obvious, while the
 upsides are more elusive.

And since this patch looks more like an April fools joke: I have
gathered a few machine-months of testing, including tortures that
specifically stress the removed codepaths.  This is a serious
submission and unless someone can show me a _very_ good reason for
keeping the deleted code, I would like to get it merged.

Jörn

--
Computer system analysis is like child-rearing; you can do grievous damage,
but you cannot ensure success.
-- Tom DeMarco
--
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 v2 2/3] ufs: don't disable_irq() if the IRQ can be shared among devices

2013-07-19 Thread Akinobu Mita
When removing the UFS driver, disable_irq() is called and the IRQ is
not enabled again.  Unfortunately, the IRQ is requested with IRQF_SHARED
and it can be shared among several devices.  So disabling the IRQ in
this way is just breaking other devices which are sharing the IRQ.

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd-pci.c| 1 -
 drivers/scsi/ufs/ufshcd-pltfrm.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index de4c52b..2349c0e 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -91,7 +91,6 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
 {
struct ufs_hba *hba = pci_get_drvdata(pdev);
 
-   disable_irq(pdev-irq);
ufshcd_remove(hba);
pci_set_drvdata(pdev, NULL);
 }
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index c42db40..94ba40c 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -144,7 +144,6 @@ static int ufshcd_pltfrm_remove(struct platform_device 
*pdev)
 {
struct ufs_hba *hba =  platform_get_drvdata(pdev);
 
-   disable_irq(hba-irq);
ufshcd_remove(hba);
return 0;
 }
-- 
1.8.3.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 v2 3/3] ufs: don't stop controller before scsi_remove_host()

2013-07-19 Thread Akinobu Mita
scsi_remove_host() sends SYNCHRONIZE CACHE commands for write cache
enabled scsi disk devices.  So stopping controller working shouldn't
be done before scsi_remove_host().

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b743bd6..bd4d418 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1657,11 +1657,11 @@ EXPORT_SYMBOL_GPL(ufshcd_resume);
  */
 void ufshcd_remove(struct ufs_hba *hba)
 {
+   scsi_remove_host(hba-host);
/* disable interrupts */
ufshcd_disable_intr(hba, hba-intr_mask);
ufshcd_hba_stop(hba);
 
-   scsi_remove_host(hba-host);
scsi_host_put(hba-host);
 }
 EXPORT_SYMBOL_GPL(ufshcd_remove);
-- 
1.8.3.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 v2 1/3] ufshcd-pci: release ioremapped region during removing driver

2013-07-19 Thread Akinobu Mita
Before commit 2953f850c3b80bdca004967c83733365d8aa0aa2 ([SCSI] ufs:
use devres functions for ufshcd), UFSHCI register was ioremapped by
each glue-driver (ufshcd-pltfrm and ufshcd-pci) during probing and it
was iounmapped by core-driver during removing driver.  The commit
converted ufshcd-pltfrm to use devres functions, but it didn't convert
ufshcd-pci.

Therefore, the change causes ufshcd-pci driver not to iounmap UFSHCI
register region during removing driver.  This fixes it by converting
ufshcd-pci to use devres functions.

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Seungwon Jeon tgih@samsung.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd-pci.c | 37 +
 1 file changed, 9 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 48be39a..de4c52b 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -93,10 +93,7 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
 
disable_irq(pdev-irq);
ufshcd_remove(hba);
-   pci_release_regions(pdev);
pci_set_drvdata(pdev, NULL);
-   pci_clear_master(pdev);
-   pci_disable_device(pdev);
 }
 
 /**
@@ -133,53 +130,37 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *id)
void __iomem *mmio_base;
int err;
 
-   err = pci_enable_device(pdev);
+   err = pcim_enable_device(pdev);
if (err) {
-   dev_err(pdev-dev, pci_enable_device failed\n);
-   goto out_error;
+   dev_err(pdev-dev, pcim_enable_device failed\n);
+   return err;
}
 
pci_set_master(pdev);
 
-
-   err = pci_request_regions(pdev, UFSHCD);
+   err = pcim_iomap_regions(pdev, 1  0, UFSHCD);
if (err  0) {
-   dev_err(pdev-dev, request regions failed\n);
-   goto out_disable;
+   dev_err(pdev-dev, request and iomap failed\n);
+   return err;
}
 
-   mmio_base = pci_ioremap_bar(pdev, 0);
-   if (!mmio_base) {
-   dev_err(pdev-dev, memory map failed\n);
-   err = -ENOMEM;
-   goto out_release_regions;
-   }
+   mmio_base = pcim_iomap_table(pdev)[0];
 
err = ufshcd_set_dma_mask(pdev);
if (err) {
dev_err(pdev-dev, set dma mask failed\n);
-   goto out_iounmap;
+   return err;
}
 
err = ufshcd_init(pdev-dev, hba, mmio_base, pdev-irq);
if (err) {
dev_err(pdev-dev, Initialization failed\n);
-   goto out_iounmap;
+   return err;
}
 
pci_set_drvdata(pdev, hba);
 
return 0;
-
-out_iounmap:
-   iounmap(mmio_base);
-out_release_regions:
-   pci_release_regions(pdev);
-out_disable:
-   pci_clear_master(pdev);
-   pci_disable_device(pdev);
-out_error:
-   return err;
 }
 
 static DEFINE_PCI_DEVICE_TABLE(ufshcd_pci_tbl) = {
-- 
1.8.3.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 v2 0/3] ufs: fix bugs in probing and removing driver paths

2013-07-19 Thread Akinobu Mita
The changes in this patch set are almost same as the previous one
that I send on Jul 8.  But the previous version wasn't cleanly applied
to the upstream kernel because it depend on my local work in progress
patches.  So this version fixes it and it also includes another bug fix
in the same area.

Akinobu Mita (3):
  ufshcd-pci: release ioremapped region during removing driver
  ufs: don't disable_irq() if the IRQ can be shared among devices
  ufs: don't stop controller before scsi_remove_host()

 drivers/scsi/ufs/ufshcd-pci.c| 38 +-
 drivers/scsi/ufs/ufshcd-pltfrm.c |  1 -
 drivers/scsi/ufs/ufshcd.c|  2 +-
 3 files changed, 10 insertions(+), 31 deletions(-)

Cc: Seungwon Jeon tgih@samsung.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org

-- 
1.8.3.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 RESEND 0/1] AHCI: Optimize interrupt processing

2013-07-19 Thread Nicholas A. Bellinger
On Fri, 2013-07-19 at 14:01 -0700, Nicholas A. Bellinger wrote:
 On Fri, 2013-07-19 at 08:33 -0700, James Bottomley wrote:
  On Thu, 2013-07-18 at 23:34 -0700, Nicholas A. Bellinger wrote:
   diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
   index 0101af5..191bc15 100644
   --- a/drivers/ata/libata-scsi.c
   +++ b/drivers/ata/libata-scsi.c
   @@ -1144,7 +1144,11 @@ static int ata_scsi_dev_config(struct scsi_device 
   *sdev,
   sector_size=%u  PAGE_SIZE, PIO may 
   malfunction\n,
   sdev-sector_size);

   -   blk_queue_update_dma_alignment(q, sdev-sector_size - 1);
   +   if (!q-mq_ops) {
   +   blk_queue_update_dma_alignment(q, sdev-sector_size - 1);
   +   } else {
   +   printk(Skipping dma_alignment for libata w/ scsi-mq\n);
   +   }
  
  Amazingly enough there is a reason for the dma alignment, and it wasn't
  just to annoy you, so you can't blindly do this.
  
  The email thread is probably lost in the mists of time, but if I
  remember correctly the problem is that some ahci DMA controllers barf if
  the sector they're doing DMA on crosses a page boundary.  Some are
  annoying enough to actually cause silent data corruption.  You won't
  find every ahci DMA controller doing this, so the change will work for
  some, but it will be hard to identify those it won't work for until
  people start losing data.
 
 Thanks for the extra background.
 
 So at least from what I gather thus far this shouldn't be an issue for
 initial testing with scsi-mq - libata w/ ata_piix.
 
  
  The correct fix, obviously, is to do the bio copy on the kernel path for
  unaligned data.  It is OK to assume that REQ_TYPE_FS data is correctly
  aligned (because of the block to page alignment).
  
 
 Indeed.  Looking into the bio_copy_kern() breakage next..
 

OK, after further investigation the root cause is a actually a missing
bio-bio_end_io() - bio_copy_kern_endio() - bio_put() from the
blk_end_sync_rq() callback path that scsi-mq REQ_TYPE_BLOCK_PC is
currently using.

Including the following patch into the scsi-mq working branch now, and
reverting the libata dma_alignment=0x03 hack.

Alexander, care to give this a try..?

--nab

diff --git a/block/blk-exec.c b/block/blk-exec.c
index 0761c89..70303d2 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -25,7 +25,10 @@ static void blk_end_sync_rq(struct request *rq, int error)
struct completion *waiting = rq-end_io_data;
 
rq-end_io_data = NULL;
-   if (!rq-q-mq_ops) {
+   if (rq-q-mq_ops) {
+   if (rq-bio)
+   bio_endio(rq-bio, error);
+   } else {
__blk_put_request(rq-q, rq);
}

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