Re: [PATCHv3] Update scsi host to use ida for host number

2015-09-01 Thread Christoph Hellwig 
On Tue, Sep 01, 2015 at 05:03:28PM -0700, Lee Duncan wrote:
> +static int host_get_index(int *index)
> +{
> + int error = -ENOMEM;
> +
> + do {
> + if (!ida_pre_get(&host_index_ida, GFP_KERNEL))
> + break;
> + spin_lock(&host_index_lock);
> + error = ida_get_new(&host_index_ida, index);
> + spin_unlock(&host_index_lock);
> + } while (error == -EAGAIN);
> +
> + return error;
> +}
> +
> +static inline void host_put_index(int index)
> +{
> + spin_lock(&host_index_lock);
> + ida_remove(&host_index_ida, index);
> + spin_unlock(&host_index_lock);
> +}

I really hate how this pattern (and the equivalent for IDA) are
spread all over.  Any chance to have some simple_ida/simple_idr helpers
instead of duplicating it again an again.

Besides that why aren't we using and idr here and use it for host lookup
as well?
--
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 19/23] scsi_dh_alua: Use workqueue for RTPG

2015-09-01 Thread Christoph Hellwig 
On Tue, Sep 01, 2015 at 02:57:57PM +0200, Hannes Reinecke wrote:
> That is what I'm eventually planning to do.
> My final goal is to move the multipath path monitoring stuff
> into the kernel (via the block device polling mechanism), and sending
> block events for path failure and re-establishment.

It might be a good idea to prioritize that.  Todd has been asking
for multipath monitoring APIs and suggested adding D-BUS APIs to
multipathd.  I'd much prefer them directly talking to the kernel
instead of cementing multipathd, which is a bit of a wart.
--
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

[PATCHv3] Update scsi host to use ida for host number

2015-09-01 Thread Lee Duncan 
Each Scsi_host instance gets a host number starting
at 0, but this is implemented with an atomic integer,
and rollover doesn't seem to have been considered.
Another side-effect of this design is that scsi host
numbers used by iscsi are never reused, thereby  making
rollover more likely. This patch converts Scsi_host
instances to use ida to manage their instance
numbers.

This also means that host instance numbers will be
reused, when available.

Changes from v2:
* Use host_put_index() throughout
* Put host index code together
Changes from v1:
* Added inline host_put_index() and its use

Signed-off-by: Lee Duncan 
Reviewed-by: Hannes Reinecke 
Reviewed-by: Johannes Thumshirn 
---
 drivers/scsi/hosts.c | 47 ++-
 1 file changed, 38 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 8bb173e01084..a47944867e65 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -33,7 +33,7 @@
 #include 
 #include 
 #include 
-
+#include 
 #include 
 #include 
 #include 
@@ -42,8 +42,6 @@
 #include "scsi_logging.h"
 
 
-static atomic_t scsi_host_next_hn = ATOMIC_INIT(0);/* host_no for next new 
host */
-
 
 static void scsi_host_cls_release(struct device *dev)
 {
@@ -304,6 +302,31 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct 
device *dev,
 }
 EXPORT_SYMBOL(scsi_add_host_with_dma);
 
+static DEFINE_SPINLOCK(host_index_lock);
+static DEFINE_IDA(host_index_ida);
+
+static int host_get_index(int *index)
+{
+   int error = -ENOMEM;
+
+   do {
+   if (!ida_pre_get(&host_index_ida, GFP_KERNEL))
+   break;
+   spin_lock(&host_index_lock);
+   error = ida_get_new(&host_index_ida, index);
+   spin_unlock(&host_index_lock);
+   } while (error == -EAGAIN);
+
+   return error;
+}
+
+static inline void host_put_index(int index)
+{
+   spin_lock(&host_index_lock);
+   ida_remove(&host_index_ida, index);
+   spin_unlock(&host_index_lock);
+}
+
 static void scsi_host_dev_release(struct device *dev)
 {
struct Scsi_Host *shost = dev_to_shost(dev);
@@ -337,6 +360,8 @@ static void scsi_host_dev_release(struct device *dev)
 
kfree(shost->shost_data);
 
+   host_put_index(shost->host_no);
+
if (parent)
put_device(parent);
kfree(shost);
@@ -370,6 +395,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template 
*sht, int privsize)
 {
struct Scsi_Host *shost;
gfp_t gfp_mask = GFP_KERNEL;
+   int index;
+   int error;
 
if (sht->unchecked_isa_dma && privsize)
gfp_mask |= __GFP_DMA;
@@ -388,11 +415,11 @@ struct Scsi_Host *scsi_host_alloc(struct 
scsi_host_template *sht, int privsize)
init_waitqueue_head(&shost->host_wait);
mutex_init(&shost->scan_mutex);
 
-   /*
-* subtract one because we increment first then return, but we need to
-* know what the next host number was before increment
-*/
-   shost->host_no = atomic_inc_return(&scsi_host_next_hn) - 1;
+   error = host_get_index(&index);
+   if (error < 0)
+   goto fail_kfree;
+   shost->host_no = index;
+
shost->dma_channel = 0xff;
 
/* These three are default values which can be overridden */
@@ -477,7 +504,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template 
*sht, int privsize)
shost_printk(KERN_WARNING, shost,
"error handler thread failed to spawn, error = %ld\n",
PTR_ERR(shost->ehandler));
-   goto fail_kfree;
+   goto fail_free_index;
}
 
shost->tmf_work_q = alloc_workqueue("scsi_tmf_%d",
@@ -493,6 +520,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template 
*sht, int privsize)
 
  fail_kthread:
kthread_stop(shost->ehandler);
+ fail_free_index:
+   host_put_index(index);
  fail_kfree:
kfree(shost);
return NULL;
-- 
2.1.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

Re: [PATCHv2] Update scsi host to use ida for host number

2015-09-01 Thread Lee Duncan 
On 09/01/2015 10:49 AM, Sagi Grimberg wrote:
> On 9/1/2015 8:38 PM, ldun...@suse.com wrote:
>> From: Lee Duncan 
>>
>> Each Scsi_host instance gets a host number starting
>> at 0, but this is implemented with an atomic integer,
>> and rollover doesn't seem to have been considered.
>> Another side-effect of this design is that scsi host
>> numbers used by iscsi are never reused, thereby  making
>> rollover more likely. This patch converts Scsi_host
>> instances to use ida to manage their instance
>> numbers.
>>
>> This also means that host instance numbers will be
>> reused, when available.
>>
>> Changes from v1:
>> * Added inline host_put_index() and its use
>>
>> Signed-off-by: Lee Duncan 
>> Reviewed-by: Hannes Reinecke 
>> Reviewed-by: Johannes Thumshirn 
>> ---
>>   drivers/scsi/hosts.c | 47
>> +++
>>   1 file changed, 39 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
>> index 8bb173e01084..36cb65612821 100644
>> --- a/drivers/scsi/hosts.c
>> +++ b/drivers/scsi/hosts.c
>> @@ -33,7 +33,7 @@
>>   #include 
>>   #include 
>>   #include 
>> -
>> +#include 
>>   #include 
>>   #include 
>>   #include 
>> @@ -42,7 +42,8 @@
>>   #include "scsi_logging.h"
>>
>>
>> -static atomic_t scsi_host_next_hn = ATOMIC_INIT(0);/* host_no for
>> next new host */
>> +static DEFINE_SPINLOCK(host_index_lock);
>> +static DEFINE_IDA(host_index_ida);
>>
>>
>>   static void scsi_host_cls_release(struct device *dev)
>> @@ -337,6 +338,10 @@ static void scsi_host_dev_release(struct device
>> *dev)
>>
>>   kfree(shost->shost_data);
>>
>> +spin_lock(&host_index_lock);
>> +ida_remove(&host_index_ida, shost->host_no);
>> +spin_unlock(&host_index_lock);
> 
> Why not host_put_index(index)?
> 

Doh! I'll fix that in both places.
-- 
Lee Duncan
--
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: [Open-FCoE] [PATCH] scsi: fcoe: Convert use of __constant_htons to htons

2015-09-01 Thread Vasu Dev 
On Wed, 2015-08-19 at 11:18 +0530, Vaishali Thakkar wrote:
> In little endian cases, the macro htons unfolds to __swab16 which
> provides special case for constants. In big endian cases,
> __constant_htons and htons expand directly to the same expression.
> So, replace __constant_htons with htons with the goal of getting
> rid of the definition of __constant_htons completely.
> 
> The semantic patch that performs this transformation is as follows:
> 
> @@expression x;@@
> 
> - __constant_htons(x)
> + htons(x)
> 
> Signed-off-by: Vaishali Thakkar 
> ---
>  drivers/scsi/fcoe/fcoe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index ec193a8..d3eb80c 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -364,7 +364,7 @@ static int fcoe_interface_setup(struct fcoe_interface 
> *fcoe,
>* on the ethertype for the given device
>*/
>   fcoe->fcoe_packet_type.func = fcoe_rcv;
> - fcoe->fcoe_packet_type.type = __constant_htons(ETH_P_FCOE);
> + fcoe->fcoe_packet_type.type = htons(ETH_P_FCOE);
>   fcoe->fcoe_packet_type.dev = netdev;
>   dev_add_pack(&fcoe->fcoe_packet_type);
>  

Acked-by: Vasu Dev 

--
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 09/14] fix: lpfc_send_rscn_event sends bigger buffer size

2015-09-01 Thread Sebastian Herbszt 
James Smart wrote:
> 
> From: Ales Novak 
> 
> lpfc_send_rscn_event() allocates data for sizeof(struct
> lpfc_rscn_event_header) + payload_len, but claims that the data has size
> of sizeof(struct lpfc_els_event_header) + payload_len. That leads to
> buffer overruns.
> 
> Signed-off-by: Ales Novak 
> Signed-off-by: James Smart 
> Reviewed-by: Hannes Reinecke 
> ---
>  drivers/scsi/lpfc/lpfc_els.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index c859aa3..f9c957d 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c
> @@ -5401,7 +5401,7 @@ lpfc_send_rscn_event(struct lpfc_vport *vport,
>  
>   fc_host_post_vendor_event(shost,
>   fc_get_event_number(),
> - sizeof(struct lpfc_els_event_header) + payload_len,
> + sizeof(struct lpfc_rscn_event_header) + payload_len,
>   (char *)rscn_event_data,
>   LPFC_NL_VENDOR_ID);
>  

Reviewed-by: Sebastian Herbszt 
--
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 08/14] lpfc: Fix possible use-after-free and double free in lpfc_mbx_cmpl_rdp_page_a2()

