Re: linux-next: build failure after merge of the scsi tree

2013-01-11 Thread wenxiong


Quoting James Bottomley :


On Fri, 2013-01-11 at 11:35 -0600, Brian King wrote:

On 01/11/2013 10:05 AM, Greg KH wrote:
> On Fri, Jan 11, 2013 at 03:37:17PM +, James Bottomley wrote:
>> On Fri, 2013-01-11 at 09:27 -0600, Brian King wrote:
>>> It looks like this was a due to the fact that the new patches
>>> added __devinit tags in the same merge window the __devinit tag
>>> itself was getting removed.
>>
>> Not exactly.  The patch which makes them nops went into 3.8.  Now
>> there's a patch queued in, Gregs tree I presume, to remove them all and
>> the #defines which causes the compile failure.
>>
>>> As to the sparse warnings, this patch fixed the ones that
>>> were actual bugs in the new code, although we could have
>>> made that more clear in the patch description.
>>>
>>> http://marc.info/?l=linux-scsi&m=135716576204083&w=2
>>
>> Ah, thanks ... I've been on holiday for a while, so I did miss that.
>>
>>> There is one outstanding issue I am aware of which was an
>>> array bounds compiler warning which looks to be a misdetection
>>> by the compiler. Wendy and I discussed adding a BUG_ON
>>> to stop the compiler from complaining.
>>>
>>> Wendy - lets queue these two changes up ASAP. They should both
>>> be very simple changes.
>>
>> If it's a simple gcc bug, just ignore it.
>>
>> I do need you to redo the patches to remove the __dev annotations,
>> though.  We can't risk introducing a bisect killing compile breakage if
>> Greg's tree merges before mine in the next merge window.
>
> This change should be pushed to Linus in time for 3.8-final, so there
> should not be any bisect issues.

We can do this either way.

James - what is your preference? Drop everything and do a resend of the
entire series or delta patches on top of what is currently in your tree?


Drop everything and resend still, I think.  There's still a rebase
problem, because the merge failure will happen if I rebase the misc tree
to beyond Greg's merge point and I'd rather not have to worry about it.

Thanks,

James


Hi James, I just re-sent all patches which are against
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git.
This kernel already have Greg's "Drivers: scsi: remove _dev*  
attributes" patch.


Let me know if you have any questions for these patches.

Thanks for for your help!
Wendy

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/8] V2 ipr: Implement block iopoll

2013-01-11 Thread Asias He
Hello Wen Xiong,

On Sat, Jan 12, 2013 at 7:43 AM,   wrote:
> This patch implements blk iopoll in ipr driver for performance improvement.

Can you provide the performance numbers with/without the io polling?
It would be interesting to know.

