Re: WARNING: CPU: 1 PID: 495 at mm/slab_common.c:69 kmem_cache_create+0x1a9/0x330()

[Vladimir Davydov]

On Sat, Jul 26, 2014 at 06:21:02PM +0200, Christoph Hellwig wrote:
 Here's a formal one.
 
 James, can I get your signoff for it?
 
 Vladimir, can I get a reviewed-by from you (or anyone else)?
 
 ---
 From 73b1034ab1418e2dea75ccf642bc85c728b57313 Mon Sep 17 00:00:00 2001
 From: James Bottomley james.bottom...@hansenpartnership.com
 Date: Sat, 26 Jul 2014 12:21:26 -0400
 Subject: scsi: use short driver name for per-driver cmd slab caches
 
 hostt-name might contain space, so use the -proc_name short name instead
 when creating per-driver command slabs.
 
 Reported-by: Vladimir Davydov vdavy...@parallels.com
 Tested-by: Vladimir Davydov vdavy...@parallels.com

Actually, it was poma pomidorabelis...@gmail.com, not me, who
reported the issue and tested the patch. Please fix the tags.

Reviewed-by: Vladimir Davydov vdavy...@parallels.com

 ---
  drivers/scsi/scsi.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
 index 33318f5..df33060 100644
 --- a/drivers/scsi/scsi.c
 +++ b/drivers/scsi/scsi.c
 @@ -365,8 +365,8 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
   if (!pool)
   return NULL;
  
 - pool-cmd_name = kasprintf(GFP_KERNEL, %s_cmd, hostt-name);
 - pool-sense_name = kasprintf(GFP_KERNEL, %s_sense, hostt-name);
 + pool-cmd_name = kasprintf(GFP_KERNEL, %s_cmd, hostt-proc_name);
 + pool-sense_name = kasprintf(GFP_KERNEL, %s_sense, hostt-proc_name);
   if (!pool-cmd_name || !pool-sense_name) {
   scsi_free_host_cmd_pool(pool);
   return NULL;
 -- 
 1.9.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 v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

[Boaz Harrosh]

On 06/25/2014 04:17 AM, Vladislav Bolkhovitin wrote:
 Martin K. Petersen, on 06/23/2014 06:58 PM wrote:
 Mike == Mike Christie micha...@cs.wisc.edu writes:
 + unsigned int xfer_len = blk_rq_bytes(scmd-request);

 Mike Can you do bidi and dif/dix?

 Nope.
 
 Correction: at the moment.
 
 There is a proposal of READ GATHERED command, which is bidirectional and 
 potentially 
 DIF/DIX.
 

That's all very good. But current infrastructure can not support BIDI+DIFF.
Because you we'll need *two* diff vectors one for each side.

Now in fact at the block layer this is actually supported. Since BIDI is
two requests, and each can have its own DIFF bio. But on the scsi layer
this is not implemented.

So I'd say: *For now DIFF and BIDI are mutually exclusive.*

(You'll need someone to love these new commands a lot in order to
 enable it)

 Vlad
 

Cheers
Boaz

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

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

[Boaz Harrosh]

On 06/25/2014 01:32 PM, Sagi Grimberg wrote:
 On 6/25/2014 11:48 AM, Sagi Grimberg wrote:
 
 SNIP

 I made the patch below which should fix both bidi
 support in iscsi and also WRITE_SAME (and similar commands) support.

 I'm a bit confused, for all commands (bidi or not) the iscsi header 
 data_length
 should describe the scsi_data_buffer length, bidi information will lie 
 in AHS header.
 (in case bidi will ever co-exist with PI, we might need another helper 
 that will look
 in req-special + PI, something like scsi_bidi_transfer_length)

 So why not keep scsi_transfer_length to work on sdb length (take 
 scsi_bufflen(scmnd) or
 scsi_out(scmnd)-length as MKP suggested) and that's it - don't touch 
 libiscsi.

 Let me test that.

 
 So I tested a bidirectional command using:
 $ sg_raw --infile=/root/filex --send=1024 --request=1024 
 --outfile=/root/filex /dev/bsg/7:0:0:0 53 00 00 00 00 00 00 00 02 00
 
 And I see:
 kernel: session1: iscsi_prep_scsi_cmd_pdu iscsi prep [bidirectional cid 
 0 sc 880468ca1e00 cdb 0x53 itt 0x16 len 1024 bidi_len 1024 cmdsn 223 
 win 64]
 

This is a very bad example and tested nothing, since len  bidi_len
are the same. So even if you had a bug and took length from the
wrong side it would come out the same.

You must test with a bidi command that has two different lengths for
each side

If you have a tree that you want me to test I will be glad too.
From this thread I'm confused as to what patches you want me to
test? please point me to a tree you need testing. You can bug me
any time, any tree. I will be happy to test.

Cheers
Boaz

 This confirms what I wrote above, so AFAICT no additional fix is 
 required for libiscsi wrt bidi commands support.
 
 Mike?
 
 Sagi.
 --
 To unsubscribe from this list: send the line unsubscribe linux-scsi in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

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

Re: [PATCH v2 2/3] libiscsi, iser: Adjust data_length to include protection information

[Boaz Harrosh]

On 06/11/2014 12:09 PM, Sagi Grimberg wrote:
 In case protection information exists over the wire
 iscsi header data length is required to include it.
 Use protection information aware scsi helpers to set
 the correct transfer length.
 
 In order to avoid breakage, remove iser transfer length
 checks for each task as they are not always true and
 somewhat redundant anyway.
 
 Signed-off-by: Sagi Grimberg sa...@mellanox.com
 Reviewed-by: Mike Christie micha...@cs.wisc.edu
 ---
  drivers/infiniband/ulp/iser/iser_initiator.c |   34 +++--
  drivers/scsi/libiscsi.c  |   18 +++---
  2 files changed, 19 insertions(+), 33 deletions(-)
 
 diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c 
 b/drivers/infiniband/ulp/iser/iser_initiator.c
 index 2e2d903..8d44a40 100644
 --- a/drivers/infiniband/ulp/iser/iser_initiator.c
 +++ b/drivers/infiniband/ulp/iser/iser_initiator.c
 @@ -41,11 +41,11 @@
  #include iscsi_iser.h
  
  /* Register user buffer memory and initialize passive rdma
 - *  dto descriptor. Total data size is stored in
 - *  iser_task-data[ISER_DIR_IN].data_len
 + *  dto descriptor. Data size is stored in
 + *  task-data[ISER_DIR_IN].data_len, Protection size
 + *  os stored in task-prot[ISER_DIR_IN].data_len
   */
 -static int iser_prepare_read_cmd(struct iscsi_task *task,
 -  unsigned int edtl)
 +static int iser_prepare_read_cmd(struct iscsi_task *task)
  
  {
   struct iscsi_iser_task *iser_task = task-dd_data;
 @@ -73,14 +73,6 @@ static int iser_prepare_read_cmd(struct iscsi_task *task,
   return err;
   }
  
 - if (edtl  iser_task-data[ISER_DIR_IN].data_len) {
 - iser_err(Total data length: %ld, less than EDTL: 
 -  %d, in READ cmd BHS itt: %d, conn: 0x%p\n,
 -  iser_task-data[ISER_DIR_IN].data_len, edtl,
 -  task-itt, iser_task-ib_conn);
 - return -EINVAL;
 - }
 -
   err = device-iser_reg_rdma_mem(iser_task, ISER_DIR_IN);
   if (err) {
   iser_err(Failed to set up Data-IN RDMA\n);
 @@ -100,8 +92,9 @@ static int iser_prepare_read_cmd(struct iscsi_task *task,
  }
  
  /* Register user buffer memory and initialize passive rdma
 - *  dto descriptor. Total data size is stored in
 - *  task-data[ISER_DIR_OUT].data_len
 + *  dto descriptor. Data size is stored in
 + *  task-data[ISER_DIR_OUT].data_len, Protection size
 + *  is stored at task-prot[ISER_DIR_OUT].data_len
   */
  static int
  iser_prepare_write_cmd(struct iscsi_task *task,
 @@ -135,14 +128,6 @@ iser_prepare_write_cmd(struct iscsi_task *task,
   return err;
   }
  
 - if (edtl  iser_task-data[ISER_DIR_OUT].data_len) {
 - iser_err(Total data length: %ld, less than EDTL: %d, 
 -  in WRITE cmd BHS itt: %d, conn: 0x%p\n,
 -  iser_task-data[ISER_DIR_OUT].data_len,
 -  edtl, task-itt, task-conn);
 - return -EINVAL;
 - }
 -
   err = device-iser_reg_rdma_mem(iser_task, ISER_DIR_OUT);
   if (err != 0) {
   iser_err(Failed to register write cmd RDMA mem\n);
 @@ -417,11 +402,12 @@ int iser_send_command(struct iscsi_conn *conn,
   if (scsi_prot_sg_count(sc)) {
   prot_buf-buf  = scsi_prot_sglist(sc);
   prot_buf-size = scsi_prot_sg_count(sc);
 - prot_buf-data_len = sc-prot_sdb-length;
 + prot_buf-data_len = data_buf-data_len 
 +  ilog2(sc-device-sector_size) * 8;
   }
  
   if (hdr-flags  ISCSI_FLAG_CMD_READ) {
 - err = iser_prepare_read_cmd(task, edtl);
 + err = iser_prepare_read_cmd(task);
   if (err)
   goto send_command_error;
   }
 diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
 index 26dc005..3f46234 100644
 --- a/drivers/scsi/libiscsi.c
 +++ b/drivers/scsi/libiscsi.c
 @@ -338,7 +338,7 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task 
 *task)
   struct iscsi_session *session = conn-session;
   struct scsi_cmnd *sc = task-sc;
   struct iscsi_scsi_req *hdr;
 - unsigned hdrlength, cmd_len;
 + unsigned hdrlength, cmd_len, transfer_length;

I hate that you introduced this new transfer_length variable. It does
not exist. In BIDI supporting driver there is out_len and in_len just
as original code.

   itt_t itt;
   int rc;
  
 @@ -391,11 +391,11 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task 
 *task)
   if (scsi_get_prot_op(sc) != SCSI_PROT_NORMAL)
   task-protected = true;
  
 + transfer_length = scsi_transfer_length(sc);
 + hdr-data_length = cpu_to_be32(transfer_length);
   if (sc-sc_data_direction == DMA_TO_DEVICE) {
 - unsigned out_len = scsi_out(sc)-length;

In light of all the comments and the obvious bugs, please just
re do this patch. Do not just 

Re: [PATCH v2 RESEND 18/23] pm8001: Use pci_enable_msix_exact() instead of pci_enable_msix()

[Jack Wang]

Hi Alex,

Looks Ok for me.
Please feel free to add my:
Reviewed-by: Jack Wang xjtu...@gmail.com

Thanks,
Jack

2014-07-26 10:33 GMT+02:00 Alexander Gordeev agord...@redhat.com:
 On Wed, Jul 16, 2014 at 08:05:22PM +0200, Alexander Gordeev wrote:
 As result of deprecation of MSI-X/MSI enablement functions
 pci_enable_msix() and pci_enable_msi_block() all drivers
 using these two interfaces need to be updated to use the
 new pci_enable_msi_range()  or pci_enable_msi_exact()
 and pci_enable_msix_range() or pci_enable_msix_exact()
 interfaces.

 Hi Jack, Lindar,

 Could you please review this patch?

 Thanks!

 Signed-off-by: Alexander Gordeev agord...@redhat.com
 Cc: xjtu...@gmail.com
 Cc: lindar_...@usish.com
 Cc: linux-scsi@vger.kernel.org
 Cc: linux-...@vger.kernel.org
 ---
  drivers/scsi/pm8001/pm8001_init.c |   39 
 +++--
  1 files changed, 20 insertions(+), 19 deletions(-)

 diff --git a/drivers/scsi/pm8001/pm8001_init.c 
 b/drivers/scsi/pm8001/pm8001_init.c
 index e837ece..4057c24 100644
 --- a/drivers/scsi/pm8001/pm8001_init.c
 +++ b/drivers/scsi/pm8001/pm8001_init.c
 @@ -729,34 +729,35 @@ static u32 pm8001_setup_msix(struct pm8001_hba_info 
 *pm8001_ha)
   sizeof(pm8001_ha-msix_entries[0]);
   for (i = 0; i  max_entry ; i++)
   pm8001_ha-msix_entries[i].entry = i;
 - rc = pci_enable_msix(pm8001_ha-pdev, pm8001_ha-msix_entries,
 + rc = pci_enable_msix_exact(pm8001_ha-pdev, pm8001_ha-msix_entries,
   number_of_intr);
   pm8001_ha-number_of_intr = number_of_intr;
 - if (!rc) {
 - PM8001_INIT_DBG(pm8001_ha, pm8001_printk(
 - pci_enable_msix request ret:%d no of intr %d\n,
 - rc, pm8001_ha-number_of_intr));
 + if (rc)
 + return rc;

 + PM8001_INIT_DBG(pm8001_ha, pm8001_printk(
 + pci_enable_msix_exact request ret:%d no of intr %d\n,
 + rc, pm8001_ha-number_of_intr));

 - for (i = 0; i  number_of_intr; i++) {
 - snprintf(intr_drvname[i], sizeof(intr_drvname[0]),
 - DRV_NAME%d, i);
 - pm8001_ha-irq_vector[i].irq_id = i;
 - pm8001_ha-irq_vector[i].drv_inst = pm8001_ha;
 + for (i = 0; i  number_of_intr; i++) {
 + snprintf(intr_drvname[i], sizeof(intr_drvname[0]),
 + DRV_NAME%d, i);
 + pm8001_ha-irq_vector[i].irq_id = i;
 + pm8001_ha-irq_vector[i].drv_inst = pm8001_ha;

 - rc = request_irq(pm8001_ha-msix_entries[i].vector,
 - pm8001_interrupt_handler_msix, flag,
 - intr_drvname[i], (pm8001_ha-irq_vector[i]));
 - if (rc) {
 - for (j = 0; j  i; j++)
 - free_irq(
 - pm8001_ha-msix_entries[j].vector,
 + rc = request_irq(pm8001_ha-msix_entries[i].vector,
 + pm8001_interrupt_handler_msix, flag,
 + intr_drvname[i], (pm8001_ha-irq_vector[i]));
 + if (rc) {
 + for (j = 0; j  i; j++) {
 + free_irq(pm8001_ha-msix_entries[j].vector,
   (pm8001_ha-irq_vector[i]));
 - pci_disable_msix(pm8001_ha-pdev);
 - break;
   }
 + pci_disable_msix(pm8001_ha-pdev);
 + break;
   }
   }
 +
   return rc;
  }
  #endif
 --
 1.7.7.6


 --
 Regards,
 Alexander Gordeev
 agord...@redhat.com
--
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 v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

[Christoph Hellwig]

On Sun, Jul 27, 2014 at 12:11:11PM +0300, Boaz Harrosh wrote:
 If you have a tree that you want me to test I will be glad too.
 From this thread I'm confused as to what patches you want me to
 test? please point me to a tree you need testing. You can bug me
 any time, any tree. I will be happy to test.

You're about a month late to the game :)

I think everything is sorted out properly, but if you want to verify
it yourself just test the latest 3.16 release candidate from Linus.

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

[PATCH] scsi: qla4xxx: ql4_mbx.c: Cleaning up missing null-terminate in conjunction with strncpy

[Rickard Strandqvist]

Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
And in some cases modified to copy one character less than the overall
length, as the entire area is already zeroed.

Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
---
 drivers/scsi/qla4xxx/ql4_mbx.c |   14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_mbx.c b/drivers/scsi/qla4xxx/ql4_mbx.c
index 0a3312c..2b2756e 100644
--- a/drivers/scsi/qla4xxx/ql4_mbx.c
+++ b/drivers/scsi/qla4xxx/ql4_mbx.c
@@ -1620,8 +1620,8 @@ int qla4xxx_get_chap(struct scsi_qla_host *ha, char 
*username, char *password,
goto exit_get_chap;
}
 
-   strncpy(password, chap_table-secret, QL4_CHAP_MAX_SECRET_LEN);
-   strncpy(username, chap_table-name, QL4_CHAP_MAX_NAME_LEN);
+   strlcpy(password, chap_table-secret, QL4_CHAP_MAX_SECRET_LEN);
+   strlcpy(username, chap_table-name, QL4_CHAP_MAX_NAME_LEN);
chap_table-cookie = __constant_cpu_to_le16(CHAP_VALID_COOKIE);
 
 exit_get_chap:
@@ -1663,8 +1663,8 @@ int qla4xxx_set_chap(struct scsi_qla_host *ha, char 
*username, char *password,
else
chap_table-flags |= BIT_7; /* local */
chap_table-secret_len = strlen(password);
-   strncpy(chap_table-secret, password, MAX_CHAP_SECRET_LEN);
-   strncpy(chap_table-name, username, MAX_CHAP_NAME_LEN);
+   strncpy(chap_table-secret, password, MAX_CHAP_SECRET_LEN - 1);
+   strncpy(chap_table-name, username, MAX_CHAP_NAME_LEN - 1);
chap_table-cookie = __constant_cpu_to_le16(CHAP_VALID_COOKIE);
 
if (is_qla40XX(ha)) {
@@ -1742,8 +1742,8 @@ int qla4xxx_get_uni_chap_at_index(struct scsi_qla_host 
*ha, char *username,
goto exit_unlock_uni_chap;
}
 
-   strncpy(password, chap_table-secret, MAX_CHAP_SECRET_LEN);
-   strncpy(username, chap_table-name, MAX_CHAP_NAME_LEN);
+   strlcpy(password, chap_table-secret, MAX_CHAP_SECRET_LEN);
+   strlcpy(username, chap_table-name, MAX_CHAP_NAME_LEN);
 
rval = QLA_SUCCESS;
 
@@ -2295,7 +2295,7 @@ int qla4_8xxx_set_param(struct scsi_qla_host *ha, int 
param)
if (param == SET_DRVR_VERSION) {
mbox_cmd[1] = SET_DRVR_VERSION;
strncpy((char *)mbox_cmd[2], QLA4XXX_DRIVER_VERSION,
-   MAX_DRVR_VER_LEN);
+   MAX_DRVR_VER_LEN - 1);
} else {
ql4_printk(KERN_ERR, ha, %s: invalid parameter 0x%x\n,
   __func__, param);
-- 
1.7.10.4

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

Re: [PATCH 4/5] [SCSI] Do not use platform_bus as a parent

[Greg Kroah-Hartman]

On Sun, Jul 27, 2014 at 07:52:57AM +0400, James Bottomley wrote:
 On Sat, 2014-07-26 at 13:11 -0700, Greg Kroah-Hartman wrote:
  On Fri, Jul 25, 2014 at 07:46:56AM -0700, James Bottomley wrote:
   On Fri, 2014-07-25 at 15:23 +0100, Pawel Moll wrote:
The host devices without a parent were forcefully adopted
by platform bus. This patch removes this assignment. In
effect the dev_dev may be NULL now, which means ISA.

Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Pawel Moll pawel.m...@arm.com
---

This patch is a part of effort to remove references to platform_bus
and make it static.

James, could you please have a look and advice if the change is
correct? Would you happen to know the real reasons behind
using the root platform_bus device a parent?
   
   Yes, for DMA purposes, the parent cannot now be NULL; we'll get a panic
   in the DMA transfers if it is.  A lot of the legacy ISA device on x86
   and I thought some ARM SOC devices don't pass in the parent device, so
   we hang them off a known parent.
  
  The generic platform bus device is not a known parent.  I don't
  understand the difference between just setting the parent to be NULL,
  which will then have a proper parent pointer filled in by the driver
  core when the device is registered, or faking it out here.  What is the
  difference?
 
 If you set the parent to NULL, the host template dma_dev will end up
 NULL as well and that will trigger a NULL deref panic in the dma segment
 routines.

 If you want to remove platform_bus, we have to have a well known device
 to set dma_dev to at scsi_host_add time.

Why not set the dma_dev after you call device_add()?  That way you will
pick up the right parent no matter what.

  In the end, the device always ends up with a parent pointer, right?
 
 The parent pointer isn't the problem ... assigning the correct dma
 device is.

Ah, ok, it's a scsi core thing, not a driver core thing, that's less
confusing now.  For a fallback of a platform device, if you switch the
lines around you should be fine, something like this patch perhaps:

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 3cbb57a8b846..d8d3b294f5bc 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -218,16 +218,16 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, 
struct device *dev,
goto fail;
 
if (!shost-shost_gendev.parent)
-   shost-shost_gendev.parent = dev ? dev : platform_bus;
-   if (!dma_dev)
-   dma_dev = shost-shost_gendev.parent;
-
-   shost-dma_dev = dma_dev;
+   shost-shost_gendev.parent = dev;
 
error = device_add(shost-shost_gendev);
if (error)
goto out;
 
+   if (!dma_dev)
+   dma_dev = shost-shost_gendev.parent;
+   shost-dma_dev = dma_dev;
+
pm_runtime_set_active(shost-shost_gendev);
pm_runtime_enable(shost-shost_gendev);
device_enable_async_suspend(shost-shost_gendev);
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: 3w-9xxx.c: Cleaning up missing null-terminate in conjunction with strncpy

[Rickard Strandqvist]

Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
And use the sizeof on the to string rather than strlen on the from string.

Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
---
 drivers/scsi/3w-9xxx.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index 0a73253..f4d2331 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -621,7 +621,8 @@ static int twa_check_srl(TW_Device_Extension *tw_dev, int 
*flashed)
}
 
/* Load rest of compatibility struct */
-   strncpy(tw_dev-tw_compat_info.driver_version, TW_DRIVER_VERSION, 
strlen(TW_DRIVER_VERSION));
+   strlcpy(tw_dev-tw_compat_info.driver_version, TW_DRIVER_VERSION,
+   sizeof(tw_dev-tw_compat_info.driver_version));
tw_dev-tw_compat_info.driver_srl_high = TW_CURRENT_DRIVER_SRL;
tw_dev-tw_compat_info.driver_branch_high = TW_CURRENT_DRIVER_BRANCH;
tw_dev-tw_compat_info.driver_build_high = TW_CURRENT_DRIVER_BUILD;
-- 
1.7.10.4

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

[PATCH] scsi: bfa: bfa_fcbuild.c: Cleaning up missing null-terminate in conjunction with strncpy

[Rickard Strandqvist]

Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
And use the sizeof on the to string rather than strlen on the from string.

Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
---
 drivers/scsi/bfa/bfa_fcbuild.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_fcbuild.c b/drivers/scsi/bfa/bfa_fcbuild.c
index dce787f..9294c14 100644
--- a/drivers/scsi/bfa/bfa_fcbuild.c
+++ b/drivers/scsi/bfa/bfa_fcbuild.c
@@ -1249,8 +1249,8 @@ fc_rspnid_build(struct fchs_s *fchs, void *pyld, u32 
s_id, u16 ox_id,
memset(rspnid, 0, sizeof(struct fcgs_rspnid_req_s));
 
rspnid-dap = s_id;
-   rspnid-spn_len = (u8) strlen((char *)name);
-   strncpy((char *)rspnid-spn, (char *)name, rspnid-spn_len);
+   strlcpy((char *)rspnid-spn, (char *)name, sizeof(rspnid-spn));
+   rspnid-spn_len = (u8) strlen((char *)rspnid-spn);
 
return sizeof(struct fcgs_rspnid_req_s) + sizeof(struct ct_hdr_s);
 }
@@ -1270,8 +1270,8 @@ fc_rsnn_nn_build(struct fchs_s *fchs, void *pyld, u32 
s_id,
memset(rsnn_nn, 0, sizeof(struct fcgs_rsnn_nn_req_s));
 
rsnn_nn-node_name = node_name;
-   rsnn_nn-snn_len = (u8) strlen((char *)name);
-   strncpy((char *)rsnn_nn-snn, (char *)name, rsnn_nn-snn_len);
+   strlcpy((char *)rsnn_nn-snn, (char *)name, sizeof(rsnn_nn-snn));
+   rsnn_nn-snn_len = (u8) strlen((char *)rsnn_nn-snn);
 
return sizeof(struct fcgs_rsnn_nn_req_s) + sizeof(struct ct_hdr_s);
 }
-- 
1.7.10.4

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

Re: [RFC] hpsa: work in progress lockless monster patches

[Hannes Reinecke]

On 07/25/2014 09:28 PM, scame...@beardog.cce.hp.com wrote:


hpsa: Work In Progress: lockless monster patches

To be clear, I am not suggesting that these patches be merged at this time.

This patchset is vs. Christoph Hellwig's scsi-mq.4 branch which
may be found here: git://git.infradead.org/users/hch/scsi.git

We've been working for a long time on a patchset for hpsa to remove
all the locks from the main i/o path in pursuit of high IOPS.  Some
of that work is already upstream, but a lot more of it is not quite
yet ready to be merged.  However, I think we've gone dark for a bit
too long on this, and even though the patches aren't really ready to
be merged just yet, I thought I should let other people who might be
interested have a look anyway, as things are starting to be at least
more stable than unstable.  Be warned though, there are still some
problems, esp. around error recovery.

That being said, with the right hardware (fast SAS SSDs, recent Smart
Arrays e.g. P430, with up-to-date firmware, attached to recent disk enclosures)
with these patches and the scsi-mq patches, it is possible to get around
~970k IOPS from a single logical drive on a single controller.

There are about 150 patches in this set.  Rather than bomb the list
with that, here is a link to a tarball of the patches in the form of
an stgit patch series:

https://github.com/smcameron/hpsa-lockless-patches-work-in-progress/blob/master/hpsa-lockless-vs-hch-scsi-mq.4-2014-07-25-1415CDT.tar.bz2?raw=true

In some cases, I have probably erred on the side of having too many too
small patches, on the theory that it is easier to bake a cake than to
unbake a cake.  Before these are submitted for reals, there will be
some squashing of patches, along with other cleaning up.

There are some patches in this set which are already upstream in
James's tree which do not happen to be in Christoph's tree
(most of which are named *_sent_to_james).  There are also
quite a few patches which are strictly for debugging and are not
ever intended to be merged.

Hmm. While you're about to engage on a massive rewrite _and_ we're 
having 64bit LUN support now, what about getting rid of the weird 
internal LUN mapping? That way you would get rid of this LUN rescan 
thingie and the driver would look more sane.

Plus the REPORT LUN command would actually return the correct data ...

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: break from queue depth adjusting loops when device found

[Hannes Reinecke]

On 07/26/2014 06:17 PM, Christoph Hellwig wrote:

On Sat, Jul 26, 2014 at 11:14:35AM -0500, Stephen Cameron wrote:

Hmm, I forgot that that patch was in there, I wasn't trying to keep pushing
it along.  From the previous discussion, I got the impression I was simply
wrong, and that this patch wasn't needed, so I had meant to drop it, I just
forgot to actually drop it.


It's more complicated - as Robert indicated you're tenically right, although
in practice it's not what the existing users expect.  If you have some
cycles for it I'd love to at lest get the per-LUN and per-target
ramp up/down in ASAP.  We can then start looking into doing it even
better based on the target response later on.

The current implementation is based on the needs of the FC HBAs, which 
would need a way to throttle I/O to avoid flooding a target port.
The reason it was done per target is (from what I can tell) due to a 
specific implementation from a large vendor which is using a 
per-target-port request queue.
And more often than not defaulting to SCSI-2 conformance to be 'legacy 
compatible'. So no way one could use any shiny new commands.


Having said that it has been quite some time since it's been 
implemented, and it _might_ be that things have changed and we can get 
away with a per-LUN throttling. As I doubt we'll get information about 
this from the various storage vendors (at least not from those we've got 
issues with even today) we probably have to bite the bullet here and 
test things out.


But hey, it's worth a shot anyway. So, storage vendors, speak up!

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

[PATCH] scsi: bfa: bfa_fcs_lport.c: Cleaning up missing null-terminate in conjunction with strncpy

[Rickard Strandqvist]

Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
In some cases modified to copy one character less than the overall
length, as the entire area is already zeroed. And replacing strncat
with strlcat because of incorrect use.

Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
---
 drivers/scsi/bfa/bfa_fcs_lport.c |   47 +++---
 1 file changed, 19 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_fcs_lport.c b/drivers/scsi/bfa/bfa_fcs_lport.c
index ff75ef8..89e97ee 100644
--- a/drivers/scsi/bfa/bfa_fcs_lport.c
+++ b/drivers/scsi/bfa/bfa_fcs_lport.c
@@ -2631,10 +2631,10 @@ bfa_fcs_fdmi_get_hbaattr(struct bfa_fcs_lport_fdmi_s 
*fdmi,
hba_attr-fw_version);
 
strncpy(hba_attr-driver_version, (char *)driver_info-version,
-   sizeof(hba_attr-driver_version));
+   sizeof(hba_attr-driver_version) - 1);
 
strncpy(hba_attr-os_name, driver_info-host_os_name,
-   sizeof(hba_attr-os_name));
+   sizeof(hba_attr-os_name) - 1);
 
/*
 * If there is a patch level, append it
@@ -2652,12 +2652,13 @@ bfa_fcs_fdmi_get_hbaattr(struct bfa_fcs_lport_fdmi_s 
*fdmi,
hba_attr-max_ct_pyld = fcs_port_attr.max_frm_size;
 
strncpy(hba_attr-node_sym_name.symname,
-   port-port_cfg.node_sym_name.symname, BFA_SYMNAME_MAXLEN);
+   port-port_cfg.node_sym_name.symname, BFA_SYMNAME_MAXLEN - 1);
strcpy(hba_attr-vendor_info, BROCADE);
hba_attr-num_ports =
cpu_to_be32(bfa_ioc_get_nports(port-fcs-bfa-ioc));
hba_attr-fabric_name = port-fabric-lps-pr_nwwn;
-   strncpy(hba_attr-bios_ver, hba_attr-option_rom_ver, BFA_VERSION_LEN);
+   strncpy(hba_attr-bios_ver, hba_attr-option_rom_ver,
+   BFA_VERSION_LEN - 1);
 
 }
 
@@ -2725,19 +2726,20 @@ bfa_fcs_fdmi_get_portattr(struct bfa_fcs_lport_fdmi_s 
*fdmi,
 * OS device Name
 */
strncpy(port_attr-os_device_name, (char *)driver_info-os_device_name,
-   sizeof(port_attr-os_device_name));
+   sizeof(port_attr-os_device_name) - 1);
 
/*
 * Host name
 */
strncpy(port_attr-host_name, (char *)driver_info-host_machine_name,
-   sizeof(port_attr-host_name));
+   sizeof(port_attr-host_name) - 1);
 
port_attr-node_name = bfa_fcs_lport_get_nwwn(port);
port_attr-port_name = bfa_fcs_lport_get_pwwn(port);
 
strncpy(port_attr-port_sym_name.symname,
-   (char *)bfa_fcs_lport_get_psym_name(port), BFA_SYMNAME_MAXLEN);
+   (char *)bfa_fcs_lport_get_psym_name(port),
+   BFA_SYMNAME_MAXLEN - 1);
bfa_fcs_lport_get_attr(port, lport_attr);
port_attr-port_type = cpu_to_be32(lport_attr.port_type);
port_attr-scos = pport_attr.cos_supported;
@@ -3217,7 +3219,7 @@ bfa_fcs_lport_ms_gmal_response(void *fcsarg, struct 
bfa_fcxp_s *fcxp,
rsp_str[gmal_entry-len-1] = 0;
 
/* copy IP Address to fabric */
-   strncpy(bfa_fcs_lport_get_fabric_ipaddr(port),
+   strlcpy(bfa_fcs_lport_get_fabric_ipaddr(port),
gmal_entry-ip_addr,
BFA_FCS_FABRIC_IPADDR_SZ);
break;
@@ -4655,21 +4657,15 @@ bfa_fcs_lport_ns_send_rspn_id(void *ns_cbarg, struct 
bfa_fcxp_s *fcxp_alloced)
 * to that of the base port.
 */
 
-   strncpy((char *)psymbl,
+   strlcpy((char *)psymbl,
(char *) 
(bfa_fcs_lport_get_psym_name
 (bfa_fcs_get_base_port(port-fcs))),
-   strlen((char *) 
-  bfa_fcs_lport_get_psym_name(bfa_fcs_get_base_port
- (port-fcs;
-
-   /* Ensure we have a null terminating string. */
-   ((char *)psymbl)[strlen((char *) 
-   bfa_fcs_lport_get_psym_name(bfa_fcs_get_base_port
-   (port-fcs)))] = 0;
-   strncat((char *)psymbl,
+   sizeof(symbl));
+
+   strlcat((char *)psymbl,
(char *) (bfa_fcs_lport_get_psym_name(port)),
-   strlen((char *) bfa_fcs_lport_get_psym_name(port)));
+   sizeof(symbl));
} else {
psymbl = (u8 *) (bfa_fcs_lport_get_psym_name(port));
}
@@ -5191,18 +5187,13 @@ bfa_fcs_lport_ns_util_send_rspn_id(void *cbarg, struct 
bfa_fcxp_s *fcxp_alloced)
 * For Vports, we append the vport's port symbolic name
 * to that of the base port.

[PATCH] scsi: bfa: bfa_fcs.c: Cleaning up missing null-terminate in conjunction with strncpy strncat

[Rickard Strandqvist]

Replacing strncp with strlcpy to avoid strings that lacks null terminate.
And strncat with strlcat because of incorrect use,
removed same the duplicated code.

Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
---
 drivers/scsi/bfa/bfa_fcs.c |   80 ++--
 1 file changed, 33 insertions(+), 47 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_fcs.c b/drivers/scsi/bfa/bfa_fcs.c
index a3ab5cc..48112ee 100644
--- a/drivers/scsi/bfa/bfa_fcs.c
+++ b/drivers/scsi/bfa/bfa_fcs.c
@@ -831,52 +831,41 @@ bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric)
bfa_ioc_get_adapter_model(fabric-fcs-bfa-ioc, model);
 
/* Model name/number */
-   strncpy((char *)port_cfg-sym_name, model,
-   BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
-   strncat((char *)port_cfg-sym_name, BFA_FCS_PORT_SYMBNAME_SEPARATOR,
-   sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
+   strlcpy((char *)port_cfg-sym_name, model,
+   BFA_SYMNAME_MAXLEN);
+   strlcat((char *)port_cfg-sym_name, BFA_FCS_PORT_SYMBNAME_SEPARATOR,
+   BFA_SYMNAME_MAXLEN);
 
/* Driver Version */
-   strncat((char *)port_cfg-sym_name, (char *)driver_info-version,
-   BFA_FCS_PORT_SYMBNAME_VERSION_SZ);
-   strncat((char *)port_cfg-sym_name, BFA_FCS_PORT_SYMBNAME_SEPARATOR,
-   sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
+   strlcat((char *)port_cfg-sym_name, (char *)driver_info-version,
+   BFA_SYMNAME_MAXLEN);
+   strlcat((char *)port_cfg-sym_name, BFA_FCS_PORT_SYMBNAME_SEPARATOR,
+   BFA_SYMNAME_MAXLEN);
 
/* Host machine name */
-   strncat((char *)port_cfg-sym_name,
+   strlcat((char *)port_cfg-sym_name,
(char *)driver_info-host_machine_name,
-   BFA_FCS_PORT_SYMBNAME_MACHINENAME_SZ);
-   strncat((char *)port_cfg-sym_name, BFA_FCS_PORT_SYMBNAME_SEPARATOR,
-   sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
+   BFA_SYMNAME_MAXLEN);
+   strlcat((char *)port_cfg-sym_name, BFA_FCS_PORT_SYMBNAME_SEPARATOR,
+   BFA_SYMNAME_MAXLEN);
 
/*
 * Host OS Info :
 * If OS Patch Info is not there, do not truncate any bytes from the
 * OS name string and instead copy the entire OS info string (64 bytes).
 */
+   strlcat((char *)port_cfg-sym_name,
+   (char *)driver_info-host_os_name,
+   BFA_SYMNAME_MAXLEN);
+   strlcat((char *)port_cfg-sym_name,
+   BFA_FCS_PORT_SYMBNAME_SEPARATOR,
+   BFA_SYMNAME_MAXLEN);
if (driver_info-host_os_patch[0] == '\0') {
-   strncat((char *)port_cfg-sym_name,
-   (char *)driver_info-host_os_name,
-   BFA_FCS_OS_STR_LEN);
-   strncat((char *)port_cfg-sym_name,
-   BFA_FCS_PORT_SYMBNAME_SEPARATOR,
-   sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
-   } else {
-   strncat((char *)port_cfg-sym_name,
-   (char *)driver_info-host_os_name,
-   BFA_FCS_PORT_SYMBNAME_OSINFO_SZ);
-   strncat((char *)port_cfg-sym_name,
-   BFA_FCS_PORT_SYMBNAME_SEPARATOR,
-   sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
-
/* Append host OS Patch Info */
-   strncat((char *)port_cfg-sym_name,
+   strlcat((char *)port_cfg-sym_name,
(char *)driver_info-host_os_patch,
-   BFA_FCS_PORT_SYMBNAME_OSPATCH_SZ);
+   BFA_SYMNAME_MAXLEN);
}
-
-   /* null terminate */
-   port_cfg-sym_name.symname[BFA_SYMNAME_MAXLEN - 1] = 0;
 }
 
 /*
@@ -892,29 +881,26 @@ bfa_fcs_fabric_nsymb_init(struct bfa_fcs_fabric_s *fabric)
bfa_ioc_get_adapter_model(fabric-fcs-bfa-ioc, model);
 
/* Model name/number */
-   strncpy((char *)port_cfg-node_sym_name, model,
-   BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
-   strncat((char *)port_cfg-node_sym_name,
+   strlcpy((char *)port_cfg-node_sym_name, model,
+   BFA_SYMNAME_MAXLEN);
+   strlcat((char *)port_cfg-node_sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
-   sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
+   BFA_SYMNAME_MAXLEN);
 
/* Driver Version */
-   strncat((char *)port_cfg-node_sym_name, (char *)driver_info-version,
-   BFA_FCS_PORT_SYMBNAME_VERSION_SZ);
-   strncat((char *)port_cfg-node_sym_name,
+   strlcat((char *)port_cfg-node_sym_name, (char *)driver_info-version,
+   BFA_SYMNAME_MAXLEN);
+   strlcat((char *)port_cfg-node_sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
-   sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
+   BFA_SYMNAME_MAXLEN);
 

Re: [PATCH] usb-storage/SCSI: Add broken_fua blacklist flag

[Laszlo T.]

2014-07-27 3:50 GMT+02:00 Alan Stern st...@rowland.harvard.edu:
 On Sat, 26 Jul 2014, Laszlo T. wrote:

 Hello,

 First of all I've just subscribed the linux-scsi thread. I hope you
 get my email the right way.

 I have also problems with Jmicron JMS567 mobile rack.

 I tried on different kernels:
 3.15.5
 3.16.rc6
 the 'Invalid field in cdb' errors disappeared but still not ok.

 Since your device uses the uas driver, you might get more help on the
 linux-usb mailing list.  (You don't have to subscribe to the list in
 order to post to it.)

 Alan Stern


Thank you, I will email them.

In the meantime, is there any alternate solution to use this mobile rack?

I tried to backlist the uas driver on 3.16.rc6 but in that case the
/dev/sd* file wasn't created.
and also tried to disable fua (libata fua=0) on 3.14 kernel, but I got
the 'Invalid field in cdb' errors.

Br,
Laszlo
--
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: [Xen-devel] [PATCH V2 3/4] Introduce XEN scsiback module

[Jürgen Groß]

On 07/26/2014 05:23 PM, Christoph Hellwig wrote:

Just a quick glance:

  a) this should move to drivers/target with the other target code


I don't mind. I just followed the example of drivers/vhost/vhost.c and
thought the similar xen module should be located under drivers/xen.


  b) you're still having your own CDB emulation in there, the target code
 should be taking care of all that.


Okay, I'll remove the CDB emulation.


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