2015-09-01 Thread Sebastian Herbszt 
James Smart wrote:
> 
> From: Johannes Thumshirn 
> 
> If the bf_get() call in lpfc_mbx_cmpl_rdp_page_a2() does succeeds, execution
> continues normally and mp gets kfree()d.
> 
> If the subsequent call to lpfc_sli_issue_mbox() fails execution jumps to the
> error label where lpfc_mbuf_free() is called with mp->virt and mp->phys as
> function arguments. This is the use after free. Following the use after free 
> mp
> gets kfree()d again which is a double free.
> 
> Signed-off-by: Johannes Thumshirn 
> Signed-off-by: James Smart 
> ---
>  drivers/scsi/lpfc/lpfc_mbox.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_mbox.c b/drivers/scsi/lpfc/lpfc_mbox.c
> index 723e110..18838ea 100644
> --- a/drivers/scsi/lpfc/lpfc_mbox.c
> +++ b/drivers/scsi/lpfc/lpfc_mbox.c
> @@ -2276,7 +2276,7 @@ lpfc_mbx_cmpl_rdp_page_a2(struct lpfc_hba *phba, 
> LPFC_MBOXQ_t *mbox)
>   (struct lpfc_rdp_context *)(mbox->context2);
>  
>   if (bf_get(lpfc_mqe_status, &mbox->u.mqe))
> - goto error;
> + goto error_mbuf_free;
>  
>   lpfc_sli_bemem_bcopy(mp->virt, &rdp_context->page_a2,
>   DMP_SFF_PAGE_A2_SIZE);
> @@ -2291,13 +2291,14 @@ lpfc_mbx_cmpl_rdp_page_a2(struct lpfc_hba *phba, 
> LPFC_MBOXQ_t *mbox)
>   mbox->mbox_cmpl = lpfc_mbx_cmpl_rdp_link_stat;
>   mbox->context2 = (struct lpfc_rdp_context *) rdp_context;
>   if (lpfc_sli_issue_mbox(phba, mbox, MBX_NOWAIT) == MBX_NOT_FINISHED)
> - goto error;
> + goto error_cmd_free;
>  
>   return;
>  
> -error:
> +error_mbuf_free:
>   lpfc_mbuf_free(phba, mp->virt, mp->phys);
>   kfree(mp);
> +error_cmd_free:
>   lpfc_sli4_mbox_cmd_free(phba, mbox);
>   rdp_context->cmpl(phba, rdp_context, FAILURE);
>  }

Reviewed-by: Sebastian Herbszt 
--
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 06/14] lpfc:Make the function lpfc_sli4_mbox_completions_pending static in order to comply with function prototype

2015-09-01 Thread Sebastian Herbszt 
James Smart wrote:
> 
> From: Nicholas Krause 
> 
> This makes the function lpfc_sli4_mbox_completion's definition
> static now in order to comply with its prototype being also
> declared as static too.
> 
> Signed-off-by: Nicholas Krause 
> Signed-off-by: James Smart 
> ---
>  drivers/scsi/lpfc/lpfc_sli.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
> index 4feb931..95d53c7 100644
> --- a/drivers/scsi/lpfc/lpfc_sli.c
> +++ b/drivers/scsi/lpfc/lpfc_sli.c
> @@ -6696,7 +6696,7 @@ lpfc_mbox_timeout(unsigned long ptr)
>   * This function checks if any mailbox completions are present on the mailbox
>   * completion queue.
>   **/
> -bool
> +static bool
>  lpfc_sli4_mbox_completions_pending(struct lpfc_hba *phba)
>  {
>  

Reviewed-by: Sebastian Herbszt 
--
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 03/14] lpfc: in sli3 use configured sg_seg_cnt for sg_tablesize

2015-09-01 Thread Sebastian Herbszt 
James Smart wrote:
> 
> From: Bodo Stroesser 
> 
> Hi James,
> 
> We had some performance problems with RAID systems connected to LPe12k.
> AFAICS, the reason is a small bug in lpfc.ko, causing the IO-size to
> be smaller than expected.
> 
> The patch below fixes it for us.
> 
> Please CC me, I'm not on the list.
> 
> Best regards
> Bodo
> 
> --
> 
> Currently the module parameter lpfc_sg_seg_count does not have effect
> for sli3 devices.
> 
> In lpfc_sli_driver_resource_setup(), which is used for sli3, the code
> writes the configured sg_seg_cnt into lpfc_template.sg_tablesize.
> But lpfc_template is the template used for sli4 only. Thus the value should
> correctly be written to lpfc_template_s3->sg_tablesize.
> 
> This patch is for kernel 4.1-rc5, but is tested with lpfc 10.2.405.26 only.
> 
> Signed-off-by: Bodo Stroesser 
> Signed-off-by: James Smart 
> ---
>  drivers/scsi/lpfc/lpfc_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index da9b6fc..81bfb2d 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -4994,7 +4994,7 @@ lpfc_sli_driver_resource_setup(struct lpfc_hba *phba)
>  
>   /* Initialize the host templates the configured values. */
>   lpfc_vport_template.sg_tablesize = phba->cfg_sg_seg_cnt;
> - lpfc_template.sg_tablesize = phba->cfg_sg_seg_cnt;
> + lpfc_template_s3.sg_tablesize = phba->cfg_sg_seg_cnt;
>  
>   /* There are going to be 2 reserved BDEs: 1 FCP cmnd + 1 FCP rsp */
>   if (phba->cfg_enable_bg) {

Reviewed-by: Sebastian Herbszt 
--
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 02/14] lpfc: Remove unnessary cast

2015-09-01 Thread Sebastian Herbszt 
James Smart wrote:
> 
> From: Firo Yang 
> 
> kzalloc() returns a void pointer - no need to cast it in
> drivers/scsi/lpfc/lpfc_init.c::lpfc_sli_driver_resource_setup()
> 
> Signed-off-by: Firo Yang 
> Signed-off-by: James Smart 
> ---
>  drivers/scsi/lpfc/lpfc_init.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index 1992e74..da9b6fc 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -4982,8 +4982,7 @@ lpfc_sli_driver_resource_setup(struct lpfc_hba *phba)
>   }
>  
>   if (!phba->sli.ring)
> - phba->sli.ring = (struct lpfc_sli_ring *)
> - kzalloc(LPFC_SLI3_MAX_RING *
> + phba->sli.ring = kzalloc(LPFC_SLI3_MAX_RING *
>   sizeof(struct lpfc_sli_ring), GFP_KERNEL);
>   if (!phba->sli.ring)
>   return -ENOMEM;

Reviewed-by: Sebastian Herbszt 
--
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 6/8] scsi: ufs: make the UFS variant a platform device

2015-09-01 Thread Rob Herring 
On Sun, Aug 30, 2015 at 9:52 AM, Yaniv Gardi  wrote:
> This change turns the UFS variant (SCSI_UFS_QCOM) into a UFS
> a platform device.
> In order to do so a few additional changes are required:
> 1. The ufshcd-pltfrm is no longer serves as a platform device.
>Now it only serves as a group of platform APIs such as PM APIs
>(runtime suspend/resume, system suspend/resume etc), parsers of
>clocks, regulators and pm_levels from DT.
> 2. What used to be the old platform "probe" is now "only"
>a pltfrm_init() routine, that does exactly the same, but only
>being called by the new probe function of the UFS variant.
>
> Signed-off-by: Yaniv Gardi 
>
> ---
>  Documentation/devicetree/bindings/ufs/ufs-qcom.txt | 57 ++
>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt  |  4 +-

For the binding:

Reviewed-by: Rob Herring 

A comment on the driver part still.

> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index 329ac84..8027435 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c

[...]