> Signed-off-by: Wen Xiong 
> ---
>  drivers/scsi/ipr.c |  221 
> +
>  drivers/scsi/ipr.h |6 +
>  2 files changed, 178 insertions(+), 49 deletions(-)
>
> Index: b/drivers/scsi/ipr.c
> ===
> --- a/drivers/scsi/ipr.c2013-01-11 16:11:20.062476228 -0600
> +++ b/drivers/scsi/ipr.c2013-01-11 16:11:50.275910225 -0600
> @@ -108,6 +108,7 @@ static const struct ipr_chip_cfg_t ipr_c
> .max_cmds = 100,
> .cache_line_size = 0x20,
> .clear_isr = 1,
> +   .iopoll_weight = 0,
> {
> .set_interrupt_mask_reg = 0x0022C,
> .clr_interrupt_mask_reg = 0x00230,
> @@ -132,6 +133,7 @@ static const struct ipr_chip_cfg_t ipr_c
> .max_cmds = 100,
> .cache_line_size = 0x20,
> .clear_isr = 1,
> +   .iopoll_weight = 0,
> {
> .set_interrupt_mask_reg = 0x00288,
> .clr_interrupt_mask_reg = 0x0028C,
> @@ -156,6 +158,7 @@ static const struct ipr_chip_cfg_t ipr_c
> .max_cmds = 1000,
> .cache_line_size = 0x20,
> .clear_isr = 0,
> +   .iopoll_weight = 64,
> {
> .set_interrupt_mask_reg = 0x00010,
> .clr_interrupt_mask_reg = 0x00018,
> @@ -3560,6 +3563,95 @@ static struct device_attribute ipr_ioa_r
> .store = ipr_store_reset_adapter
>  };
>
> +static int ipr_iopoll(struct blk_iopoll *iop, int budget);
> + /**
> + * ipr_show_iopoll_weight - Show ipr polling mode
> + * @dev:   class device struct
> + * @buf:   buffer
> + *
> + * Return value:
> + * number of bytes printed to buffer
> + **/
> +static ssize_t ipr_show_iopoll_weight(struct device *dev,
> +  struct device_attribute *attr, char *buf)
> +{
> +   struct Scsi_Host *shost = class_to_shost(dev);
> +   struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)shost->hostdata;
> +   unsigned long lock_flags = 0;
> +   int len;
> +
> +   spin_lock_irqsave(shost->host_lock, lock_flags);
> +   len = snprintf(buf, PAGE_SIZE, "%d\n", ioa_cfg->iopoll_weight);
> +   spin_unlock_irqrestore(shost->host_lock, lock_flags);
> +
> +   return len;
> +}
> +
> +/**
> + * ipr_store_iopoll_weight - Change the adapter's polling mode
> + * @dev:   class device struct
> + * @buf:   buffer
> + *
> + * Return value:
> + * number of bytes printed to buffer
> + **/
> +static ssize_t ipr_store_iopoll_weight(struct device *dev,
> +   struct device_attribute *attr,
> +   const char *buf, size_t count)
> +{
> +   struct Scsi_Host *shost = class_to_shost(dev);
> +   struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)shost->hostdata;
> +   unsigned long user_iopoll_weight;
> +   unsigned long lock_flags = 0;
> +   int i;
> +
> +   if (!ioa_cfg->sis64) {
> +   dev_info(&ioa_cfg->pdev->dev, "blk-iopoll not supported on 
> this adapter\n");
> +   return -EINVAL;
> +   }
> +   if (kstrtoul(buf, 10, &user_iopoll_weight))
> +   return -EINVAL;
> +
> +   if (user_iopoll_weight > 256) {
> +   dev_info(&ioa_cfg->pdev->dev, "Invalid blk-iopoll weight. It 
> must be less than 256\n");
> +   return -EINVAL;
> +   }
> +
> +   if (user_iopoll_weight == ioa_cfg->iopoll_weight) {
> +   dev_info(&ioa_cfg->pdev->dev, "Current blk-iopoll weight has 
> the same weight\n");
> +   return strlen(buf);
> +   }
> +
> +   if (blk_iopoll_enabled && ioa_cfg->iopoll_weight &&
> +   ioa_cfg->sis64 && ioa_cfg->nvectors > 1) {
> +   for (i = 1; i < ioa_cfg->hrrq_num; i++)
> +   blk_iopoll_disable(&ioa_cfg->hrrq[i].iopoll);
> +   }
> +
> +   spin_lock_irqsave(shost->host_lock, lock_flags);
> +   ioa_cfg->iopoll_weight = user_iopoll_weight;
> +   if (blk_iopoll_enabled && ioa_cfg->iopoll_weight &&
> +   ioa_cfg->sis64 && ioa_cfg->nvectors > 1) {
> +   for (i = 1; i < ioa_cfg->hrrq_num; i++) {
> +   blk_iopoll_init(&ioa_cfg->hrrq[i].iopoll,
> +   ioa_cfg->iopoll_weight, ipr_iopoll);
> +   blk_iopoll_enable(&ioa_cfg->hrrq[i].iopoll);
> +   }
> +   }
> +   spin_unlock_irqrestore(shost->host_lock, lock_flags);
> +
> +   return strlen(buf)

[PATCH 4/8] V2 ipr: Add support for MSI-X and distributed completion

2013-01-11 Thread wenxiong
The new generation IBM SAS Controllers will support MSI-X interrupts and
Distributed Completion Processing features. This patch add these support
in ipr device driver.

Signed-off-by: Wen Xiong 
---
 drivers/scsi/ipr.c |  718 -
 drivers/scsi/ipr.h |   70 +++--
 2 files changed, 595 insertions(+), 193 deletions(-)

Index: b/drivers/scsi/ipr.c
===
--- a/drivers/scsi/ipr.c2013-01-11 13:31:35.105915246 -0600
+++ b/drivers/scsi/ipr.c2013-01-11 16:13:41.442476346 -0600
@@ -98,6 +98,7 @@ static unsigned int ipr_transop_timeout 
 static unsigned int ipr_debug = 0;
 static unsigned int ipr_max_devs = IPR_DEFAULT_SIS64_DEVS;
 static unsigned int ipr_dual_ioa_raid = 1;
+static unsigned int ipr_number_of_msix = 2;
 static DEFINE_SPINLOCK(ipr_driver_lock);
 
 /* This table describes the differences between DMA controller chips */
@@ -215,6 +216,8 @@ MODULE_PARM_DESC(dual_ioa_raid, "Enable 
 module_param_named(max_devs, ipr_max_devs, int, 0);
 MODULE_PARM_DESC(max_devs, "Specify the maximum number of physical devices. "
 "[Default=" __stringify(IPR_DEFAULT_SIS64_DEVS) "]");
+module_param_named(number_of_msix, ipr_number_of_msix, int, 0);
+MODULE_PARM_DESC(number_of_msix, "Specify the number of MSIX interrupts to use 
on capable adapters (1 - 5).  (default:2)");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(IPR_DRIVER_VERSION);
 
@@ -595,8 +598,11 @@ static void ipr_reinit_ipr_cmnd(struct i
struct ipr_ioasa *ioasa = &ipr_cmd->s.ioasa;
struct ipr_ioasa64 *ioasa64 = &ipr_cmd->s.ioasa64;
dma_addr_t dma_addr = ipr_cmd->dma_addr;
+   int hrrq_id;
 
+   hrrq_id = ioarcb->cmd_pkt.hrrq_id;
memset(&ioarcb->cmd_pkt, 0, sizeof(struct ipr_cmd_pkt));
+   ioarcb->cmd_pkt.hrrq_id = hrrq_id;
ioarcb->data_transfer_length = 0;
ioarcb->read_data_transfer_length = 0;
ioarcb->ioadl_len = 0;
@@ -646,12 +652,16 @@ static void ipr_init_ipr_cmnd(struct ipr
  * pointer to ipr command struct
  **/
 static
-struct ipr_cmnd *__ipr_get_free_ipr_cmnd(struct ipr_ioa_cfg *ioa_cfg)
+struct ipr_cmnd *__ipr_get_free_ipr_cmnd(struct ipr_hrr_queue *hrrq)
 {
-   struct ipr_cmnd *ipr_cmd;
+   struct ipr_cmnd *ipr_cmd = NULL;
+
+   if (likely(!list_empty(&hrrq->hrrq_free_q))) {
+   ipr_cmd = list_entry(hrrq->hrrq_free_q.next,
+   struct ipr_cmnd, queue);
+   list_del(&ipr_cmd->queue);
+   }
 
-   ipr_cmd = list_entry(ioa_cfg->free_q.next, struct ipr_cmnd, queue);
-   list_del(&ipr_cmd->queue);
 
return ipr_cmd;
 }
@@ -666,7 +676,8 @@ struct ipr_cmnd *__ipr_get_free_ipr_cmnd
 static
 struct ipr_cmnd *ipr_get_free_ipr_cmnd(struct ipr_ioa_cfg *ioa_cfg)
 {
-   struct ipr_cmnd *ipr_cmd = __ipr_get_free_ipr_cmnd(ioa_cfg);
+   struct ipr_cmnd *ipr_cmd =
+   __ipr_get_free_ipr_cmnd(&ioa_cfg->hrrq[IPR_INIT_HRRQ]);
ipr_init_ipr_cmnd(ipr_cmd, ipr_lock_and_done);
return ipr_cmd;
 }
@@ -761,13 +772,12 @@ static int ipr_set_pcix_cmd_reg(struct i
  **/
 static void ipr_sata_eh_done(struct ipr_cmnd *ipr_cmd)
 {
-   struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg;
struct ata_queued_cmd *qc = ipr_cmd->qc;
struct ipr_sata_port *sata_port = qc->ap->private_data;
 
qc->err_mask |= AC_ERR_OTHER;
sata_port->ioasa.status |= ATA_BUSY;
-   list_add_tail(&ipr_cmd->queue, &ioa_cfg->free_q);
+   list_add_tail(&ipr_cmd->queue, &ipr_cmd->hrrq->hrrq_free_q);
ata_qc_complete(qc);
 }
 
@@ -783,14 +793,13 @@ static void ipr_sata_eh_done(struct ipr_
  **/
 static void ipr_scsi_eh_done(struct ipr_cmnd *ipr_cmd)
 {
-   struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg;
struct scsi_cmnd *scsi_cmd = ipr_cmd->scsi_cmd;
 
scsi_cmd->result |= (DID_ERROR << 16);
 
scsi_dma_unmap(ipr_cmd->scsi_cmd);
scsi_cmd->scsi_done(scsi_cmd);
-   list_add_tail(&ipr_cmd->queue, &ioa_cfg->free_q);
+   list_add_tail(&ipr_cmd->queue, &ipr_cmd->hrrq->hrrq_free_q);
 }
 
 /**
@@ -805,24 +814,30 @@ static void ipr_scsi_eh_done(struct ipr_
 static void ipr_fail_all_ops(struct ipr_ioa_cfg *ioa_cfg)
 {
struct ipr_cmnd *ipr_cmd, *temp;
+   struct ipr_hrr_queue *hrrq;
 
ENTER;
-   list_for_each_entry_safe(ipr_cmd, temp, &ioa_cfg->pending_q, queue) {
-   list_del(&ipr_cmd->queue);
+   for_each_hrrq(hrrq, ioa_cfg) {
+   list_for_each_entry_safe(ipr_cmd,
+   temp, &hrrq->hrrq_pending_q, queue) {
+   list_del(&ipr_cmd->queue);
+
+   ipr_cmd->s.ioasa.hdr.ioasc =
+   cpu_to_be32(IPR_IOASC_IOA_WAS_RESET);
+   ipr_cmd->s.ioasa.hdr.ilid =
+   cpu_to_be32(IPR_DRIVER_ILID);
 
-   ipr_cmd->s.ioasa.hdr.ioasc = 
cpu_to_be3

[PATCH 3/8] V2 ipr: Resource path error logging cleanup

2013-01-11 Thread wenxiong
From: Brian King 

The resource path as displayed by the ipr driver is the
location string identifying a location on the SAS fabric.
This patch adds the SCSI host number such that error logs
can be more easily correlated in multiple adapter configurations.

Signed-off-by: Brian King 
Signed-off-by: Wen Xiong 
---

 drivers/scsi/ipr.c |   78 -
 drivers/scsi/ipr.h |5 ++-
 2 files changed, 56 insertions(+), 27 deletions(-)

Index: b/drivers/scsi/ipr.h
===
--- a/drivers/scsi/ipr.h2013-01-11 13:30:56.053727535 -0600
+++ b/drivers/scsi/ipr.h2013-01-11 13:31:35.095911184 -0600
@@ -409,7 +409,7 @@ struct ipr_config_table_entry64 {
__be64 dev_id;
__be64 lun;
__be64 lun_wwn[2];
-#define IPR_MAX_RES_PATH_LENGTH24
+#define IPR_MAX_RES_PATH_LENGTH48
__be64 res_path;
struct ipr_std_inq_data std_inq_data;
u8 reserved2[4];
@@ -1722,7 +1722,8 @@ struct ipr_ucode_image_header {
if (ipr_is_device(hostrcb)) {   \
if ((hostrcb)->ioa_cfg->sis64) {\
printk(KERN_ERR IPR_NAME ": %s: " fmt,  \
-   
ipr_format_res_path(hostrcb->hcam.u.error64.fd_res_path, \
+   ipr_format_res_path(hostrcb->ioa_cfg,   \
+   hostrcb->hcam.u.error64.fd_res_path, \
hostrcb->rp_buffer, \
sizeof(hostrcb->rp_buffer)),\
__VA_ARGS__);   \
Index: b/drivers/scsi/ipr.c
===
--- a/drivers/scsi/ipr.c2013-01-11 13:31:21.703731278 -0600
+++ b/drivers/scsi/ipr.c2013-01-11 13:31:35.105915246 -0600
@@ -1166,14 +1166,15 @@ static int ipr_is_same_device(struct ipr
 }
 
 /**
- * ipr_format_res_path - Format the resource path for printing.
+ * __ipr_format_res_path - Format the resource path for printing.
  * @res_path:  resource path
  * @buf:   buffer
+ * @len:   length of buffer provided
  *
  * Return value:
  * pointer to buffer
  **/
-static char *ipr_format_res_path(u8 *res_path, char *buffer, int len)
+static char *__ipr_format_res_path(u8 *res_path, char *buffer, int len)
 {
int i;
char *p = buffer;
@@ -1187,6 +1188,27 @@ static char *ipr_format_res_path(u8 *res
 }
 
 /**
+ * ipr_format_res_path - Format the resource path for printing.
+ * @ioa_cfg:   ioa config struct
+ * @res_path:  resource path
+ * @buf:   buffer
+ * @len:   length of buffer provided
+ *
+ * Return value:
+ * pointer to buffer
+ **/
+static char *ipr_format_res_path(struct ipr_ioa_cfg *ioa_cfg,
+u8 *res_path, char *buffer, int len)
+{
+   char *p = buffer;
+
+   *p = '\0';
+   p += snprintf(p, buffer + len - p, "%d/", ioa_cfg->host->host_no);
+   __ipr_format_res_path(res_path, p, len - (buffer - p));
+   return buffer;
+}
+
+/**
  * ipr_update_res_entry - Update the resource entry.
  * @res:   resource entry struct
  * @cfgtew:config table entry wrapper struct
@@ -1226,8 +1248,8 @@ static void ipr_update_res_entry(struct 
 
if (res->sdev && new_path)
sdev_printk(KERN_INFO, res->sdev, "Resource path: %s\n",
-   ipr_format_res_path(res->res_path, buffer,
-   sizeof(buffer)));
+   ipr_format_res_path(res->ioa_cfg,
+   res->res_path, buffer, sizeof(buffer)));
} else {
res->flags = cfgtew->u.cfgte->flags;
if (res->flags & IPR_IS_IOA_RESOURCE)
@@ -1613,8 +1635,8 @@ static void ipr_log_sis64_config_error(s
ipr_err_separator;
 
ipr_err("Device %d : %s", i + 1,
-ipr_format_res_path(dev_entry->res_path, buffer,
-sizeof(buffer)));
+   __ipr_format_res_path(dev_entry->res_path,
+ buffer, sizeof(buffer)));
ipr_log_ext_vpd(&dev_entry->vpd);
 
ipr_err("-New Device Information-\n");
@@ -1960,14 +1982,16 @@ static void ipr_log64_fabric_path(struct
 
ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s\n",
 path_active_desc[i].desc, 
path_state_desc[j].desc,
-ipr_format_res_path(fabric->res_path, 
buffer,
-sizeof(buffer)));
+ipr_format_res_path(hostrcb->ioa_cfg,
+   

[PATCH 5/8] V2 ipr: Reduce lock contention

2013-01-11 Thread wenxiong
This patch reduces lock contention while implementing distributed 
completion processing. 

Signed-off-by: Wen Xiong 
---
 drivers/scsi/ipr.c |  323 +
 drivers/scsi/ipr.h |   21 +--
 2 files changed, 240 insertions(+), 104 deletions(-)

Index: b/drivers/scsi/ipr.c
===
--- a/drivers/scsi/ipr.c2013-01-11 13:40:15.992165067 -0600
+++ b/drivers/scsi/ipr.c2013-01-11 16:11:20.062476228 -0600
@@ -552,7 +552,8 @@ static void ipr_trc_hook(struct ipr_cmnd
struct ipr_trace_entry *trace_entry;
struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg;
 
-   trace_entry = &ioa_cfg->trace[ioa_cfg->trace_index++];
+   trace_entry = &ioa_cfg->trace[atomic_add_return
+   (1, &ioa_cfg->trace_index)%IPR_NUM_TRACE_ENTRIES];
trace_entry->time = jiffies;
trace_entry->op_code = ipr_cmd->ioarcb.cmd_pkt.cdb[0];
trace_entry->type = type;
@@ -563,6 +564,7 @@ static void ipr_trc_hook(struct ipr_cmnd
trace_entry->cmd_index = ipr_cmd->cmd_index & 0xff;
trace_entry->res_handle = ipr_cmd->ioarcb.res_handle;
trace_entry->u.add_data = add_data;
+   wmb();
 }
 #else
 #define ipr_trc_hook(ipr_cmd, type, add_data) do { } while (0)
@@ -697,9 +699,15 @@ static void ipr_mask_and_clear_interrupt
  u32 clr_ints)
 {
volatile u32 int_reg;
+   int i;
 
/* Stop new interrupts */
-   ioa_cfg->allow_interrupts = 0;
+   for (i = 0; i < ioa_cfg->hrrq_num; i++) {
+   spin_lock(&ioa_cfg->hrrq[i]._lock);
+   ioa_cfg->hrrq[i].allow_interrupts = 0;
+   spin_unlock(&ioa_cfg->hrrq[i]._lock);
+   }
+   wmb();
 
/* Set interrupt mask to stop all new interrupts */
if (ioa_cfg->sis64)
@@ -818,6 +826,7 @@ static void ipr_fail_all_ops(struct ipr_
 
ENTER;
for_each_hrrq(hrrq, ioa_cfg) {
+   spin_lock(&hrrq->_lock);
list_for_each_entry_safe(ipr_cmd,
temp, &hrrq->hrrq_pending_q, queue) {
list_del(&ipr_cmd->queue);
@@ -837,6 +846,7 @@ static void ipr_fail_all_ops(struct ipr_
del_timer(&ipr_cmd->timer);
ipr_cmd->done(ipr_cmd);
}
+   spin_unlock(&hrrq->_lock);
}
LEAVE;
 }
@@ -991,12 +1001,9 @@ static void ipr_send_blocking_cmd(struct
 static int ipr_get_hrrq_index(struct ipr_ioa_cfg *ioa_cfg)
 {
if (ioa_cfg->hrrq_num == 1)
-   ioa_cfg->hrrq_index = 0;
-   else {
-   if (++ioa_cfg->hrrq_index >= ioa_cfg->hrrq_num)
-   ioa_cfg->hrrq_index = 1;
-   }
-   return ioa_cfg->hrrq_index;
+   return 0;
+   else
+   return (atomic_add_return(1, &ioa_cfg->hrrq_index) % 
(ioa_cfg->hrrq_num - 1)) + 1;
 }
 
 /**
@@ -1018,7 +1025,7 @@ static void ipr_send_hcam(struct ipr_ioa
struct ipr_cmnd *ipr_cmd;
struct ipr_ioarcb *ioarcb;
 
-   if (ioa_cfg->allow_cmds) {
+   if (ioa_cfg->hrrq[IPR_INIT_HRRQ].allow_cmds) {
ipr_cmd = ipr_get_free_ipr_cmnd(ioa_cfg);
list_add_tail(&ipr_cmd->queue, &ipr_cmd->hrrq->hrrq_pending_q);
list_add_tail(&hostrcb->queue, &ioa_cfg->hostrcb_pending_q);
@@ -2564,7 +2571,7 @@ static int ipr_reset_reload(struct ipr_i
 
/* If we got hit with a host reset while we were already resetting
 the adapter for some reason, and the reset failed. */
-   if (ioa_cfg->ioa_is_dead) {
+   if (ioa_cfg->hrrq[IPR_INIT_HRRQ].ioa_is_dead) {
ipr_trace;
return FAILED;
}
@@ -3205,7 +3212,8 @@ static void ipr_worker_thread(struct wor
 restart:
do {
did_work = 0;
-   if (!ioa_cfg->allow_cmds || !ioa_cfg->allow_ml_add_del) {
+   if (!ioa_cfg->hrrq[IPR_INIT_HRRQ].allow_cmds ||
+   !ioa_cfg->allow_ml_add_del) {
spin_unlock_irqrestore(ioa_cfg->host->host_lock, 
lock_flags);
return;
}
@@ -3453,7 +3461,7 @@ static ssize_t ipr_show_adapter_state(st
int len;
 
spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
-   if (ioa_cfg->ioa_is_dead)
+   if (ioa_cfg->hrrq[IPR_INIT_HRRQ].ioa_is_dead)
len = snprintf(buf, PAGE_SIZE, "offline\n");
else
len = snprintf(buf, PAGE_SIZE, "online\n");
@@ -3479,14 +3487,20 @@ static ssize_t ipr_store_adapter_state(s
struct Scsi_Host *shost = class_to_shost(dev);
struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)shost->hostdata;
unsigned long lock_flags;
-   int result = count;
+   int result = count, i;
 
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
 
spin_lo

[PATCH 7/8] V2 ipr: Driver version 2.6.0

2013-01-11 Thread wenxiong
Bump driver version.

Signed-off-by: Wen Xiong 
---
 drivers/scsi/ipr.h |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: b/drivers/scsi/ipr.h
===
--- a/drivers/scsi/ipr.h2013-01-11 16:11:50.275910225 -0600
+++ b/drivers/scsi/ipr.h2013-01-11 16:12:03.673719736 -0600
@@ -39,8 +39,8 @@
 /*
  * Literals
  */
-#define IPR_DRIVER_VERSION "2.5.4"
-#define IPR_DRIVER_DATE "(July 11, 2012)"
+#define IPR_DRIVER_VERSION "2.6.0"
+#define IPR_DRIVER_DATE "(November 16, 2012)"
 
 /*
  * IPR_MAX_CMD_PER_LUN: This defines the maximum number of outstanding

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 6/8] V2 ipr: Implement block iopoll

2013-01-11 Thread wenxiong
This patch implements blk iopoll in ipr driver for performance improvement.

Signed-off-by: Wen Xiong 
---
 drivers/scsi/ipr.c |  221 +
 drivers/scsi/ipr.h |6 +
 2 files changed, 178 insertions(+), 49 deletions(-)

Index: b/drivers/scsi/ipr.c
===
--- a/drivers/scsi/ipr.c2013-01-11 16:11:20.062476228 -0600
+++ b/drivers/scsi/ipr.c2013-01-11 16:11:50.275910225 -0600
@@ -108,6 +108,7 @@ static const struct ipr_chip_cfg_t ipr_c
.max_cmds = 100,
.cache_line_size = 0x20,
.clear_isr = 1,
+   .iopoll_weight = 0,
{
.set_interrupt_mask_reg = 0x0022C,
.clr_interrupt_mask_reg = 0x00230,
@@ -132,6 +133,7 @@ static const struct ipr_chip_cfg_t ipr_c
.max_cmds = 100,
.cache_line_size = 0x20,
.clear_isr = 1,
+   .iopoll_weight = 0,
{
.set_interrupt_mask_reg = 0x00288,
.clr_interrupt_mask_reg = 0x0028C,
@@ -156,6 +158,7 @@ static const struct ipr_chip_cfg_t ipr_c
.max_cmds = 1000,
.cache_line_size = 0x20,
.clear_isr = 0,
+   .iopoll_weight = 64,
{
.set_interrupt_mask_reg = 0x00010,
.clr_interrupt_mask_reg = 0x00018,
@@ -3560,6 +3563,95 @@ static struct device_attribute ipr_ioa_r
.store = ipr_store_reset_adapter
 };
 
+static int ipr_iopoll(struct blk_iopoll *iop, int budget);
+ /**
+ * ipr_show_iopoll_weight - Show ipr polling mode
+ * @dev:   class device struct
+ * @buf:   buffer
+ *
+ * Return value:
+ * number of bytes printed to buffer
+ **/
+static ssize_t ipr_show_iopoll_weight(struct device *dev,
+  struct device_attribute *attr, char *buf)
+{
+   struct Scsi_Host *shost = class_to_shost(dev);
+   struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)shost->hostdata;
+   unsigned long lock_flags = 0;
+   int len;
+
+   spin_lock_irqsave(shost->host_lock, lock_flags);
+   len = snprintf(buf, PAGE_SIZE, "%d\n", ioa_cfg->iopoll_weight);
+   spin_unlock_irqrestore(shost->host_lock, lock_flags);
+
+   return len;
+}
+
+/**
+ * ipr_store_iopoll_weight - Change the adapter's polling mode
+ * @dev:   class device struct
+ * @buf:   buffer
+ *
+ * Return value:
+ * number of bytes printed to buffer
+ **/
+static ssize_t ipr_store_iopoll_weight(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf, size_t count)
+{
+   struct Scsi_Host *shost = class_to_shost(dev);
+   struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)shost->hostdata;
+   unsigned long user_iopoll_weight;
+   unsigned long lock_flags = 0;
+   int i;
+
+   if (!ioa_cfg->sis64) {
+   dev_info(&ioa_cfg->pdev->dev, "blk-iopoll not supported on this 
adapter\n");
+   return -EINVAL;
+   }
+   if (kstrtoul(buf, 10, &user_iopoll_weight))
+   return -EINVAL;
+
+   if (user_iopoll_weight > 256) {
+   dev_info(&ioa_cfg->pdev->dev, "Invalid blk-iopoll weight. It 
must be less than 256\n");
+   return -EINVAL;
+   }
+
+   if (user_iopoll_weight == ioa_cfg->iopoll_weight) {
+   dev_info(&ioa_cfg->pdev->dev, "Current blk-iopoll weight has 
the same weight\n");
+   return strlen(buf);
+   }
+
+   if (blk_iopoll_enabled && ioa_cfg->iopoll_weight &&
+   ioa_cfg->sis64 && ioa_cfg->nvectors > 1) {
+   for (i = 1; i < ioa_cfg->hrrq_num; i++)
+   blk_iopoll_disable(&ioa_cfg->hrrq[i].iopoll);
+   }
+
+   spin_lock_irqsave(shost->host_lock, lock_flags);
+   ioa_cfg->iopoll_weight = user_iopoll_weight;
+   if (blk_iopoll_enabled && ioa_cfg->iopoll_weight &&
+   ioa_cfg->sis64 && ioa_cfg->nvectors > 1) {
+   for (i = 1; i < ioa_cfg->hrrq_num; i++) {
+   blk_iopoll_init(&ioa_cfg->hrrq[i].iopoll,
+   ioa_cfg->iopoll_weight, ipr_iopoll);
+   blk_iopoll_enable(&ioa_cfg->hrrq[i].iopoll);
+   }
+   }
+   spin_unlock_irqrestore(shost->host_lock, lock_flags);
+
+   return strlen(buf);
+}
+
+static struct device_attribute ipr_iopoll_weight_attr = {
+   .attr = {
+   .name = "iopoll_weight",
+   .mode = S_IRUGO | S_IWUSR,
+   },
+   .show = ipr_show_iopoll_weight,
+   .store = ipr_store_iopoll_weight
+};
+
 /**
  * ipr_alloc_ucode_buffer - Allocates a microcode download buffer
  * @buf_len:   buffer length
@@ -3928,6 

[PATCH 8/8] V2 ipr: Fix sparse error in ipr driver

2013-01-11 Thread wenxiong
This patch fixes the following sparse error:

 CHECK   drivers/scsi/ipr.c
spinlock.h:147:9: warning: context imbalance in 'ipr_reset_reload' - unexpected 
unlock

Signed-off-by: Wen Xiong 
---
 drivers/scsi/ipr.c |   69 +
 1 file changed, 17 insertions(+), 52 deletions(-)

Index: b/drivers/scsi/ipr.c
===
--- a/drivers/scsi/ipr.c2013-01-11 16:13:50.803418076 -0600
+++ b/drivers/scsi/ipr.c2013-01-11 16:14:53.092477895 -0600
@@ -2553,36 +2553,6 @@ static void ipr_oper_timeout(struct ipr_
 }
 
 /**
- * ipr_reset_reload - Reset/Reload the IOA
- * @ioa_cfg:   ioa config struct
- * @shutdown_type: shutdown type
- *
- * This function resets the adapter and re-initializes it.
- * This function assumes that all new host commands have been stopped.
- * Return value:
- * SUCCESS / FAILED
- **/
-static int ipr_reset_reload(struct ipr_ioa_cfg *ioa_cfg,
-   enum ipr_shutdown_type shutdown_type)
-{
-   if (!ioa_cfg->in_reset_reload)
-   ipr_initiate_ioa_reset(ioa_cfg, shutdown_type);
-
-   spin_unlock_irq(ioa_cfg->host->host_lock);
-   wait_event(ioa_cfg->reset_wait_q, !ioa_cfg->in_reset_reload);
-   spin_lock_irq(ioa_cfg->host->host_lock);
-
-   /* If we got hit with a host reset while we were already resetting
-the adapter for some reason, and the reset failed. */
-   if (ioa_cfg->hrrq[IPR_INIT_HRRQ].ioa_is_dead) {
-   ipr_trace;
-   return FAILED;
-   }
-
-   return SUCCESS;
-}
-
-/**
  * ipr_find_ses_entry - Find matching SES in SES table
  * @res:   resource entry struct of SES
  *
@@ -4797,22 +4767,18 @@ static int ipr_slave_alloc(struct scsi_d
return rc;
 }
 
-/**
- * ipr_eh_host_reset - Reset the host adapter
- * @scsi_cmd:  scsi command struct
- *
- * Return value:
- * SUCCESS / FAILED
- **/
-static int __ipr_eh_host_reset(struct scsi_cmnd *scsi_cmd)
+static int ipr_eh_host_reset(struct scsi_cmnd *cmd)
 {
struct ipr_ioa_cfg *ioa_cfg;
-   int rc;
+   unsigned long lock_flags = 0;
+   int rc = SUCCESS;
 
ENTER;
-   ioa_cfg = (struct ipr_ioa_cfg *) scsi_cmd->device->host->hostdata;
+   ioa_cfg = (struct ipr_ioa_cfg *) cmd->device->host->hostdata;
+   spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
 
if (!ioa_cfg->in_reset_reload) {
+   ipr_initiate_ioa_reset(ioa_cfg, IPR_SHUTDOWN_ABBREV);
dev_err(&ioa_cfg->pdev->dev,
"Adapter being reset as a result of error recovery.\n");
 
@@ -4820,20 +4786,19 @@ static int __ipr_eh_host_reset(struct sc
ioa_cfg->sdt_state = GET_DUMP;
}
 
-   rc = ipr_reset_reload(ioa_cfg, IPR_SHUTDOWN_ABBREV);
-
-   LEAVE;
-   return rc;
-}
-
-static int ipr_eh_host_reset(struct scsi_cmnd *cmd)
-{
-   int rc;
+   spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
+   wait_event(ioa_cfg->reset_wait_q, !ioa_cfg->in_reset_reload);
+   spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
 
-   spin_lock_irq(cmd->device->host->host_lock);
-   rc = __ipr_eh_host_reset(cmd);
-   spin_unlock_irq(cmd->device->host->host_lock);
+   /* If we got hit with a host reset while we were already resetting
+the adapter for some reason, and the reset failed. */
+   if (ioa_cfg->hrrq[IPR_INIT_HRRQ].ioa_is_dead) {
+   ipr_trace;
+   rc = FAILED;
+   }
 
+   spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
+   LEAVE;
return rc;
 }
 

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/8] V2 ipr: Handler ID memory allocation failure

2013-01-11 Thread wenxiong
From: Brian King 

Add code to handle memory allocation failures at module load time.

Reported-by: Fengguang Wu 

Signed-off-by: Brian King 
Signed-off-by: Wen Xiong 
---

 drivers/scsi/ipr.c |7 +++
 1 file changed, 7 insertions(+)

Index: b/drivers/scsi/ipr.c
===
--- a/drivers/scsi/ipr.c2013-01-11 13:30:56.053727535 -0600
+++ b/drivers/scsi/ipr.c2013-01-11 13:31:21.703731278 -0600
@@ -8516,6 +8516,10 @@ static int ipr_alloc_mem(struct ipr_ioa_
 
BITS_TO_LONGS(ioa_cfg->max_devs_supported), GFP_KERNEL);
ioa_cfg->vset_ids = kzalloc(sizeof(unsigned long) *

BITS_TO_LONGS(ioa_cfg->max_devs_supported), GFP_KERNEL);
+
+   if (!ioa_cfg->target_ids || !ioa_cfg->array_ids
+   || !ioa_cfg->vset_ids)
+   goto out_free_res_entries;
}
 
for (i = 0; i < ioa_cfg->max_devs_supported; i++) {
@@ -8591,6 +8595,9 @@ out_free_vpd_cbs:
ioa_cfg->vpd_cbs, ioa_cfg->vpd_cbs_dma);
 out_free_res_entries:
kfree(ioa_cfg->res_entries);
+   kfree(ioa_cfg->target_ids);
+   kfree(ioa_cfg->array_ids);
+   kfree(ioa_cfg->vset_ids);
goto out;
 }
 

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/8] V2 Add support for new IBM SAS controllers

2013-01-11 Thread wenxiong
This is version 2 of ipr patches to support new IBM SAS controllers. 
In V2, we have fixed the following suggestions/warning/sparse errors:

1.Changed simple_strtoul() to kstrtoul() in ipr_restore_iopoll_weight routine.
2.Removed the __dev annotations.
3.Fixed unlock bugs which are caused by my previous patches(reported by sparse).
4.BUG_ON gcc 4.7 warning.
5.Fixed sparse error in original ipr driver.

Let me know if you have any questions.

Thanks for your help!
Wendy
-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/8] V2 ipr: Add sereral new CCIN definitions for new adapters support

2013-01-11 Thread wenxiong
Add the appropriate definitions and table entries for new adapter support.

Signed-off-by: Wen Xiong 
---
 drivers/scsi/ipr.c |   10 ++
 drivers/scsi/ipr.h |5 +
 2 files changed, 15 insertions(+)

Index: b/drivers/scsi/ipr.c
===
--- a/drivers/scsi/ipr.c2013-01-11 13:29:48.162478273 -0600
+++ b/drivers/scsi/ipr.c2013-01-11 13:30:56.053727535 -0600
@@ -9277,6 +9277,8 @@ static struct pci_device_id ipr_pci_tabl
{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROC_FPGA_E2,
PCI_VENDOR_ID_IBM, IPR_SUBS_DEV_ID_57B2, 0, 0, 0 },
{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROC_FPGA_E2,
+   PCI_VENDOR_ID_IBM, IPR_SUBS_DEV_ID_57C0, 0, 0, 0 },
+   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROC_FPGA_E2,
PCI_VENDOR_ID_IBM, IPR_SUBS_DEV_ID_57C3, 0, 0, 0 },
{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROC_FPGA_E2,
PCI_VENDOR_ID_IBM, IPR_SUBS_DEV_ID_57C4, 0, 0, 0 },
@@ -9290,6 +9292,14 @@ static struct pci_device_id ipr_pci_tabl
PCI_VENDOR_ID_IBM, IPR_SUBS_DEV_ID_57C8, 0, 0, 0 },
{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROCODILE,
PCI_VENDOR_ID_IBM, IPR_SUBS_DEV_ID_57CE, 0, 0, 0 },
+   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROCODILE,
+   PCI_VENDOR_ID_IBM, IPR_SUBS_DEV_ID_57D5, 0, 0, 0 },
+   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROCODILE,
+   PCI_VENDOR_ID_IBM, IPR_SUBS_DEV_ID_57D6, 0, 0, 0 },
+   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROCODILE,
+   PCI_VENDOR_ID_IBM, IPR_SUBS_DEV_ID_57D7, 0, 0, 0 },
+   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROCODILE,
+   PCI_VENDOR_ID_IBM, IPR_SUBS_DEV_ID_57D8, 0, 0, 0 },
{ }
 };
 MODULE_DEVICE_TABLE(pci, ipr_pci_table);
Index: b/drivers/scsi/ipr.h
===
--- a/drivers/scsi/ipr.h2013-01-11 13:29:53.292478040 -0600
+++ b/drivers/scsi/ipr.h2013-01-11 13:30:56.053727535 -0600
@@ -82,6 +82,7 @@
 
 #define IPR_SUBS_DEV_ID_57B40x033B
 #define IPR_SUBS_DEV_ID_57B20x035F
+#define IPR_SUBS_DEV_ID_57C00x0352
 #define IPR_SUBS_DEV_ID_57C30x0353
 #define IPR_SUBS_DEV_ID_57C40x0354
 #define IPR_SUBS_DEV_ID_57C60x0357
@@ -94,6 +95,10 @@
 #define IPR_SUBS_DEV_ID_574D0x0356
 #define IPR_SUBS_DEV_ID_57C80x035D
 
+#define IPR_SUBS_DEV_ID_57D50x03FB
+#define IPR_SUBS_DEV_ID_57D60x03FC
+#define IPR_SUBS_DEV_ID_57D70x03FF
+#define IPR_SUBS_DEV_ID_57D80x03FE
 #define IPR_NAME   "ipr"
 
 /*

-- 
--
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] cxgb4i: Remove the scsi host device when removing device

2013-01-11 Thread Thadeu Lima de Souza Cascardo
When doing a hotplug remove of a cxgb4 device, there are still dandling
symlinks at /sys/class/scsi_host/hostX to the removed PCI device. The
upper layer device may also try to send data, which may crash the
system.

The DETACH message from the lower level driver is sent to the ULD when
the device is removed, when the scsi host should be removed from the
system, avoiding any problems.

After this patch, there are no more dangling symlinks and many attempts
to crash the system while there is SCSI activity and removing the device
have failed. Adding the device back again works as expected, with the
scsi hosts showing up again.

Based on a patch by Karen Xie.

Cc: Karen Xie 
Cc: Divy La Rey 
Signed-off-by: Thadeu Lima de Souza Cascardo 
---
 drivers/scsi/cxgbi/cxgb4i/cxgb4i.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c 
b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index f924b3c..3fecf35 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -1564,6 +1564,7 @@ static int t4_uld_state_change(void *handle, enum 
cxgb4_state state)
break;
case CXGB4_STATE_DETACH:
pr_info("cdev 0x%p, DETACH.\n", cdev);
+   cxgbi_device_unregister(cdev);
break;
default:
pr_info("cdev 0x%p, unknown state %d.\n", cdev, state);
-- 
1.7.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 v12 7/9] libata: scsi: no poll when ODD is powered off

2013-01-11 Thread Tejun Heo
Hello,

On Fri, Jan 11, 2013 at 11:16:26AM +0800, Aaron Lu wrote:
> OK, will make it atomic in next version, thanks for the advice.
> 
> Perhaps I can add two scsi helper functions in scsi_lib.c like:
> 
> void sdev_disable_disk_events(struct scsi_device *sdev)
> {
>   atomic_inc(&sdev->disk_events_disable_depth);
> }
> 
> void sdev_enable_disk_events(struct scsi_device *sdev)
> {
>   if (WARN_ON_ONCE(atomic_read(&sdev->disk_events_disable_depth) <= 0))
>   return;
>   atomic_dec(&sdev->disk_events_disable_depth);
> }
> 
> And call them in ATA layer. Do you like this?

Sounds good to me.  James, how does the series look to you?

Thanks!

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: linux-next: build failure after merge of the scsi tree

2013-01-11 Thread James Bottomley
On Fri, 2013-01-11 at 11:35 -0600, Brian King wrote:
> On 01/11/2013 10:05 AM, Greg KH wrote:
> > On Fri, Jan 11, 2013 at 03:37:17PM +, James Bottomley wrote:
> >> On Fri, 2013-01-11 at 09:27 -0600, Brian King wrote:
> >>> It looks like this was a due to the fact that the new patches
> >>> added __devinit tags in the same merge window the __devinit tag
> >>> itself was getting removed.
> >>
> >> Not exactly.  The patch which makes them nops went into 3.8.  Now
> >> there's a patch queued in, Gregs tree I presume, to remove them all and
> >> the #defines which causes the compile failure.
> >>
> >>> As to the sparse warnings, this patch fixed the ones that
> >>> were actual bugs in the new code, although we could have
> >>> made that more clear in the patch description.
> >>>
> >>> http://marc.info/?l=linux-scsi&m=135716576204083&w=2
> >>
> >> Ah, thanks ... I've been on holiday for a while, so I did miss that.
> >>
> >>> There is one outstanding issue I am aware of which was an
> >>> array bounds compiler warning which looks to be a misdetection
> >>> by the compiler. Wendy and I discussed adding a BUG_ON
> >>> to stop the compiler from complaining.
> >>>
> >>> Wendy - lets queue these two changes up ASAP. They should both
> >>> be very simple changes.
> >>
> >> If it's a simple gcc bug, just ignore it.
> >>
> >> I do need you to redo the patches to remove the __dev annotations,
> >> though.  We can't risk introducing a bisect killing compile breakage if
> >> Greg's tree merges before mine in the next merge window.
> > 
> > This change should be pushed to Linus in time for 3.8-final, so there
> > should not be any bisect issues.
> 
> We can do this either way.
> 
> James - what is your preference? Drop everything and do a resend of the
> entire series or delta patches on top of what is currently in your tree?

Drop everything and resend still, I think.  There's still a rebase
problem, because the merge failure will happen if I rebase the misc tree
to beyond Greg's merge point and I'd rather not have to worry about it.

Thanks,

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: linux-next: build failure after merge of the scsi tree

2013-01-11 Thread Brian King
On 01/11/2013 10:05 AM, Greg KH wrote:
> On Fri, Jan 11, 2013 at 03:37:17PM +, James Bottomley wrote:
>> On Fri, 2013-01-11 at 09:27 -0600, Brian King wrote:
>>> It looks like this was a due to the fact that the new patches
>>> added __devinit tags in the same merge window the __devinit tag
>>> itself was getting removed.
>>
>> Not exactly.  The patch which makes them nops went into 3.8.  Now
>> there's a patch queued in, Gregs tree I presume, to remove them all and
>> the #defines which causes the compile failure.
>>
>>> As to the sparse warnings, this patch fixed the ones that
>>> were actual bugs in the new code, although we could have
>>> made that more clear in the patch description.
>>>
>>> http://marc.info/?l=linux-scsi&m=135716576204083&w=2
>>
>> Ah, thanks ... I've been on holiday for a while, so I did miss that.
>>
>>> There is one outstanding issue I am aware of which was an
>>> array bounds compiler warning which looks to be a misdetection
>>> by the compiler. Wendy and I discussed adding a BUG_ON
>>> to stop the compiler from complaining.
>>>
>>> Wendy - lets queue these two changes up ASAP. They should both
>>> be very simple changes.
>>
>> If it's a simple gcc bug, just ignore it.
>>
>> I do need you to redo the patches to remove the __dev annotations,
>> though.  We can't risk introducing a bisect killing compile breakage if
>> Greg's tree merges before mine in the next merge window.
> 
> This change should be pushed to Linus in time for 3.8-final, so there
> should not be any bisect issues.

We can do this either way.

James - what is your preference? Drop everything and do a resend of the
entire series or delta patches on top of what is currently in your tree?

Thanks,

Brian


-- 
Brian King
Power Linux I/O
IBM Linux Technology Center


--
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 v1 1/1] block: blk-merge: don't merge the pages with non-contiguous descriptors

2013-01-11 Thread Subhash Jadavani
blk_rq_map_sg() function merges the physically contiguous pages to use same
scatter-gather node without checking if their page descriptors are
contiguous or not.

Now when dma_map_sg() is called on the scatter gather list, it would
take the base page pointer from each node (one by one) and iterates
through all of the pages in same sg node by keep incrementing the base
page pointer with the assumption that physically contiguous pages will
have their page descriptor address contiguous which may not be true
if SPARSEMEM config is enabled. So here we may end referring to invalid
page descriptor.

Following table shows the example of physically contiguous pages but
their page descriptor addresses non-contiguous.
---
| Page Descriptor|   Physical Address |
--
| 0xc1e43fdc |   0xd000   |
| 0xc2052000 |   0xe000   |
---

With this patch, relevant blk-merge functions will also check if the
physically contiguous pages are having page descriptors address contiguous
or not? If not then, these pages are separated to be in different
scatter-gather nodes.

Signed-off-by: Subhash Jadavani 
---
 block/blk-merge.c |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 936a110..623fca5 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -42,6 +42,9 @@ static unsigned int __blk_recalc_rq_segments(struct 
request_queue *q,
goto new_segment;
if (!BIOVEC_SEG_BOUNDARY(q, bvprv, bv))
goto new_segment;
+   if ((bvprv->bv_page != bv->bv_page) &&
+   (bvprv->bv_page + 1) != bv->bv_page)
+   goto new_segment;
 
seg_size += bv->bv_len;
bvprv = bv;
@@ -126,6 +129,9 @@ __blk_segment_map_sg(struct request_queue *q, struct 
bio_vec *bvec,
goto new_segment;
if (!BIOVEC_SEG_BOUNDARY(q, *bvprv, bvec))
goto new_segment;
+   if ((bvprv->bv_page != bvec->bv_page) &&
+   ((bvprv->bv_page + 1) != bvec->bv_page))
+   goto new_segment;
 
(*sg)->length += nbytes;
} else {
-- 
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of 
Code Aurora Forum, hosted by The Linux Foundation

--
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: linux-next: build failure after merge of the scsi tree

2013-01-11 Thread Greg KH
On Fri, Jan 11, 2013 at 03:37:17PM +, James Bottomley wrote:
> On Fri, 2013-01-11 at 09:27 -0600, Brian King wrote:
> > It looks like this was a due to the fact that the new patches
> > added __devinit tags in the same merge window the __devinit tag
> > itself was getting removed.
> 
> Not exactly.  The patch which makes them nops went into 3.8.  Now
> there's a patch queued in, Gregs tree I presume, to remove them all and
> the #defines which causes the compile failure.
> 
> > As to the sparse warnings, this patch fixed the ones that
> > were actual bugs in the new code, although we could have
> > made that more clear in the patch description.
> > 
> > http://marc.info/?l=linux-scsi&m=135716576204083&w=2
> 
> Ah, thanks ... I've been on holiday for a while, so I did miss that.
> 
> > There is one outstanding issue I am aware of which was an
> > array bounds compiler warning which looks to be a misdetection
> > by the compiler. Wendy and I discussed adding a BUG_ON
> > to stop the compiler from complaining.
> > 
> > Wendy - lets queue these two changes up ASAP. They should both
> > be very simple changes.
> 
> If it's a simple gcc bug, just ignore it.
> 
> I do need you to redo the patches to remove the __dev annotations,
> though.  We can't risk introducing a bisect killing compile breakage if
> Greg's tree merges before mine in the next merge window.

This change should be pushed to Linus in time for 3.8-final, so there
should not be any bisect issues.

thanks,

greg k-h
--
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: linux-next: build failure after merge of the scsi tree

2013-01-11 Thread James Bottomley
On Fri, 2013-01-11 at 09:27 -0600, Brian King wrote:
> It looks like this was a due to the fact that the new patches
> added __devinit tags in the same merge window the __devinit tag
> itself was getting removed.

Not exactly.  The patch which makes them nops went into 3.8.  Now
there's a patch queued in, Gregs tree I presume, to remove them all and
the #defines which causes the compile failure.

> As to the sparse warnings, this patch fixed the ones that
> were actual bugs in the new code, although we could have
> made that more clear in the patch description.
> 
> http://marc.info/?l=linux-scsi&m=135716576204083&w=2

Ah, thanks ... I've been on holiday for a while, so I did miss that.

> There is one outstanding issue I am aware of which was an
> array bounds compiler warning which looks to be a misdetection
> by the compiler. Wendy and I discussed adding a BUG_ON
> to stop the compiler from complaining.
> 
> Wendy - lets queue these two changes up ASAP. They should both
> be very simple changes.

If it's a simple gcc bug, just ignore it.

I do need you to redo the patches to remove the __dev annotations,
though.  We can't risk introducing a bisect killing compile breakage if
Greg's tree merges before mine in the next merge window.

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: linux-next: build failure after merge of the scsi tree

2013-01-11 Thread Brian King
It looks like this was a due to the fact that the new patches
added __devinit tags in the same merge window the __devinit tag
itself was getting removed.

As to the sparse warnings, this patch fixed the ones that
were actual bugs in the new code, although we could have
made that more clear in the patch description.

http://marc.info/?l=linux-scsi&m=135716576204083&w=2

There is one outstanding issue I am aware of which was an
array bounds compiler warning which looks to be a misdetection
by the compiler. Wendy and I discussed adding a BUG_ON
to stop the compiler from complaining.

Wendy - lets queue these two changes up ASAP. They should both
be very simple changes.

Thanks,

Brian


On 01/11/2013 01:34 AM, James Bottomley wrote:
> On Fri, 2013-01-11 at 12:03 +1100, Stephen Rothwell wrote:
>> Hi James,
>>
>> After merging the scsi tree, today's linux-next build (powerpc
>> ppc64_defconfig) failed like this:
>>
>> drivers/scsi/ipr.c:9138:22: error: expected '=', ',', ';', 'asm' or 
>> '__attribute__' before 'ipr_enable_msix'
>> drivers/scsi/ipr.c:9165:22: error: expected '=', ',', ';', 'asm' or 
>> '__attribute__' before 'ipr_enable_msi'
>> drivers/scsi/ipr.c:9188:23: error: expected '=', ',', ';', 'asm' or 
>> '__attribute__' before 'name_msi_vectors'
>> drivers/scsi/ipr.c:9200:22: error: expected '=', ',', ';', 'asm' or 
>> '__attribute__' before 'ipr_request_other_msi_irqs'
>> drivers/scsi/ipr.c: In function 'ipr_probe_ioa':
>> drivers/scsi/ipr.c:9422:4: error: implicit declaration of function 
>> 'ipr_enable_msix' [-Werror=implicit-function-declaration]
>> drivers/scsi/ipr.c:9425:4: error: implicit declaration of function 
>> 'ipr_enable_msi' [-Werror=implicit-function-declaration]
>> drivers/scsi/ipr.c:9517:3: error: implicit declaration of function 
>> 'name_msi_vectors' [-Werror=implicit-function-declaration]
>> drivers/scsi/ipr.c:9523:4: error: implicit declaration of function 
>> 'ipr_request_other_msi_irqs' [-Werror=implicit-function-declaration]
> 
> OK, fine, I'll drop all the ipr patches.  I've been waiting for a month
> for them to fix the smatch and sparse warnings.  Please resend the
> series with all the fixes.
> 
> Thanks,
> 
> 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
> 


-- 
Brian King
Power Linux I/O
IBM Linux Technology Center


--
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 V5 3/4] [SCSI] ufs: Add Platform glue driver for ufshcd

2013-01-11 Thread Subhash Jadavani

On 1/11/2013 4:11 PM, Sujit Reddy Thumma wrote:

On 1/9/2013 5:41 PM, vinayak holikatti wrote:

On Mon, Jan 7, 2013 at 1:11 PM, Sujit Reddy Thumma
 wrote:

Hi Vinayak,

I have few comments below:



+#ifdef CONFIG_PM
+/**
+ * ufshcd_pltfrm_suspend - suspend power management function
+ * @pdev: pointer to Platform device handle
+ * @mesg: power state
+ *
+ * Returns -ENOSYS


What breaks if you return 0 instead of return -ENOSYS? Returning 
error seems
to break platform suspend/resume until all the TODO's are addressed. 
If the
current s/w cannot make h/w suspend, it should be okay to let the 
rest of

the system be suspended.



In that case how will the controller be in a working state once it 
resumes.

It does not make sense to return zero and to notify the OS that
everything is fine.


Since the suspend routine doesn't do anything except returning zero, 
no power/clocks would be removed and the controller should be in the 
same state after resume. Do you see any system that removes 
power/clocks to controllers during suspend without knowledge of 
corresponding drivers? If so, then such systems must be fixed. In any 
case, blocking entire system suspend just because s/w isn't taking 
care of powering down one controller is not a good idea.


I would like to hear from others on this as well.


Yes, i agree with Sujit that there is no point in blocking the entire 
system suspend just because ufshcd haven't implemented their suspend 
functionality. returning 0 from this function should be fine. And as 
Sujit already mentioned, if during resume you don't find the UFS 
(controller / phy) state as it was left in suspend then it's a 
particular system's issue and which needs to be fixed.







+ */
+static int ufshcd_pltfrm_suspend(struct platform_device *pdev,
+pm_message_t mesg)
+{
+   /*
+* TODO:
+* 1. Call ufshcd_suspend
+* 2. Do bus specific power management
+*/
+
+   return -ENOSYS;


Returning error doesn't allow entire system to be suspended. 
Perhaps, you

can do disable_irq() and return 0?


+}
+
+/**
+ * ufshcd_pltfrm_resume - resume power management function
+ * @pdev: pointer to Platform device handle
+ *
+ * Returns -ENOSYS
+ */
+static int ufshcd_pltfrm_resume(struct platform_device *pdev)
+{
+   /*
+* TODO:
+* 1. Call ufshcd_resume.
+* 2. Do bus specific wake up
+*/
+
+   return -ENOSYS;


enable_irq() and return 0?


+}
+#endif
+



+static int __devexit ufshcd_pltfrm_remove(struct platform_device
*pdev)
+{
+   struct resource *mem_res;
+   struct resource *irq_res;
+   resource_size_t mem_size;
+   struct ufs_hba *hba = platform_get_drvdata(pdev);
+
+   irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);


It would be better to save the irq number under "struct ufs_hba" 
during

probe. So here during remove you just need simply need to call the
"free_irq(irq_res->start, hba)"


Will modify the code accordingly in the next patchset.


+
+   if (!irq_res)
+   dev_err(&pdev->dev, "ufshcd: IRQ resource not
available\n");
+   else
+   free_irq(irq_res->start, hba);



The documentation of free_irq says:
"... On a shared IRQ the caller must ensure the interrupt is 
disabled on the
card it drives before calling this function. .." I don't see 
disable_irq()

getting called either here or ufshcd_remove().


Why would you want to disable the entire IRQ line when it is shared?
Logical thing is to disable the interrupt on the controller.


Since you have enabled the irq in ufshcd_init() and decremented the 
desc->depth you need to need to do disable_irq(). disable_irq() 
doesn't disable the irq line until all the shared irq drivers disables 
it.


Also, on some systems not calling disable_irq() could be a problem - 
the power to wakeup irq monitor block couldn't be turned off if there 
are active irqs.








+
+   ufshcd_remove(hba);


Remove should be exactly opposite of probe(). So shouldn't you 
call the

ufshcd_remove() first and then free_irq() after that.


Some bugging controllers might raise the interrupt after resources 
are

removed.
this sequence will prevent it.



Could you please add the same as comment in above code sequence?


+   mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);


You might want to save the pointer to mem_res in "struct ufs_hba" 
during

probe and may use the same here.


+   if (!mem_res)
+   dev_err(&pdev->dev, "ufshcd: Memory resource not
available\n");
+   else {
+   mem_size = resource_size(mem_res);
+   release_mem_region(mem_res->start, mem_size);
+   }
+   platform_set_drvdata(pdev, NULL);
+   return 0;
+}
+
+static const struct of_device_id ufs_of_match[] = {
+   { .compatible = "jedec,ufs-1.1"},
+};
+
+static struct platform_driver ufshcd_pltfrm_driver = {
+   .probe  = ufshcd_pltfrm_probe,
+   .remove = __d

Re: [PATCH V5 3/4] [SCSI] ufs: Add Platform glue driver for ufshcd

2013-01-11 Thread Sujit Reddy Thumma

On 1/9/2013 5:41 PM, vinayak holikatti wrote:

On Mon, Jan 7, 2013 at 1:11 PM, Sujit Reddy Thumma
 wrote:

Hi Vinayak,

I have few comments below:



+#ifdef CONFIG_PM
+/**
+ * ufshcd_pltfrm_suspend - suspend power management function
+ * @pdev: pointer to Platform device handle
+ * @mesg: power state
+ *
+ * Returns -ENOSYS


What breaks if you return 0 instead of return -ENOSYS? Returning error seems
to break platform suspend/resume until all the TODO's are addressed. If the
current s/w cannot make h/w suspend, it should be okay to let the rest of
the system be suspended.



In that case how will the controller be in a working state once it resumes.
It does not make sense to return zero and to notify the OS that
everything is fine.


Since the suspend routine doesn't do anything except returning zero, no 
power/clocks would be removed and the controller should be in the same 
state after resume. Do you see any system that removes power/clocks to 
controllers during suspend without knowledge of corresponding drivers? 
If so, then such systems must be fixed. In any case, blocking entire 
system suspend just because s/w isn't taking care of powering down one 
controller is not a good idea.


I would like to hear from others on this as well.




+ */
+static int ufshcd_pltfrm_suspend(struct platform_device *pdev,
+pm_message_t mesg)
+{
+   /*
+* TODO:
+* 1. Call ufshcd_suspend
+* 2. Do bus specific power management
+*/
+
+   return -ENOSYS;


Returning error doesn't allow entire system to be suspended. Perhaps, you
can do disable_irq() and return 0?


+}
+
+/**
+ * ufshcd_pltfrm_resume - resume power management function
+ * @pdev: pointer to Platform device handle
+ *
+ * Returns -ENOSYS
+ */
+static int ufshcd_pltfrm_resume(struct platform_device *pdev)
+{
+   /*
+* TODO:
+* 1. Call ufshcd_resume.
+* 2. Do bus specific wake up
+*/
+
+   return -ENOSYS;


enable_irq() and return 0?


+}
+#endif
+



+static int __devexit ufshcd_pltfrm_remove(struct platform_device
*pdev)
+{
+   struct resource *mem_res;
+   struct resource *irq_res;
+   resource_size_t mem_size;
+   struct ufs_hba *hba =  platform_get_drvdata(pdev);
+
+   irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);


It would be better to save the irq number under "struct ufs_hba" during
probe. So here during remove you just need simply need to call the
"free_irq(irq_res->start, hba)"


Will modify the code accordingly in the next patchset.


+
+   if (!irq_res)
+   dev_err(&pdev->dev, "ufshcd: IRQ resource not
available\n");
+   else
+   free_irq(irq_res->start, hba);



The documentation of free_irq says:
"... On a shared IRQ the caller must ensure the interrupt is disabled on the
card it drives before calling this function. .." I don't see disable_irq()
getting called either here or ufshcd_remove().


Why would you want to disable the entire IRQ line when it is shared?
Logical thing is to disable the interrupt on the controller.


Since you have enabled the irq in ufshcd_init() and decremented the 
desc->depth you need to need to do disable_irq(). disable_irq() doesn't 
disable the irq line until all the shared irq drivers disables it.


Also, on some systems not calling disable_irq() could be a problem - the 
power to wakeup irq monitor block couldn't be turned off if there are 
active irqs.








+
+   ufshcd_remove(hba);


Remove should be exactly opposite of probe(). So shouldn't you call the
ufshcd_remove() first and then free_irq() after that.


Some bugging controllers might raise the interrupt after resources are
removed.
this sequence will prevent it.



Could you please add the same as comment in above code sequence?


+   mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);


You might want to save the pointer to mem_res in "struct ufs_hba" during
probe and may use the same here.


+   if (!mem_res)
+   dev_err(&pdev->dev, "ufshcd: Memory resource not
available\n");
+   else {
+   mem_size = resource_size(mem_res);
+   release_mem_region(mem_res->start, mem_size);
+   }
+   platform_set_drvdata(pdev, NULL);
+   return 0;
+}
+
+static const struct of_device_id ufs_of_match[] = {
+   { .compatible = "jedec,ufs-1.1"},
+};
+
+static struct platform_driver ufshcd_pltfrm_driver = {
+   .probe  = ufshcd_pltfrm_probe,
+   .remove = __devexit_p(ufshcd_pltfrm_remove),
+#ifdef CONFIG_PM



CONFIG_PM_SLEEP would be better?


the current implementation looks fine.


Also, can you move legacy suspend/resume


Ok,


callbacks below to dev_pm_ops?


+   .suspend = ufshcd_pltfrm_suspend,
+   .resume = ufshcd_pltfrm_resume,
+#endif
+   .driver = {
+   .name   = "ufshcd",
+   .owner  = THIS_MODULE,
+   .of_match_table = ufs_of_match,
+   },
+};



--
Rega

RE: [patch] [SCSI] bfa: fix strncpy() limiter in bfad_start_ops()

2013-01-11 Thread Anil Gurumurthy


-Original Message-
From: linux-scsi-ow...@vger.kernel.org 
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of Dan Carpenter
Sent: Thursday, January 10, 2013 2:36 PM
To: Anil Gurumurthy
Cc: Vijay Mohan Guvva; James E.J. Bottomley; linux-scsi@vger.kernel.org; 
linux-ker...@vger.kernel.org; kernel-janit...@vger.kernel.org
Subject: [patch] [SCSI] bfa: fix strncpy() limiter in bfad_start_ops()

The closing parenthesis is in the wrong place so it takes the sizeof a pointer 
instead of the sizeof the buffer minus one.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c index 
e6bf126..a5f7690 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -1034,7 +1034,7 @@ bfad_start_ops(struct bfad_s *bfad) {
sizeof(driver_info.host_os_patch) - 1);
 
strncpy(driver_info.os_device_name, bfad->pci_name,
-   sizeof(driver_info.os_device_name - 1));
+   sizeof(driver_info.os_device_name) - 1);
 
/* FCS driver info init */
spin_lock_irqsave(&bfad->bfad_lock, flags);
--


Looks good. Thanks for the patch, Dan.

Acked-by: Anil Gurumurthy


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