> +static int ufs_qcom_probe(struct platform_device *pdev)
> +{
> +   int err;
> +   struct device *dev = &pdev->dev;
> +   struct ufs_hba *hba;
> +
> +   /* Perform generic probe */
> +   err = ufshcd_pltfrm_init(pdev, &ufs_hba_qcom_vops);
> +   if (err) {
> +   dev_err(dev, "ufshcd_pltfrm_init() failed %d\n", err);
> +   goto out;
> +   }
> +
> +   hba = platform_get_drvdata(pdev);

I thought this was not necessary?

> +   if (unlikely(!hba)) {
> +   dev_err(dev, "no hba structure after successful probing\n");
> +   goto dealloc_host;
> +   }
> +
> +   return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SCSI: mvsas: Fix NULL pointer dereference in mvs_slot_task_free

2015-09-01 Thread Dāvis Mosāns 
2015-08-21 7:29 GMT+03:00 Dāvis Mosāns :
> When pci_pool_alloc fails in mvs_task_prep then task->lldd_task stays
> NULL but it's later used in mvs_abort_task as slot which is passed
> to mvs_slot_task_free causing NULL pointer dereference.
>
> Just return from mvs_slot_task_free when passed with NULL slot.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=101891
> Signed-off-by: Dāvis Mosāns 
> ---
>  drivers/scsi/mvsas/mv_sas.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
> index 454536c..9c78074 100644
> --- a/drivers/scsi/mvsas/mv_sas.c
> +++ b/drivers/scsi/mvsas/mv_sas.c
> @@ -887,6 +887,8 @@ static void mvs_slot_free(struct mvs_info *mvi, u32 
> rx_desc)
>  static void mvs_slot_task_free(struct mvs_info *mvi, struct sas_task *task,
>   struct mvs_slot_info *slot, u32 slot_idx)
>  {
> +   if (!slot)
> +   return;
> if (!slot->task)
> return;
> if (!sas_protocol_ata(task->task_proto))
> --
> 2.5.0
>

ping? do I need to CC someone else?
--
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: [PATCHv2] Update scsi host to use ida for host number

2015-09-01 Thread Sagi Grimberg 

On 9/1/2015 8:38 PM, ldun...@suse.com wrote:

From: Lee Duncan 

Each Scsi_host instance gets a host number starting
at 0, but this is implemented with an atomic integer,
and rollover doesn't seem to have been considered.
Another side-effect of this design is that scsi host
numbers used by iscsi are never reused, thereby  making
rollover more likely. This patch converts Scsi_host
instances to use ida to manage their instance
numbers.

This also means that host instance numbers will be
reused, when available.

Changes from v1:
* Added inline host_put_index() and its use

Signed-off-by: Lee Duncan 
Reviewed-by: Hannes Reinecke 
Reviewed-by: Johannes Thumshirn 
---
  drivers/scsi/hosts.c | 47 +++
  1 file changed, 39 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 8bb173e01084..36cb65612821 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -33,7 +33,7 @@
  #include 
  #include 
  #include 
-
+#include 
  #include 
  #include 
  #include 
@@ -42,7 +42,8 @@
  #include "scsi_logging.h"


-static atomic_t scsi_host_next_hn = ATOMIC_INIT(0);/* host_no for next new 
host */
+static DEFINE_SPINLOCK(host_index_lock);
+static DEFINE_IDA(host_index_ida);


  static void scsi_host_cls_release(struct device *dev)
@@ -337,6 +338,10 @@ static void scsi_host_dev_release(struct device *dev)

kfree(shost->shost_data);

+   spin_lock(&host_index_lock);
+   ida_remove(&host_index_ida, shost->host_no);
+   spin_unlock(&host_index_lock);


Why not host_put_index(index)?
--
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

[PATCHv2] Update scsi host to use ida for host number

2015-09-01 Thread lduncan 
From: Lee Duncan 

Each Scsi_host instance gets a host number starting
at 0, but this is implemented with an atomic integer,
and rollover doesn't seem to have been considered.
Another side-effect of this design is that scsi host
numbers used by iscsi are never reused, thereby  making
rollover more likely. This patch converts Scsi_host
instances to use ida to manage their instance
numbers.

This also means that host instance numbers will be
reused, when available.

Changes from v1:
* Added inline host_put_index() and its use

Signed-off-by: Lee Duncan 
Reviewed-by: Hannes Reinecke 
Reviewed-by: Johannes Thumshirn 
---
 drivers/scsi/hosts.c | 47 +++
 1 file changed, 39 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 8bb173e01084..36cb65612821 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -33,7 +33,7 @@
 #include 
 #include 
 #include 
-
+#include 
 #include 
 #include 
 #include 
@@ -42,7 +42,8 @@
 #include "scsi_logging.h"
 
 
-static atomic_t scsi_host_next_hn = ATOMIC_INIT(0);/* host_no for next new 
host */
+static DEFINE_SPINLOCK(host_index_lock);
+static DEFINE_IDA(host_index_ida);
 
 
 static void scsi_host_cls_release(struct device *dev)
@@ -337,6 +338,10 @@ static void scsi_host_dev_release(struct device *dev)
 
kfree(shost->shost_data);
 
+   spin_lock(&host_index_lock);
+   ida_remove(&host_index_ida, shost->host_no);
+   spin_unlock(&host_index_lock);
+
if (parent)
put_device(parent);
kfree(shost);
@@ -353,6 +358,28 @@ static struct device_type scsi_host_type = {
.release =  scsi_host_dev_release,
 };
 
+static int host_get_index(int *index)
+{
+   int error = -ENOMEM;
+
+   do {
+   if (!ida_pre_get(&host_index_ida, GFP_KERNEL))
+   break;
+   spin_lock(&host_index_lock);
+   error = ida_get_new(&host_index_ida, index);
+   spin_unlock(&host_index_lock);
+   } while (error == -EAGAIN);
+
+   return error;
+}
+
+static inline void host_put_index(int index)
+{
+   spin_lock(&host_index_lock);
+   ida_remove(&host_index_ida, index);
+   spin_unlock(&host_index_lock);
+}
+
 /**
  * scsi_host_alloc - register a scsi host adapter instance.
  * @sht:   pointer to scsi host template
@@ -370,6 +397,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template 
*sht, int privsize)
 {
struct Scsi_Host *shost;
gfp_t gfp_mask = GFP_KERNEL;
+   int index;
+   int error;
 
if (sht->unchecked_isa_dma && privsize)
gfp_mask |= __GFP_DMA;
@@ -388,11 +417,11 @@ struct Scsi_Host *scsi_host_alloc(struct 
scsi_host_template *sht, int privsize)
init_waitqueue_head(&shost->host_wait);
mutex_init(&shost->scan_mutex);
 
-   /*
-* subtract one because we increment first then return, but we need to
-* know what the next host number was before increment
-*/
-   shost->host_no = atomic_inc_return(&scsi_host_next_hn) - 1;
+   error = host_get_index(&index);
+   if (error < 0)
+   goto fail_kfree;
+   shost->host_no = index;
+
shost->dma_channel = 0xff;
 
/* These three are default values which can be overridden */
@@ -477,7 +506,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template 
*sht, int privsize)
shost_printk(KERN_WARNING, shost,
"error handler thread failed to spawn, error = %ld\n",
PTR_ERR(shost->ehandler));
-   goto fail_kfree;
+   goto fail_free_index;
}
 
shost->tmf_work_q = alloc_workqueue("scsi_tmf_%d",
@@ -493,6 +522,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template 
*sht, int privsize)
 
  fail_kthread:
kthread_stop(shost->ehandler);
+ fail_free_index:
+   host_put_index(index);
  fail_kfree:
kfree(shost);
return NULL;
-- 
2.1.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

[GIT PULL] first round of SCSI updates for the 4.2+ merge window

2015-09-01 Thread James Bottomley 
This includes one new driver: cxlflash plus the usual grab bag of
updates for the major drivers: qla2xxx, ipr, storvsc, pm80xx, hptiop,
plus a few assorted fixes.  There's another tranch coming, but I want to
incubate it another few days in the checkers, plus it includes a mpt2sas
separated lifetime fix, which Avago won't get done testing until
Friday. 

The patch is available here:

git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-misc

The Short Changelog is:


Alexey Khoroshilov (1):
  bfa: fix leak of bfad_im_port_index on module unload

Arun Easi (1):
  qla2xxx: Fix missing device login retries.

Bart Van Assche (11):
  qla2xxx: Avoid that sparse complains about context imbalances
  qla2xxx: Remove dead code
  qla2xxx: Remove a superfluous test
  qla2xxx: Fix sparse annotations
  qla2xxx: Avoid that sparse complains about duplicate [noderef] attributes
  qla2xxx: Remove __constant_ prefix
  qla2xxx: Replace two macros with an inline function
  qla2xxx: Remove set-but-not-used variables
  qla2xxx: Declare local functions static
  qla2xxx: Report both rsp_info and rsp_info_len
  libfc: Fix a typo in a source code comment

Bjorn Helgaas (3):
  megaraid_sas: fix whitespace errors
  megaraid_sas: use dev_printk when possible
  megaraid : use dev_printk when possible

Brian King (2):
  ipr: Driver version 2.6.2
  ipr: Endian / sparse fixes

Chad Dupuis (3):
  qla2xxx: Remove decrement of sp reference count in abort handler.
  qla2xxx: Do not reset ISP for error entry with an out of range handle.
  qla2xxx: Do not reset adapter if SRB handle is in range.

Chris Leech (1):
  iSCSI: let session recovery_tmo sysfs writes persist across recovery

Dan Carpenter (5):
  aic94xx: set an error code on failure
  cxlflash: shift wrapping bug in afu_link_reset()
  cxlflash: off by one bug in cxlflash_show_port_status()
  mptfusion: prevent some memory corruption
  hpsa: fix an sprintf() overflow in the reset handler

Dilip Kumar Uppugandla (1):
  qla2xxx: Return the fabric command state for non-task management requests

Don Brace (7):
  hpsa: fix rmmod issues
  hpsa: add in new controllers
  hpsa: cleanup update scsi devices
  hpsa: add PMC to copyright
  hpsa: correct static checker warnings on driver init cleanup
  hpsa: correct decode sense data
  hpsa: Correct double unlock of mutex

Hannes Reinecke (2):
  scsi: Add ALUA state change UA handling
  scsi: retry MODE SENSE on unit attention

Himanshu Madhani (2):
  qla2xxx: Update driver version to 8.07.00.26-k
  qla2xxx: do not clear slot in outstanding cmd array

Hiral Patel (1):
  qla2xxx: Do not crash system for sp ref count zero

Jack Wang (1):
  MAINTAINERS: update email for pm8001

Joe Carnuccio (4):
  qla2xxx: Pause risc before manipulating risc semaphore.
  qla2xxx: Use ssdid to gate semaphore manipulation.
  qla2xxx: Handle AEN8014 incoming port logout.
  qla2xxx: Add serdes register read/write support for ISP25xx.

Joe Handzik (1):
  hpsa: add sysfs entry path_info to show box and bay information

Johannes Thumshirn (2):
  st: Destroy st_index_idr on module exit
  mvsas: always iounmap resources

K. Y. Srinivasan (1):
  storvsc: Set the error code correctly in failure conditions

Keith Mange (6):
  storvsc: Allow write_same when host is windows 10
  storvsc: use storage protocol version to determine storage capabilities
  storvsc: use correct defaults for values determined by protocol 
negotiation
  storvsc: Untangle the storage protocol negotiation from the vmbus 
protocol negotiation.
  storvsc: Use a single value to track protocol versions
  storvsc: Rather than look for sets of specific protocol versions, make 
decisions based on ranges.

Kevin Barnett (1):
  Change how controllers in mixed mode are handled.

Matthew R. Ochs (5):
  cxlflash: Remove unused variable from queuecommand
  cxlflash: Virtual LUN support
  cxlflash: Superpipe support
  cxlflash: Base error recovery support
  cxlflash: Base support for IBM CXL Flash Adapter

Saurav Kashyap (1):
  qla2xxx: Add adapter checks for FAWWN functionality.

Sawan Chandak (2):
  qla2xxx: Add pci device id 0x2261.
  qla2xxx: Add support to show MPI and PEP FW version for ISP27xx.

Scott Benesh (1):
  hpsa: add in new offline mode

Sebastian Herbszt (1):
  lpfc: Use && instead of & for boolean expression

Seymour, Shane M (2):
  st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO
  st: convert to using driver attr groups for sysfs

Sreekanth Reddy (1):
  mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected

Suresh Thiagarajan (1):
  pm80xx: Added pm8006 controller support

Thadeu Lima de Souza Cascardo (1):
  qla2xxx: prevent board_disable from running during EEH

Viswas G (8):
  pm80xx: Bu

Re: [PATCH] Update scsi host to use ida for host number

2015-09-01 Thread Lee Duncan 
On 09/01/2015 01:23 AM, Sagi Grimberg wrote:
> On 8/31/2015 11:28 PM, leeman.dun...@gmail.com wrote:
>> From: Lee Duncan 
>>
>> Each Scsi_host instance gets a host number starting
>> at 0, but this is implemented with an atomic integer,
>> and rollover doesn't seem to have been considered.
>> Another side-effect of this design is that scsi host
>> numbers used by iscsi are never reused, thereby  making
>> rollover more likely. This patch converts Scsi_host
>> instances to use ida to manage their instance
>> numbers.
>>
>> This also means that host instance numbers will be
>> reused, when available.
>>
>> Signed-off-by: Lee Duncan 
>> ---
>>   drivers/scsi/hosts.c | 43 +++
>>   1 file changed, 35 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
>> index 8bb173e01084..dade75478541 100644
>> --- a/drivers/scsi/hosts.c
>> +++ b/drivers/scsi/hosts.c
>> @@ -33,7 +33,7 @@
>>   #include 
>>   #include 
>>   #include 
>> -
>> +#include 
>>   #include 
>>   #include 
>>   #include 
>> @@ -42,7 +42,8 @@
>>   #include "scsi_logging.h"
>>
>>
>> -static atomic_t scsi_host_next_hn = ATOMIC_INIT(0);/* host_no for
>> next new host */
>> +static DEFINE_SPINLOCK(host_index_lock);
>> +static DEFINE_IDA(host_index_ida);
>>
>>
>>   static void scsi_host_cls_release(struct device *dev)
>> @@ -337,6 +338,10 @@ static void scsi_host_dev_release(struct device
>> *dev)
>>
>>   kfree(shost->shost_data);
>>
>> +spin_lock(&host_index_lock);
>> +ida_remove(&host_index_ida, shost->host_no);
>> +spin_unlock(&host_index_lock);
> 
> Why not get it in a symmetrical host_put_index()

Seems like a reasonable suggestion. Since it's only 3 statements, I'll
make it an inline.

I will resubmit the patch, adding the two reviewed-by lines from Hannes
and Johannes, since the code will essentially be the same.

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

Re: [PATCH] scsi_scan: move 'INQUIRY result too short' message to debug level

2015-09-01 Thread Vitaly Kuznetsov 
James Bottomley  writes:

> On Mon, 2015-08-31 at 14:50 +0200, Vitaly Kuznetsov wrote:
>> Some Hyper-V hosts are known for ignoring SPC-2/3/4 requirement
>> for 'INQUIRY data (see table ...) shall contain at least 36 bytes'. As a
>> result we get tons on 'scsi 0:7:1:1: scsi scan: INQUIRY result too short
>> (5), using 36' messages on console. As Hyper-V is also known for its
>> serial port being extremely slow multi-VCPU guests we get CPU blocked
>> putting these (useless) messages on console (e.g. happens when we add
>> multiple disks). Move them to debug level.
>
> This isn't an ignorable debug message.  It means the inquiry information
> the system relies on will be false, so it's fairly essential for bug
> reports.  It could be made a once per device print, but I don't think we
> can eliminate it.

I see. What if we introduce a special flag to mark such known-as-broken
hosts? E.g.:

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index f9f3f82..c6ffb5c 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -701,9 +701,10 @@ static int scsi_probe_lun(struct scsi_device *sdev, 
unsigned char *inq_result,
 * strings.
 */
if (sdev->inquiry_len < 36) {
-   sdev_printk(KERN_INFO, sdev,
-   "scsi scan: INQUIRY result too short (%d),"
-   " using 36\n", sdev->inquiry_len);
+   if (!sdev->host->hostt->short_inquiry)
+   sdev_printk(KERN_INFO, sdev,
+   "scsi scan: INQUIRY result too short (%d),"
+   " using 36\n", sdev->inquiry_len);
sdev->inquiry_len = 36;
}
 
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 3c6584f..f3b4d0f 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1711,6 +1711,7 @@ static struct scsi_host_template scsi_driver = {
/* Make sure we dont get a sg segment crosses a page boundary */
.dma_boundary = PAGE_SIZE-1,
.no_write_same =1,
+   .short_inquiry =1,
 };
 
 enum {
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index e113c75..04aefad 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -454,6 +454,12 @@ struct scsi_host_template {
unsigned no_async_abort:1;
 
/*
+* True if this host adapter returns short (<36 bytes) resposes to
+* INQUIRY requests.
+*/
+   unsigned short_inquiry:1;
+
+   /*
 * Countdown for host blocking with no commands outstanding.
 */
unsigned int max_host_blocked;

-- 
  Vitaly
--
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 13/23] scsi_dh_alua: Use separate alua_port_group structure

2015-09-01 Thread Hannes Reinecke 
On 09/01/2015 03:44 PM, Christoph Hellwig wrote:
> On Tue, Sep 01, 2015 at 03:02:29PM +0200, Hannes Reinecke wrote:
 +  rcu_read_lock();
 +  pg = rcu_dereference(h->pg);
 +  if (!pg) {
 +  rcu_read_unlock();
 +  return -ENXIO;
 +  }
>>>
>>> What is this rcu_read_lock supposed to protect against given
>>> that struct alua_port_group isn't RCU freed?  I think this needs
>>> to be another kref_get_unless_zero.
>>>
>> It's not freed now, but it'll be with one of the next patches (ie with
>> the 'rescan' patch).
>> I just kept it here for consistency, following the rule to always
>> enclose 'rcu_dereference' with 'rcu_read_lock()/rcu_read_unlock()'
>> pairs. Irrespective on whether it's being freed or not.
> 
> After this patch this is the only RCU call in the driver.  I don't
> think adding it here make any sense.
> 
Right, I'll be moving it to the rescan patch then.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi_scan: move 'INQUIRY result too short' message to debug level

2015-09-01 Thread James Bottomley 
On Mon, 2015-08-31 at 14:50 +0200, Vitaly Kuznetsov wrote:
> Some Hyper-V hosts are known for ignoring SPC-2/3/4 requirement
> for 'INQUIRY data (see table ...) shall contain at least 36 bytes'. As a
> result we get tons on 'scsi 0:7:1:1: scsi scan: INQUIRY result too short
> (5), using 36' messages on console. As Hyper-V is also known for its
> serial port being extremely slow multi-VCPU guests we get CPU blocked
> putting these (useless) messages on console (e.g. happens when we add
> multiple disks). Move them to debug level.

This isn't an ignorable debug message.  It means the inquiry information
the system relies on will be false, so it's fairly essential for bug
reports.  It could be made a once per device print, but I don't think we
can eliminate it.

James

> Signed-off-by: Vitaly Kuznetsov 
> ---
>  drivers/scsi/scsi_scan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index f9f3f82..cb5c50a 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -701,7 +701,7 @@ static int scsi_probe_lun(struct scsi_device *sdev, 
> unsigned char *inq_result,
>* strings.
>*/
>   if (sdev->inquiry_len < 36) {
> - sdev_printk(KERN_INFO, sdev,
> + sdev_printk(KERN_DEBUG, sdev,
>   "scsi scan: INQUIRY result too short (%d),"
>   " using 36\n", sdev->inquiry_len);
>   sdev->inquiry_len = 36;




--
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 13/23] scsi_dh_alua: Use separate alua_port_group structure

2015-09-01 Thread Christoph Hellwig 
On Tue, Sep 01, 2015 at 03:02:29PM +0200, Hannes Reinecke wrote:
> >> +  rcu_read_lock();
> >> +  pg = rcu_dereference(h->pg);
> >> +  if (!pg) {
> >> +  rcu_read_unlock();
> >> +  return -ENXIO;
> >> +  }
> > 
> > What is this rcu_read_lock supposed to protect against given
> > that struct alua_port_group isn't RCU freed?  I think this needs
> > to be another kref_get_unless_zero.
> > 
> It's not freed now, but it'll be with one of the next patches (ie with
> the 'rescan' patch).
> I just kept it here for consistency, following the rule to always
> enclose 'rcu_dereference' with 'rcu_read_lock()/rcu_read_unlock()'
> pairs. Irrespective on whether it's being freed or not.

After this patch this is the only RCU call in the driver.  I don't
think adding it here make any sense.
--
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 13/23] scsi_dh_alua: Use separate alua_port_group structure

2015-09-01 Thread Hannes Reinecke 
On 09/01/2015 12:20 PM, Christoph Hellwig wrote:
>> +struct alua_dh_data {
>> +struct alua_port_group  *pg;
>> +int rel_port;
>> +int tpgs;
> 
> The tpgs file doesn't make much sense in the new alua_dh_data
> structure.  Please return it explicitly from alua_check_tpgs and
> assign it directly to the member in struct alua_port_group.
> 
Okay.

>> +static void release_port_group(struct kref *kref)
>> +{
>> +struct alua_port_group *pg;
>> +
>> +pg = container_of(kref, struct alua_port_group, kref);
>> +printk(KERN_WARNING "alua: release port group %d\n", pg->group_id);
> 
> Why is this a warning?  I'd consider it nothing but debug noise.
> 
Yes, indeed it is. I'll be removing it.

>> -err = alua_rtpg(sdev, h, 0);
>> -if (err != SCSI_DH_OK)
>> +if (err != SCSI_DH_OK || !h->pg)
>>  goto out;
>>  
>> +kref_get(&h->pg->kref);
> 
> What prevents this kref_get from racing with the last kref_put?
> 
> I think all the kref_get calls in this patch need to be
> kref_get_unless_zero with proper error handling.
> 
Okay.

>> +rcu_read_lock();
>> +pg = rcu_dereference(h->pg);
>> +if (!pg) {
>> +rcu_read_unlock();
>> +return -ENXIO;
>> +}
> 
> What is this rcu_read_lock supposed to protect against given
> that struct alua_port_group isn't RCU freed?  I think this needs
> to be another kref_get_unless_zero.
> 
It's not freed now, but it'll be with one of the next patches (ie with
the 'rescan' patch).
I just kept it here for consistency, following the rule to always
enclose 'rcu_dereference' with 'rcu_read_lock()/rcu_read_unlock()'
pairs. Irrespective on whether it's being freed or not.

>> +
>>  if (optimize)
>> -h->flags |= ALUA_OPTIMIZE_STPG;
>> +pg->flags |= ALUA_OPTIMIZE_STPG;
>>  else
>> -h->flags &= ~ALUA_OPTIMIZE_STPG;
>> +pg->flags |= ~ALUA_OPTIMIZE_STPG;
>> +rcu_read_unlock();
> 
> The read-modify-write of pg->flags will need some sort of locking.
> Seems like most modifications of it are under pg->rtpg_lock, so
> I'd suggest to enforce that as a strict rule.
> 
Yes, of course the 'rtpg_lock' should have been taken here.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 11/23] scsi_dh_alua: Make stpg synchronous

2015-09-01 Thread Hannes Reinecke 
On 09/01/2015 12:04 PM, Christoph Hellwig wrote:
> On Thu, Aug 27, 2015 at 02:41:09PM +0200, Hannes Reinecke wrote:
>> We should be issuing STPG synchronously as we need to
>> evaluate the return code on failure.
> 
> This looks right, but I think the patch needs to be split and needs
> a way better changelog.
> 
> patch 1:
>  - factor out the alua_stpg helper
> 
> patch 2:
>  - async STPG
>  - please explain that alua_activate will now block and why this is ok
> 
> patch 3:
>  - add the new call to alua_rtpg and why it's added
> 
Okay, will be doing so with the next round.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 19/23] scsi_dh_alua: Use workqueue for RTPG

2015-09-01 Thread Hannes Reinecke 
On 09/01/2015 01:15 PM, Christoph Hellwig wrote:
>> +unsigned long   expiry;
>> +unsigned long   interval;
>> +struct workqueue_struct *work_q;
>> +struct delayed_work rtpg_work;
>> +struct delayed_work stpg_work;
>> +struct delayed_work qdata_work;
>> +spinlock_t  rtpg_lock;
> 
> This one also protects ->flag.  I'd just call it lock, and
> introduce it at the time struct alua_port_group is introduced,
> so that we can have coherent locking from the very beginning.
> 
Okay.

>> +struct alua_queue_data {
>> +struct list_headentry;
>>  activate_complete   callback_fn;
>>  void*callback_data;
> 
> I've been looking over the active callback for a while, and I
> think the model of it is fundamentally broken.  Yes, as long as
> multipathing isn't in SCSI we'll need path change notifications
> for dm-mpath, but I don't see how tying them into ->activate
> makes any sense.
> 
> I guess for this series we're stuck with it, but in the long
> run we should have a callback in the request_queue which the
> consumer that has bd_claim()ed the device can set and get path
> change notifications instead.
> 
That is what I'm eventually planning to do.
My final goal is to move the multipath path monitoring stuff
into the kernel (via the block device polling mechanism), and sending
block events for path failure and re-establishment.

But that'll be subject to further patches :-)

> That being said after spending a lot of time reading this code I think
> my orginal advice to use different work_structs for the different
> kinds of events was wrong, and I'd like to apologize for making you
> go down that direction.
> 
> STPG/RTPG is just too dependent to sensibly split them up, and the
> qdata bit while broken can be shoe horned in for now.  So I'd suggest
> to revert back to the original model with a single work_struct per
> port group, and a global workqueue.
> 
Weelll ... there was a reason why I did that initially :-)
But anyway, no harm done. I still have the original series around.

>> -if (h->pg)
>> +if (h->pg) {
>> +synchronize_rcu();
>>  return SCSI_DH_OK;
>> +}
> 
> What is this synchronize_rcu supposed to help with?
> 
>> -h->pg = tmp_pg;
>>  kref_get(&tmp_pg->kref);
>> +spin_lock(&h->pg_lock);
>> +rcu_assign_pointer(h->pg, tmp_pg);
>> +spin_unlock(&h->pg_lock);
> 
> I think the assignment to h->ph should have been past the kref_get
> from the very beginning, please fold this into the original patch.
> 
Okay.

>> +struct alua_port_group *pg = NULL;
>> +int error;
>> +
>> +mutex_lock(&h->init_mutex);
>> +error = alua_check_tpgs(sdev, h);
>> +if (error == SCSI_DH_OK) {
>> +error = alua_check_vpd(sdev, h);
>> +rcu_read_lock();
>> +pg = rcu_dereference(h->pg);
>> +if (!pg) {
>> +rcu_read_unlock();
>> +h->tpgs = TPGS_MODE_NONE;
>> +error = SCSI_DH_DEV_UNSUPP;
>> +} else {
>> +WARN_ON(error != SCSI_DH_OK);
>> +kref_get(&pg->kref);
>> +rcu_read_unlock();
>> +}
>> +}
> 
> Eww. Please grab the reference to the pg inside the replacement for
> alua_check_vpd, and ensure that we never return from that without a
> valid h->pg.  Together with the previously suggested removal of h->tpgs,
> and moving the call to alua_rtpg_queue into the alua_check_vpd replacement
> that would keep alua_initialize nice and simple.
> 
You are right in that the interface for alua_check_tpgs isn't very
clear. Currently we need to check both the return code _and_ 'h->pg',
even though both are interrelated.
I guess it should rather be 'h->pg = alua_check_vpd()' or even making
alua_check_vpd() a void function, which would eliminate the need for the
separate return value.
I'll have to think about it.

>> @@ -679,6 +895,10 @@ static int alua_set_params(struct scsi_device *sdev, 
>> const char *params)
>>  unsigned int optimize = 0, argc;
>>  const char *p = params;
>>  int result = SCSI_DH_OK;
>> +unsigned long flags;
>> +
>> +if (!h)
>> +return -ENXIO;
> 
> How could that happen?  Why does it beling into this patch?
> 
Leftover from the original patch, which didn't have your scsi_dh
patchset. I'll remove it.

>> @@ -723,24 +944,46 @@ static int alua_activate(struct scsi_device *sdev,
>>  {
>>  struct alua_dh_data *h = sdev->handler_data;
>>  int err = SCSI_DH_OK;
>> +struct alua_queue_data *qdata;
>> +struct alua_port_group *pg;
>>  
>> -if (!h->pg)
>> +if (!h) {
>> +err = SCSI_DH_NOSYS;
>>  goto out;
>> +}
> 
> Same here.
> 
>> +qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
>> +if (!qdata) {
>> +err = SCSI_DH_RES_TEMP_UNAVAIL;
>> +

Re: [PATCH 16/23] scsi: Add scsi_vpd_lun_id()

2015-09-01 Thread Hannes Reinecke 
On 09/01/2015 12:22 PM, Christoph Hellwig wrote:
> On Thu, Aug 27, 2015 at 02:41:14PM +0200, Hannes Reinecke wrote:
>> Add a function scsi_vpd_lun_id() to return a unique device
>> identifcation based on the designation descriptors of
>> VPD page 0x83.
>>
>> As devices might implement several descriptors the order
>> of preference is:
>> - NAA IEE Registered Extended
>> - EUI-64 based 16-byte
>> - EUI-64 based 12-byte
>> - NAA IEEE Registered
>> - NAA IEEE Extended
>> A SCSI name string descriptor is preferred to all of them
>> if the identification is longer than 16 bytes.
>>
>> The returned unique device identification will be formatted
>> as a SCSI Name string to avoid clashes between different
>> designator types.
> 
> Looks good in general, but I wonder if it might make sense to switch
> to kasprintf and let the caller free the buffer?
> 
I really am no friend of caller-needs-to-free-up-buffer thingies.
Allocations and deallocations should be done in the same context for
better tracking (and debugging), IMO.

> Otherwise:
> 
> Reviewed-by: Christoph Hellwig 
> 
Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 04/23] scsi_dh_alua: use standard logging functions

2015-09-01 Thread Hannes Reinecke 
On 09/01/2015 11:48 AM, Christoph Hellwig wrote:
> On Thu, Aug 27, 2015 at 02:41:02PM +0200, Hannes Reinecke wrote:
>>  }
>>  
>>  err = alua_check_sense(sdev, &sense_hdr);
>> -if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry))
>> +if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) {
>> +sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n",
>> +ALUA_DH_NAME);
>> +scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
>>  goto retry;
>> -sdev_printk(KERN_INFO, sdev,
>> -"%s: rtpg sense code %02x/%02x/%02x\n",
>> -ALUA_DH_NAME, sense_hdr.sense_key,
>> -sense_hdr.asc, sense_hdr.ascq);
>> -err = SCSI_DH_IO;
>> +}
>> +sdev_printk(KERN_ERR, sdev, "%s: rtpg failed\n",
>> +ALUA_DH_NAME);
>> +scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
>> +return SCSI_DH_IO;
>>  }
>> -if (err != SCSI_DH_OK)
>> -return err;
> 
> I think you need to keep this if, given that submit_rtpg can return
> others error than SCSI_DH_IO as well.
> 
> While you're at it you might remove the h->senselen check, and
> kill the assignment of the scsi_normalize_sense bool return value
> to the err variable similar to how you did in stpg_endio. 
> 
Okay, will be doing so.

Cheers,

Hannes

-- 
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 19/23] scsi_dh_alua: Use workqueue for RTPG

2015-09-01 Thread Christoph Hellwig 
> + unsigned long   expiry;
> + unsigned long   interval;
> + struct workqueue_struct *work_q;
> + struct delayed_work rtpg_work;
> + struct delayed_work stpg_work;
> + struct delayed_work qdata_work;
> + spinlock_t  rtpg_lock;

This one also protects ->flag.  I'd just call it lock, and
introduce it at the time struct alua_port_group is introduced,
so that we can have coherent locking from the very beginning.

> +struct alua_queue_data {
> + struct list_headentry;
>   activate_complete   callback_fn;
>   void*callback_data;

I've been looking over the active callback for a while, and I
think the model of it is fundamentally broken.  Yes, as long as
multipathing isn't in SCSI we'll need path change notifications
for dm-mpath, but I don't see how tying them into ->activate
makes any sense.

I guess for this series we're stuck with it, but in the long
run we should have a callback in the request_queue which the
consumer that has bd_claim()ed the device can set and get path
change notifications instead.

That being said after spending a lot of time reading this code I think
my orginal advice to use different work_structs for the different
kinds of events was wrong, and I'd like to apologize for making you
go down that direction.

STPG/RTPG is just too dependent to sensibly split them up, and the
qdata bit while broken can be shoe horned in for now.  So I'd suggest
to revert back to the original model with a single work_struct per
port group, and a global workqueue.

> - if (h->pg)
> + if (h->pg) {
> + synchronize_rcu();
>   return SCSI_DH_OK;
> + }

What is this synchronize_rcu supposed to help with?

> - h->pg = tmp_pg;
>   kref_get(&tmp_pg->kref);
> + spin_lock(&h->pg_lock);
> + rcu_assign_pointer(h->pg, tmp_pg);
> + spin_unlock(&h->pg_lock);

I think the assignment to h->ph should have been past the kref_get
from the very beginning, please fold this into the original patch.

> + struct alua_port_group *pg = NULL;
> + int error;
> +
> + mutex_lock(&h->init_mutex);
> + error = alua_check_tpgs(sdev, h);
> + if (error == SCSI_DH_OK) {
> + error = alua_check_vpd(sdev, h);
> + rcu_read_lock();
> + pg = rcu_dereference(h->pg);
> + if (!pg) {
> + rcu_read_unlock();
> + h->tpgs = TPGS_MODE_NONE;
> + error = SCSI_DH_DEV_UNSUPP;
> + } else {
> + WARN_ON(error != SCSI_DH_OK);
> + kref_get(&pg->kref);
> + rcu_read_unlock();
> + }
> + }

Eww. Please grab the reference to the pg inside the replacement for
alua_check_vpd, and ensure that we never return from that without a
valid h->pg.  Together with the previously suggested removal of h->tpgs,
and moving the call to alua_rtpg_queue into the alua_check_vpd replacement
that would keep alua_initialize nice and simple.

> @@ -679,6 +895,10 @@ static int alua_set_params(struct scsi_device *sdev, 
> const char *params)
>   unsigned int optimize = 0, argc;
>   const char *p = params;
>   int result = SCSI_DH_OK;
> + unsigned long flags;
> +
> + if (!h)
> + return -ENXIO;

How could that happen?  Why does it beling into this patch?

> @@ -723,24 +944,46 @@ static int alua_activate(struct scsi_device *sdev,
>  {
>   struct alua_dh_data *h = sdev->handler_data;
>   int err = SCSI_DH_OK;
> + struct alua_queue_data *qdata;
> + struct alua_port_group *pg;
>  
> - if (!h->pg)
> + if (!h) {
> + err = SCSI_DH_NOSYS;
>   goto out;
> + }

Same here.

> + qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
> + if (!qdata) {
> + err = SCSI_DH_RES_TEMP_UNAVAIL;
> + goto out;
> + }
> + qdata->callback_fn = fn;
> + qdata->callback_data = data;
>  
> + mutex_lock(&h->init_mutex);
> + rcu_read_lock();
> + pg = rcu_dereference(h->pg);
> + if (!pg) {
> + rcu_read_unlock();
> + kfree(qdata);
> + err = h->init_error;
> + mutex_unlock(&h->init_mutex);
>   goto out;
>   }
> + mutex_unlock(&h->init_mutex);
> + fn = NULL;
> + kref_get(&pg->kref);
> + rcu_read_unlock();
> +
> + if (optimize_stpg) {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&pg->rtpg_lock, flags);
> + pg->flags |= ALUA_OPTIMIZE_STPG;
> + spin_unlock_irqrestore(&pg->rtpg_lock, flags);
> + }

The optimize_stpg should have been moved towards the alua_port_group
allocation earlier in the series, please fold that in there.

> + alua_rtpg_queue(pg, sdev, qdata);
> + kref_put(&pg->kref, release_port_group);

Note that all alua_rtpg_queue callers

Re: [PATCH 13/23] scsi_dh_alua: Use separate alua_port_group structure

2015-09-01 Thread Christoph Hellwig 
Ok, coming back to this path as it's the start of somethign building
up over various patches:

> + pg = kzalloc(sizeof(struct alua_port_group), GFP_KERNEL);
> + if (!pg) {
> + sdev_printk(KERN_WARNING, sdev,
> + "%s: kzalloc port group failed\n",
> + ALUA_DH_NAME);
> + /* Temporary failure, bypass */
> + return SCSI_DH_DEV_TEMP_BUSY;
> + }
> + pg->group_id = group_id;
> + pg->buff = pg->inq;
> + pg->bufflen = ALUA_INQUIRY_SIZE;
> + pg->tpgs = h->tpgs;
> + pg->state = TPGS_STATE_OPTIMIZED;
> + kref_init(&pg->kref);
> + spin_lock(&port_group_lock);
> + list_add(&pg->node, &port_group_list);
> + h->pg = pg;
> + spin_unlock(&port_group_lock);
> +
> + return SCSI_DH_OK;

All this code isn't really checking the VPD anymore.  Please keep
the existing alua_check_vpd as a low-level helper, and move this
new functionality into a separate alua_find_get_pg function that
calls the original alua_check_vpd.  Also please return the
pg from it so that we c

> @@ -596,13 +643,12 @@ static int alua_initialize(struct scsi_device *sdev, 
> struct alua_dh_data *h)
>   goto out;
>  
>   err = alua_check_vpd(sdev, h);
> - if (err != SCSI_DH_OK)
> - goto out;
> -
> - err = alua_rtpg(sdev, h, 0);
> - if (err != SCSI_DH_OK)
> + if (err != SCSI_DH_OK || !h->pg)
>   goto out;

How could we end up here without h->pg? Either way that should
move into a conditional together with the kref_get which should
become the not_zero variant if it is kept.
--
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 23/23] scsi_dh_alua: Update version to 2.0

2015-09-01 Thread Christoph Hellwig 
Looks fine,

Reviewed-by: Christoph Hellwig 
--
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 22/23] scsi_dh_alua: Send TEST UNIT READY to poll for transitioning

2015-09-01 Thread Christoph Hellwig 
On Thu, Aug 27, 2015 at 02:41:20PM +0200, Hannes Reinecke wrote:
> Sending a 'REPORT TARGET PORT GROUP' command is a costly operation,
> as the array has to gather information about all ports.
> So instead of using RTPG to poll for a status update when a port
> is in transitioning we should be sending a TEST UNIT READY, and
> wait for the sense code to report success.

The extra state variabe doesn't really help you as it might be stale
just as quickly.

But otherwise looks fine:

Reviewed-by: Christoph Hellwig 
--
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 21/23] scsi_dh_alua: update all port states

2015-09-01 Thread Christoph Hellwig 
On Thu, Aug 27, 2015 at 02:41:19PM +0200, Hannes Reinecke wrote:
> When we read in the target port group state we should be
> updating all affected port groups, otherwise we risk
> running out of sync.

Looks fine,

Reviewed-by: Christoph Hellwig 
--
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 20/23] scsi_dh_alua: Recheck state on unit attention

2015-09-01 Thread Christoph Hellwig 
> - alua_check(sdev);
> - return ADD_TO_MLQUEUE;
> + alua_check(sdev, false);
> + return NEEDS_RETRY;

What's the reason for the change in return value here?
--
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 17/23] scsi_dh_alua: use unique device id

2015-09-01 Thread Christoph Hellwig 
> +  * Internal error: TPGS supported by no
> +  * device identifcation found.
> +  * Disable ALUA support.

s/by/but/

Also I think the comment could fit into two lines if you try :)

> + spin_lock(&port_group_lock);
> + list_for_each_entry(tmp_pg, &port_group_list, node) {
> + if (tmp_pg->group_id != group_id)
> + continue;
> + if (tmp_pg->device_id_size != device_id_size)
> + continue;
> + if (strncmp(tmp_pg->device_id_str, device_id_str,
> + device_id_size))
> + continue;
> + h->pg = tmp_pg;
> + kref_get(&tmp_pg->kref);
> + break;
> + }
> + spin_unlock(&port_group_lock);

This exact code appears twice in this patch, please factor it into
a helper.

> + if (device_id_size)
> + strncpy(pg->device_id_str, device_id_str, 256);
> + else
> + pg->device_id_str[0] = '\0';
> +

How could we end up with a zero device ID length?  Shouldn't
have error out earlier on that?

> +  * Re-check list again to catch
> +  * concurrent updates
> +  */

Could fit onto one line..
--
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 16/23] scsi: Add scsi_vpd_lun_id()

2015-09-01 Thread Christoph Hellwig 
On Thu, Aug 27, 2015 at 02:41:14PM +0200, Hannes Reinecke wrote:
> Add a function scsi_vpd_lun_id() to return a unique device
> identifcation based on the designation descriptors of
> VPD page 0x83.
> 
> As devices might implement several descriptors the order
> of preference is:
> - NAA IEE Registered Extended
> - EUI-64 based 16-byte
> - EUI-64 based 12-byte
> - NAA IEEE Registered
> - NAA IEEE Extended
> A SCSI name string descriptor is preferred to all of them
> if the identification is longer than 16 bytes.
> 
> The returned unique device identification will be formatted
> as a SCSI Name string to avoid clashes between different
> designator types.

Looks good in general, but I wonder if it might make sense to switch
to kasprintf and let the caller free the buffer?

Otherwise:

Reviewed-by: Christoph Hellwig 
--
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 13/23] scsi_dh_alua: Use separate alua_port_group structure

2015-09-01 Thread Christoph Hellwig 
> +struct alua_dh_data {
> + struct alua_port_group  *pg;
> + int rel_port;
> + int tpgs;

The tpgs file doesn't make much sense in the new alua_dh_data
structure.  Please return it explicitly from alua_check_tpgs and
assign it directly to the member in struct alua_port_group.

> +static void release_port_group(struct kref *kref)
> +{
> + struct alua_port_group *pg;
> +
> + pg = container_of(kref, struct alua_port_group, kref);
> + printk(KERN_WARNING "alua: release port group %d\n", pg->group_id);

Why is this a warning?  I'd consider it nothing but debug noise.

> - err = alua_rtpg(sdev, h, 0);
> - if (err != SCSI_DH_OK)
> + if (err != SCSI_DH_OK || !h->pg)
>   goto out;
>  
> + kref_get(&h->pg->kref);

What prevents this kref_get from racing with the last kref_put?

I think all the kref_get calls in this patch need to be
kref_get_unless_zero with proper error handling.

> + rcu_read_lock();
> + pg = rcu_dereference(h->pg);
> + if (!pg) {
> + rcu_read_unlock();
> + return -ENXIO;
> + }

What is this rcu_read_lock supposed to protect against given
that struct alua_port_group isn't RCU freed?  I think this needs
to be another kref_get_unless_zero.

> +
>   if (optimize)
> - h->flags |= ALUA_OPTIMIZE_STPG;
> + pg->flags |= ALUA_OPTIMIZE_STPG;
>   else
> - h->flags &= ~ALUA_OPTIMIZE_STPG;
> + pg->flags |= ~ALUA_OPTIMIZE_STPG;
> + rcu_read_unlock();

The read-modify-write of pg->flags will need some sort of locking.
Seems like most modifications of it are under pg->rtpg_lock, so
I'd suggest to enforce that as a strict rule.

> + err = alua_rtpg(sdev, h->pg, 1);
> + if (err != SCSI_DH_OK) {
> + kref_put(&h->pg->kref, release_port_group);
> + goto out;

goto out_put_pg;


out_put_pg:
> + kref_put(&h->pg->kref, release_port_group);
>  out:
>   if (fn)
>   fn(data, err);
--
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 12/23] scsi_dh_alua: switch to scsi_execute_req_flags()

2015-09-01 Thread Christoph Hellwig 
Looks good fine, but please add a comment explaining why the
REQ_QUIET and REQ_PREEMPT flags added by scsi_execture are fine,
or in fact desireable.

Reviewed-by: Christoph Hellwig 
--
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 11/23] scsi_dh_alua: Make stpg synchronous

2015-09-01 Thread Christoph Hellwig 
On Thu, Aug 27, 2015 at 02:41:09PM +0200, Hannes Reinecke wrote:
> We should be issuing STPG synchronously as we need to
> evaluate the return code on failure.

This looks right, but I think the patch needs to be split and needs
a way better changelog.

patch 1:
 - factor out the alua_stpg helper

patch 2:
 - async STPG
 - please explain that alua_activate will now block and why this is ok

patch 3:
 - add the new call to alua_rtpg and why it's added
--
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 10/23] scsi_dh_alua: Pass buffer as function argument

2015-09-01 Thread Christoph Hellwig 
On Thu, Aug 27, 2015 at 02:41:08PM +0200, Hannes Reinecke wrote:
> Pass in the buffer as a function argument for submit_rtpg().

Looks fine,

Reviewed-by: Christoph Hellwig 
--
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 09/23] scsi_dh_alua: use unaligned access macros

2015-09-01 Thread Christoph Hellwig 
Looks good,

Reviewed-by: Christoph Hellwig 
--
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 06/23] scsi_dh_alua: fixup description of stpg_endio()

2015-09-01 Thread Christoph Hellwig 
Looks fine,

Reviewed-by: Christoph Hellwig 
--
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 05/23] scsi_dh_alua: return standard SCSI return codes in submit_rtpg

2015-09-01 Thread Christoph Hellwig 
On Thu, Aug 27, 2015 at 02:41:03PM +0200, Hannes Reinecke wrote:
> Fixup submit_rtpg() to always return a standard SCSI return code.

Oh, this fixes the problems in the previous patch up.  Maybe just skip the
error handling changes in the previous patch and keep them purely in this
one?

> + if (driver_byte(retval) == DRIVER_BUSY)
> + err = SCSI_DH_DEV_TEMP_BUSY;
> + else
> + err = SCSI_DH_IO;
> + return err;

This could be simplified to:

if (driver_byte(retval) == DRIVER_BUSY)
return SCSI_DH_DEV_TEMP_BUSY;
return SCSI_DH_IO;
--
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 04/23] scsi_dh_alua: use standard logging functions

2015-09-01 Thread Christoph Hellwig 
On Thu, Aug 27, 2015 at 02:41:02PM +0200, Hannes Reinecke wrote:
>   }
>  
>   err = alua_check_sense(sdev, &sense_hdr);
> - if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry))
> + if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) {
> + sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n",
> + ALUA_DH_NAME);
> + scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
>   goto retry;
> - sdev_printk(KERN_INFO, sdev,
> - "%s: rtpg sense code %02x/%02x/%02x\n",
> - ALUA_DH_NAME, sense_hdr.sense_key,
> - sense_hdr.asc, sense_hdr.ascq);
> - err = SCSI_DH_IO;
> + }
> + sdev_printk(KERN_ERR, sdev, "%s: rtpg failed\n",
> + ALUA_DH_NAME);
> + scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
> + return SCSI_DH_IO;
>   }
> - if (err != SCSI_DH_OK)
> - return err;

I think you need to keep this if, given that submit_rtpg can return
others error than SCSI_DH_IO as well.

While you're at it you might remove the h->senselen check, and
kill the assignment of the scsi_normalize_sense bool return value
to the err variable similar to how you did in stpg_endio. 
--
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 01/23] scsi_dh_alua: Disable ALUA handling for non-disk devices

2015-09-01 Thread Christoph Hellwig 
Looks fine,

Reviewed-by: Christoph Hellwig 
--
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 10/10] scsi_dh: move 'dh_state' sysfs attribute to generic code

2015-09-01 Thread Christoph Hellwig 
On Thu, Aug 27, 2015 at 02:17:03PM +0200, Hannes Reinecke wrote:
> As scsi_dh.c is now always compiled in we should be moving
> the 'dh_state' attribute to the generic code.
> 
> Signed-off-by: Hannes Reinecke 

Looks good,

Reviewed-by: Christoph Hellwig 
--
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] Update scsi host to use ida for host number

2015-09-01 Thread Sagi Grimberg 

On 8/31/2015 11:28 PM, leeman.dun...@gmail.com wrote:

From: Lee Duncan 

Each Scsi_host instance gets a host number starting
at 0, but this is implemented with an atomic integer,
and rollover doesn't seem to have been considered.
Another side-effect of this design is that scsi host
numbers used by iscsi are never reused, thereby  making
rollover more likely. This patch converts Scsi_host
instances to use ida to manage their instance
numbers.

This also means that host instance numbers will be
reused, when available.

Signed-off-by: Lee Duncan 
---
  drivers/scsi/hosts.c | 43 +++
  1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 8bb173e01084..dade75478541 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -33,7 +33,7 @@
  #include 
  #include 
  #include 
-
+#include 
  #include 
  #include 
  #include 
@@ -42,7 +42,8 @@
  #include "scsi_logging.h"


-static atomic_t scsi_host_next_hn = ATOMIC_INIT(0);/* host_no for next new 
host */
+static DEFINE_SPINLOCK(host_index_lock);
+static DEFINE_IDA(host_index_ida);


  static void scsi_host_cls_release(struct device *dev)
@@ -337,6 +338,10 @@ static void scsi_host_dev_release(struct device *dev)

kfree(shost->shost_data);

+   spin_lock(&host_index_lock);
+   ida_remove(&host_index_ida, shost->host_no);
+   spin_unlock(&host_index_lock);


Why not get it in a symmetrical host_put_index()
--
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] mpt2sas: setpci reset kernel oops fix

2015-09-01 Thread Johannes Thumshirn 
Nagarajkumar Narayanan  writes:

> mpt2sas: setpci reset on nytro warpdrive card along with sysfs access and
> cli ioctl access resulted in kernel oops
>
> 1. pci_access_mutex lock added to provide synchronization between IOCTL,
>sysfs, PCI resource handling path
>
> 2. gioc_lock spinlock to protect list operations over multiple
> controllers
>
>
> From c53a1cff4c07528b8b9ec7f6716e94950283e8f9 Mon Sep 17 00:00:00 2001
> From: Nagarajkumar Narayanan 
> Date: Tue, 18 Aug 2015 11:58:13 +0530
> Subject: [PATCH] mpt2sas setpci reset oops fix
>
> In mpt2sas driver due to lack of synchronization between ioctl,
> BRM status access through sysfs, pci resource removal kernel oops
> happen as ioctl path and BRM status sysfs access path still tries
> to access the removed resources
>
> Two locks added to provide syncrhonization
>
> 1. pci_access_mutex: Mutex to synchronize ioctl,sysfs show path and
> pci resource handling. PCI resource freeing will lead to free
> vital hardware/memory resource, which might be in use by cli/sysfs
> path functions resulting in Null pointer reference followed by kernel
> crash. To avoid the above race condition we use mutex syncrhonization
> which ensures the syncrhonization between cli/sysfs_show path
>
> Note: pci_access_mutex is used only if nytro warpdrive cards
> (ioc->is_warpdrive based on device id) are used
> as we could not test this case with other SAS2 HBA cards
> We can remove this check if this behaviour confirmed from other
> cards.
>
> 2. spinlock on list operations over IOCs
>
> Case: when multiple warpdrive cards(IOCs) are in use
> Each IOC will added to the ioc list stucture on initialization.
> Watchdog threads run at regular intervals to check IOC for any
> fault conditions which will trigger the dead_ioc thread to
> deallocate pci resource, resulting deleting the IOC netry from list,
> this deletion need to protected by spinlock to enusre that
> ioc removal is syncrhonized, if not synchronized it might lead to
> list_del corruption as the ioc list is traversed in cli path
>
> Signed-off-by: Nagarajkumar Narayanan 
> ---
> * v4
> - spinlock function moved from spinlock_irqsave to spinlock on gioc lock 
>   as it was not   used in interrupt context
> - added common exit point for pci_access_mutex unlock
>
> * v3
> - fixed lock imbalance, moved acquiring mutex lock out of if condition
>
> * v2
> - removed is_warpdrive condition for pci_access_mutex lock
>
> * v1
> - using DEFINE_SPINLOCK() to initialize the lock at compile time instead
>   of using spin_lock_init
>  drivers/scsi/mpt2sas/mpt2sas_base.c  |6 +
>  drivers/scsi/mpt2sas/mpt2sas_base.h  |   19 -
>  drivers/scsi/mpt2sas/mpt2sas_ctl.c   |   38 +++--
>  drivers/scsi/mpt2sas/mpt2sas_scsih.c |   13 ++-
>  4 files changed, 67 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c 
> b/drivers/scsi/mpt2sas/mpt2sas_base.c
> index 11248de..60fefca 100644
> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
> @@ -112,9 +112,12 @@ _scsih_set_fwfault_debug(const char *val, struct 
> kernel_param *kp)
>   if (ret)
>   return ret;
>  
> + /* global ioc spinlock to protect controller list on list operations */
>   printk(KERN_INFO "setting fwfault_debug(%d)\n", mpt2sas_fwfault_debug);
> + spin_lock(&gioc_lock);
>   list_for_each_entry(ioc, &mpt2sas_ioc_list, list)
>   ioc->fwfault_debug = mpt2sas_fwfault_debug;
> + spin_unlock(&gioc_lock);
>   return 0;
>  }
>  
> @@ -4435,6 +4438,8 @@ mpt2sas_base_free_resources(struct MPT2SAS_ADAPTER *ioc)
>   dexitprintk(ioc, printk(MPT2SAS_INFO_FMT "%s\n", ioc->name,
>   __func__));
>  
> + /* synchronizing freeing resource with pci_access_mutex lock */
> + mutex_lock(&ioc->pci_access_mutex);
>   if (ioc->chip_phys && ioc->chip) {
>   _base_mask_interrupts(ioc);
>   ioc->shost_recovery = 1;
> @@ -4454,6 +4459,7 @@ mpt2sas_base_free_resources(struct MPT2SAS_ADAPTER *ioc)
>   pci_disable_pcie_error_reporting(pdev);
>   pci_disable_device(pdev);
>   }
> + mutex_unlock(&ioc->pci_access_mutex);
>   return;
>  }
>  
> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.h 
> b/drivers/scsi/mpt2sas/mpt2sas_base.h
> index caff8d1..c82bdb3 100644
> --- a/drivers/scsi/mpt2sas/mpt2sas_base.h
> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.h
> @@ -799,6 +799,12 @@ typedef void (*MPT2SAS_FLUSH_RUNNING_CMDS)(struct 
> MPT2SAS_ADAPTER *ioc);
>   * @delayed_tr_list: target reset link list
>   * @delayed_tr_volume_list: volume target reset link list
>   * @@temp_sensors_count: flag to carry the number of temperature sensors
> + * @pci_access_mutex: Mutex to synchronize ioctl,sysfs show path and
> + * pci resource handling. PCI resource freeing will lead to free
> + * vital hardware/memory resource, which might be in use by cli/sysfs
> + * path functions resu

Re: [PATCH] Update scsi host to use ida for host number

2015-09-01 Thread Johannes Thumshirn 
leeman.dun...@gmail.com writes:

> From: Lee Duncan 
>
> Each Scsi_host instance gets a host number starting
> at 0, but this is implemented with an atomic integer,
> and rollover doesn't seem to have been considered.
> Another side-effect of this design is that scsi host
> numbers used by iscsi are never reused, thereby  making
> rollover more likely. This patch converts Scsi_host
> instances to use ida to manage their instance
> numbers.
>
> This also means that host instance numbers will be
> reused, when available.
>
> Signed-off-by: Lee Duncan 
> ---
>  drivers/scsi/hosts.c | 43 +++
>  1 file changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index 8bb173e01084..dade75478541 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -33,7 +33,7 @@
>  #include 
>  #include 
>  #include 
> -
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -42,7 +42,8 @@
>  #include "scsi_logging.h"
>  
>  
> -static atomic_t scsi_host_next_hn = ATOMIC_INIT(0);  /* host_no for next new 
> host */
> +static DEFINE_SPINLOCK(host_index_lock);
> +static DEFINE_IDA(host_index_ida);
>  
>  
>  static void scsi_host_cls_release(struct device *dev)
> @@ -337,6 +338,10 @@ static void scsi_host_dev_release(struct device *dev)
>  
>   kfree(shost->shost_data);
>  
> + spin_lock(&host_index_lock);
> + ida_remove(&host_index_ida, shost->host_no);
> + spin_unlock(&host_index_lock);
> +
>   if (parent)
>   put_device(parent);
>   kfree(shost);
> @@ -353,6 +358,22 @@ static struct device_type scsi_host_type = {
>   .release =  scsi_host_dev_release,
>  };
>  
> +static int host_get_index(int *index)
> +{
> + int error = -ENOMEM;
> +
> + do {
> + if (!ida_pre_get(&host_index_ida, GFP_KERNEL))
> + break;
> + spin_lock(&host_index_lock);
> + error = ida_get_new(&host_index_ida, index);
> + spin_unlock(&host_index_lock);
> + } while (error == -EAGAIN);
> +
> + return error;
> +}
> +
> +
>  /**
>   * scsi_host_alloc - register a scsi host adapter instance.
>   * @sht: pointer to scsi host template
> @@ -370,6 +391,8 @@ struct Scsi_Host *scsi_host_alloc(struct 
> scsi_host_template *sht, int privsize)
>  {
>   struct Scsi_Host *shost;
>   gfp_t gfp_mask = GFP_KERNEL;
> + int index;
> + int error;
>  
>   if (sht->unchecked_isa_dma && privsize)
>   gfp_mask |= __GFP_DMA;
> @@ -388,11 +411,11 @@ struct Scsi_Host *scsi_host_alloc(struct 
> scsi_host_template *sht, int privsize)
>   init_waitqueue_head(&shost->host_wait);
>   mutex_init(&shost->scan_mutex);
>  
> - /*
> -  * subtract one because we increment first then return, but we need to
> -  * know what the next host number was before increment
> -  */
> - shost->host_no = atomic_inc_return(&scsi_host_next_hn) - 1;
> + error = host_get_index(&index);
> + if (error < 0)
> + goto fail_kfree;
> + shost->host_no = index;
> +
>   shost->dma_channel = 0xff;
>  
>   /* These three are default values which can be overridden */
> @@ -477,7 +500,7 @@ struct Scsi_Host *scsi_host_alloc(struct 
> scsi_host_template *sht, int privsize)
>   shost_printk(KERN_WARNING, shost,
>   "error handler thread failed to spawn, error = %ld\n",
>   PTR_ERR(shost->ehandler));
> - goto fail_kfree;
> + goto fail_free_index;
>   }
>  
>   shost->tmf_work_q = alloc_workqueue("scsi_tmf_%d",
> @@ -493,6 +516,10 @@ struct Scsi_Host *scsi_host_alloc(struct 
> scsi_host_template *sht, int privsize)
>  
>   fail_kthread:
>   kthread_stop(shost->ehandler);
> + fail_free_index:
> + spin_lock(&host_index_lock);
> + ida_remove(&host_index_ida, index);
> + spin_unlock(&host_index_lock);
>   fail_kfree:
>   kfree(shost);
>   return NULL;

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn   Storage
jthumsh...@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600  D0D0 0393 969D 2D76 0850
--
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] Update scsi host to use ida for host number

2015-09-01 Thread Hannes Reinecke 
On 08/31/2015 10:28 PM, leeman.dun...@gmail.com wrote:
> From: Lee Duncan 
> 
> Each Scsi_host instance gets a host number starting
> at 0, but this is implemented with an atomic integer,
> and rollover doesn't seem to have been considered.
> Another side-effect of this design is that scsi host
> numbers used by iscsi are never reused, thereby  making
> rollover more likely. This patch converts Scsi_host
> instances to use ida to manage their instance
> numbers.
> 
> This also means that host instance numbers will be
> reused, when available.
> 
> Signed-off-by: Lee Duncan 
> ---
>  drivers/scsi/hosts.c | 43 +++
>  1 file changed, 35 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index 8bb173e01084..dade75478541 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -33,7 +33,7 @@
>  #include 
>  #include 
>  #include 
> -
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -42,7 +42,8 @@
>  #include "scsi_logging.h"
>  
>  
> -static atomic_t scsi_host_next_hn = ATOMIC_INIT(0);  /* host_no for next new 
> host */
> +static DEFINE_SPINLOCK(host_index_lock);
> +static DEFINE_IDA(host_index_ida);
>  
>  
>  static void scsi_host_cls_release(struct device *dev)
> @@ -337,6 +338,10 @@ static void scsi_host_dev_release(struct device *dev)
>  
>   kfree(shost->shost_data);
>  
> + spin_lock(&host_index_lock);
> + ida_remove(&host_index_ida, shost->host_no);
> + spin_unlock(&host_index_lock);
> +
>   if (parent)
>   put_device(parent);
>   kfree(shost);
> @@ -353,6 +358,22 @@ static struct device_type scsi_host_type = {
>   .release =  scsi_host_dev_release,
>  };
>  
> +static int host_get_index(int *index)
> +{
> + int error = -ENOMEM;
> +
> + do {
> + if (!ida_pre_get(&host_index_ida, GFP_KERNEL))
> + break;
> + spin_lock(&host_index_lock);
> + error = ida_get_new(&host_index_ida, index);
> + spin_unlock(&host_index_lock);
> + } while (error == -EAGAIN);
> +
> + return error;
> +}
> +
> +
>  /**
>   * scsi_host_alloc - register a scsi host adapter instance.
>   * @sht: pointer to scsi host template
> @@ -370,6 +391,8 @@ struct Scsi_Host *scsi_host_alloc(struct 
> scsi_host_template *sht, int privsize)
>  {
>   struct Scsi_Host *shost;
>   gfp_t gfp_mask = GFP_KERNEL;
> + int index;
> + int error;
>  
>   if (sht->unchecked_isa_dma && privsize)
>   gfp_mask |= __GFP_DMA;
> @@ -388,11 +411,11 @@ struct Scsi_Host *scsi_host_alloc(struct 
> scsi_host_template *sht, int privsize)
>   init_waitqueue_head(&shost->host_wait);
>   mutex_init(&shost->scan_mutex);
>  
> - /*
> -  * subtract one because we increment first then return, but we need to
> -  * know what the next host number was before increment
> -  */
> - shost->host_no = atomic_inc_return(&scsi_host_next_hn) - 1;
> + error = host_get_index(&index);
> + if (error < 0)
> + goto fail_kfree;
> + shost->host_no = index;
> +
>   shost->dma_channel = 0xff;
>  
>   /* These three are default values which can be overridden */
> @@ -477,7 +500,7 @@ struct Scsi_Host *scsi_host_alloc(struct 
> scsi_host_template *sht, int privsize)
>   shost_printk(KERN_WARNING, shost,
>   "error handler thread failed to spawn, error = %ld\n",
>   PTR_ERR(shost->ehandler));
> - goto fail_kfree;
> + goto fail_free_index;
>   }
>  
>   shost->tmf_work_q = alloc_workqueue("scsi_tmf_%d",
> @@ -493,6 +516,10 @@ struct Scsi_Host *scsi_host_alloc(struct 
> scsi_host_template *sht, int privsize)
>  
>   fail_kthread:
>   kthread_stop(shost->ehandler);
> + fail_free_index:
> + spin_lock(&host_index_lock);
> + ida_remove(&host_index_ida, index);
> + spin_unlock(&host_index_lock);
>   fail_kfree:
>   kfree(shost);
>   return NULL;
> 
Reviewed-by: Hannes Reinecke 

Cheers,

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