Re: Reported regressions for 4.7 as of Sunday, 2016-06-19

2016-06-23 Thread Eric W. Biederman 
Linus Torvalds  writes:

> On Thu, Jun 23, 2016 at 9:13 AM, Quinn Tran  wrote:
>>
>>
>> QT: setting up the interrupt vector does not mean the interrupt starts 
>> firing immediately.
>
> Actually, it very much can mean that. If the interrupt can possibly be
> shared, there is a very real possibility of it fiding immediately.
>
> Now, with MSI(-X) I guess that isn't a worry, so I suspect your patch
> that handles just the legacy INTx case anyway is sufficient, but in
> general I would like people to always act as if interrupts can happen
> immediately after request_irq().
>
> We have had *tons* of situations where the firmware left a device
> active, for example. Or where some random interrupt controller ended
> up having stale interrupts pending, even.
>
> So in general, it's just good practice to say "spurious interrupts can
> and do happen" - the shared irq case is the most obvious case, but
> there have been other sources of unexpected spurious interrupts
> firing.

One case that occassionally bytes even for MSI-X is the case of kexec on
panic where the hardware was not shut down before the kernel starts, and
the start of the kernel masks the irq.  Then when the driver initializes
and calls request_irq it is possible for an irq to be pending as soon as
the MSI-X irq is actually enabled to the hardware.

And there is always CONFIG_IRQ_DEBUG which always acts like an interrupt
happens right when after request_irq finishes.

Eric
--
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 v7] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-06-23 Thread Michael Cyr 

Responding to those issues not already addressed by Bryant.

On 6/23/16 9:22 AM, Christoph Hellwig wrote:



+   vscsi->flags &= (~PROCESSING_MAD);

No need for the braces here. (ditto for many other places later on)

Fixed.



+static struct ibmvscsis_tport *ibmvscsis_lookup_port(const char *name)
+{
+   struct ibmvscsis_tport *tport = NULL;
+   struct vio_dev *vdev;
+   struct scsi_info *vscsi;
+
+   spin_lock_bh(_dev_lock);
+   list_for_each_entry(vscsi, _dev_list, list) {
+   vdev = vscsi->dma_dev;
+   if (!strcmp(dev_name(>dev), name)) {
+   tport = >tport;
+   break;
+   }
+   }
+   spin_unlock_bh(_dev_lock);

Without grabbing a reference this looks inherently unsafe.

I don't understand your concern.  Could you please provide more detail?

+static void ibmvscsis_scheduler(struct work_struct *work)

Odd name for a work item.

Not sure why it seems odd.  It schedules work to TCM.

+{
+   struct ibmvscsis_cmd *cmd = container_of(work, struct ibmvscsis_cmd,
+work);
+   struct scsi_info *vscsi = cmd->adapter;
+
+   spin_lock_bh(>intr_lock);
+
+   /* Remove from schedule_q */
+   list_del(>list);

What do you need the schedule_q for as the workqueue already tracks
the commands?
There are a number of places where we need to see if there is anything 
on the work queue, and I am not aware of an existing function to do 
this, so we keep our own list of commands which have been queued.




+static void ibmvscsis_alloc_common_locks(struct scsi_info *vscsi)
+{
+   spin_lock_init(>intr_lock);
+}
+
+static void ibmvscsis_free_common_locks(struct scsi_info *vscsi)
+{
+   /* Nothing to do here */
+}

No need for these wrapers.

Removed.

+static irqreturn_t ibmvscsis_interrupt(int dummy, void *data)
+{
+   struct scsi_info *vscsi = data;
+
+   vio_disable_interrupts(vscsi->dma_dev);
+   tasklet_schedule(>work_task);
+
+   return IRQ_HANDLED;
+}

Can you explain the need for the tasklet?  There shouldn't be a whole
lot of working before passing the command off to the workqueue anyway.
There are some requests, like SRP Login and the various MADs, which can 
be handled at the interrupt level, and so we do so.


--
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: fix new bug in scsi_dev_info_list string matching

2016-06-23 Thread Alan Stern 
Commit b704f70ce200 ("SCSI: fix bug in scsi_dev_info_list matching")
changed the way vendor- and model-string matching was carried out in
the routine that looks up entries in a SCSI devinfo list.  The new
matching code failed to take into account the case of a maximum-length
string; in such cases it could end up testing for a terminating '\0'
byte beyond the end of the memory allocated to the string.  This
out-of-bounds bug was detected by UBSAN.

I don't know if anybody has actually encountered this bug.  The
symptom would be that a device entry in the blacklist might not be
matched properly if it contained an 8-character vendor name or a
16-character model name.  Such entries certainly exist in
scsi_static_device_list.

This patch fixes the problem by adding a check for a maximum-length
string before the '\0' test.

Signed-off-by: Alan Stern 
Fixes: b704f70ce200 ("SCSI: fix bug in scsi_dev_info_list matching")
Tested-by: Wilfried Klaebe 
CC: 

---


[as1804]


 drivers/scsi/scsi_devinfo.c |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

Index: usb-4.x/drivers/scsi/scsi_devinfo.c
===
--- usb-4.x.orig/drivers/scsi/scsi_devinfo.c
+++ usb-4.x/drivers/scsi/scsi_devinfo.c
@@ -429,7 +429,7 @@ static struct scsi_dev_info_list *scsi_d
 * here, and we don't know what device it is
 * trying to work with, leave it as-is.
 */
-   vmax = 8;   /* max length of vendor */
+   vmax = sizeof(devinfo->vendor);
vskip = vendor;
while (vmax > 0 && *vskip == ' ') {
vmax--;
@@ -439,7 +439,7 @@ static struct scsi_dev_info_list *scsi_d
while (vmax > 0 && vskip[vmax - 1] == ' ')
--vmax;
 
-   mmax = 16;  /* max length of model */
+   mmax = sizeof(devinfo->model);
mskip = model;
while (mmax > 0 && *mskip == ' ') {
mmax--;
@@ -455,10 +455,12 @@ static struct scsi_dev_info_list *scsi_d
 * Behave like the older version of get_device_flags.
 */
if (memcmp(devinfo->vendor, vskip, vmax) ||
-   devinfo->vendor[vmax])
+   (vmax < sizeof(devinfo->vendor) &&
+   devinfo->vendor[vmax]))
continue;
if (memcmp(devinfo->model, mskip, mmax) ||
-   devinfo->model[mmax])
+   (mmax < sizeof(devinfo->model) &&
+   devinfo->model[mmax]))
continue;
return devinfo;
} 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: Reported regressions for 4.7 as of Sunday, 2016-06-19

2016-06-23 Thread Quinn Tran 

-Original Message-
From: Johannes Thumshirn 
Date: Thursday, June 23, 2016 at 12:22 AM
To: Quinn Tran 
Cc: "Martin K. Petersen" , Linus Torvalds 
, Josh Boyer , 
Thorsten Leemhuis , linux-kernel 
, linux-scsi 
Subject: Re: Reported regressions for 4.7 as of Sunday, 2016-06-19

>[+ Cc linux-scsi@vger.kernel.org ]
>
>On Wed, Jun 22, 2016 at 03:57:35PM +, Quinn Tran wrote:
>> Johannes,  Martin,
>> 
>> Based on the screen shot/call trace,  it looks like this adapter is not 
>> using MSIX.  It defaulted back to MSI or INTx interrupt.  The code made an 
>> assumption  of MSIX is available.  There is no point in go through that code 
>> segment.
>> 
>> Can you try this work around?  It’s untested.  Thanks.
>> 
>> 
>> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
>> index 5649c20..e033ecb 100644
>> --- a/drivers/scsi/qla2xxx/qla_isr.c
>> +++ b/drivers/scsi/qla2xxx/qla_isr.c
>> @@ -2548,7 +2548,7 @@ void qla24xx_process_response_queue(struct 
>> scsi_qla_host *vha,
>> if (!vha->flags.online)
>> return;
>>  
>> -   if (rsp->msix->cpuid != smp_processor_id()) {
>> +   if (rsp->msix && (rsp->msix->cpuid != smp_processor_id())) {
>> /* if kernel does not notify qla of IRQ's CPU change,
>>  * then set it here.
>>  */
>> 
>
>But this still does not fix the race which would be possible if the HBA is
>using MSI-X but triggering IRQs early enough.
>
>Have a look at this (I admit theoretical) path:
>qla24xx_enable_msix(struct qla_hw_data *ha, struct rsp_que *rsp)
>{
>   [...]
>   /* Enable MSI-X vectors for the base queue */
>   for (i = 0; i < 2; i++) {
>   qentry = >msix_entries[i];
>   if (IS_P3P_TYPE(ha))
>   ret = request_irq(qentry->vector,
>   qla82xx_msix_entries[i].handler,
>   0, qla82xx_msix_entries[i].name, rsp);
>   else
>   ret = request_irq(qentry->vector,
>   msix_entries[i].handler,
>   0, msix_entries[i].name, rsp);
>   if (ret)
>   goto msix_register_fail;
>   <--- IRQ arrives here

QT: setting up the interrupt vector does not mean the interrupt starts firing 
immediately.  Interrupt starting firing when the driver is ready to accept the 
interrupt by enabling the interrupt (ha->isp_ops->enable_intrs(ha)) later on in 
time.  In addition, that particular code path/qla24xx_process_response_queue  
is not executed until driver feeds commands to the hardware work queue.  

IF there is a left over interrupt that happens to trigger the call immediately, 
there is another check that prevent the code from getting to the point of the 
“theoretical" race.


>   qentry->have_irq = 1;
>   qentry->rsp = rsp;
>   rsp->msix = qentry;
>
>   [...]
>
>
>void qla24xx_process_response_queue(struct scsi_qla_host *vha,
>struct rsp_que *rsp)
>{
--->8--
if (!vha->flags.online)
return;

---8<--
>   if (rsp->msix->cpuid != smp_processor_id()) {
>  ^
>  \--- rsp->msix == NULL
>
>   /* if kernel does not notify qla of IRQ's CPU change,
>* then set it here.
>*/
>   rsp->msix->cpuid = smp_processor_id();
>   ha->tgt.rspq_vector_cpuid = rsp->msix->cpuid;
>
>-- 
>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

Re: [PATCH] tcm_qla2xxx: fix spelling mistake: "seperator" -> "separator"

2016-06-23 Thread Laurence Oberman 


- Original Message -
> From: "Colin King" 
> To: "James E . J . Bottomley" , "Martin K . 
> Petersen" ,
> linux-scsi@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Sent: Thursday, June 23, 2016 1:12:25 PM
> Subject: [PATCH] tcm_qla2xxx: fix spelling mistake: "seperator" -> "separator"
> 
> From: Colin Ian King 
> 
> trivial fix to spelling mistake in pr_err message
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/scsi/qla2xxx/tcm_qla2xxx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> index 6643f6f..46fe6f4 100644
> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> @@ -1738,7 +1738,7 @@ static struct se_wwn *tcm_qla2xxx_npiv_make_lport(
>  
>   p = strchr(tmp, '@');
>   if (!p) {
> - pr_err("Unable to locate NPIV '@' seperator\n");
> + pr_err("Unable to locate NPIV '@' separator\n");
>   return ERR_PTR(-EINVAL);
>   }
>   *p++ = '\0';
> --
> 2.8.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Simple change, and its fine
Reviewed-by Laurence Oberman 
--
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] tcm_qla2xxx: fix spelling mistake: "seperator" -> "separator"

2016-06-23 Thread Colin King 
From: Colin Ian King 

trivial fix to spelling mistake in pr_err message

Signed-off-by: Colin Ian King 
---
 drivers/scsi/qla2xxx/tcm_qla2xxx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c 
b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 6643f6f..46fe6f4 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -1738,7 +1738,7 @@ static struct se_wwn *tcm_qla2xxx_npiv_make_lport(
 
p = strchr(tmp, '@');
if (!p) {
-   pr_err("Unable to locate NPIV '@' seperator\n");
+   pr_err("Unable to locate NPIV '@' separator\n");
return ERR_PTR(-EINVAL);
}
*p++ = '\0';
-- 
2.8.1

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

Re: Reported regressions for 4.7 as of Sunday, 2016-06-19

2016-06-23 Thread Linus Torvalds 
On Thu, Jun 23, 2016 at 9:13 AM, Quinn Tran  wrote:
>
>
> QT: setting up the interrupt vector does not mean the interrupt starts firing 
> immediately.

Actually, it very much can mean that. If the interrupt can possibly be
shared, there is a very real possibility of it fiding immediately.

Now, with MSI(-X) I guess that isn't a worry, so I suspect your patch
that handles just the legacy INTx case anyway is sufficient, but in
general I would like people to always act as if interrupts can happen
immediately after request_irq().

We have had *tons* of situations where the firmware left a device
active, for example. Or where some random interrupt controller ended
up having stale interrupts pending, even.

So in general, it's just good practice to say "spurious interrupts can
and do happen" - the shared irq case is the most obvious case, but
there have been other sources of unexpected spurious interrupts
firing.

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

Re: [PATCH v7] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-06-23 Thread Bryant G. Ly 


On 6/23/16, 9:22 AM, "Christoph Hellwig"  wrote:

>> -config SCSI_SRP
>> -tristate "SCSI RDMA Protocol helper library"
>> -depends on SCSI && PCI
>> -select SCSI_TGT
>> -help
>> -  If you wish to use SRP target drivers, say Y.
>> -
>> -  To compile this driver as a module, choose M here: the
>> -  module will be called libsrp.
>> -
>
>Please split that removal of libsrp into a separate patch before
>adding the new driver.

Why do you suggest splitting the path up for the libsrp stuff? The current 
upstream
does not contain libsrp. The only reason the patch shows that there is a 
removal of
libsrp is that James Bottomley suggested doing a revert of: 
libsrp: removal - commit f6667938cfceefd8afe6355ceb6497dce4883ae9
to make it clear that I’m bringing back what had existed a year ago within the 
upstream.

>
>> new file mode 100644
>> index 000..887574d
>> --- /dev/null
>> +++ b/drivers/scsi/ibmvscsi_tgt/Makefile
>> @@ -0,0 +1,4 @@
>> +obj-$(CONFIG_SCSI_IBMVSCSIS)+= ibmvscsis.o
>> +
>> +ibmvscsis-objs := libsrp.o ibmvscsi_tgt.o
>
>please use module-y for adding objects.  Also what's the reason
>for splitting these files?
>

I will change to module-y. The reason is we are possibly going to write
another target fabric in the future with the IBMVFC driver so just incase
we want to leave it separated for now. 

>> +/***
>> + * IBM Virtual SCSI Target Driver
>> + * Copyright (C) 2003-2005 Dave Boutcher (boutc...@us.ibm.com) IBM Corp.
>> + * Santiago Leon (san...@us.ibm.com) IBM Corp.
>> + * Linda Xie (l...@us.ibm.com) IBM Corp.
>> + *
>> + * Copyright (C) 2005-2011 FUJITA Tomonori 
>> + * Copyright (C) 2010 Nicholas A. Bellinger 
>> + * Copyright (C) 2016 Bryant G. Ly  IBM Corp.
>> + *
>> + * Authors: Bryant G. Ly 
>> + * Authors: Michael Cyr 
>
>What's the reational for the copyright vs Authors lines?

No reason, ill remove the copyright. 

>
>> +#include "ibmvscsi_tgt.h"
>> +
>> +#ifndef H_GET_PARTNER_INFO
>> +#define H_GET_PARTNER_INFO  0x0008LL
>> +#endif
>
>Should this be in a header with the other hcalls?

Yes, Moved.

>
>> +static const char ibmvscsis_driver_name[] = "ibmvscsis";
>
>I think you can just assign the name directly in the driver ops
>structure.

Fixed.

>
>> +static const char ibmvscsis_workq_name[] = "ibmvscsis";
>
>This one seems unused.

Removed

>
>> +vscsi->flags &= (~PROCESSING_MAD);
>
>No need for the braces here. (ditto for many other places later on)
>
>> +static long ibmvscsis_parse_command(struct scsi_info *vscsi,
>> +struct viosrp_crq *crq);
>
>Can you avoid forward declarations where easily possible, and if not
>keep them all at the beginning of the file?

Will do.



>> +static int ibmvscsis_alloc_cmds(struct scsi_info *vscsi, int num)
>> +{
>> +struct ibmvscsis_cmd *cmd;
>> +int i;
>> +
>> +INIT_LIST_HEAD(>free_cmd);
>> +vscsi->cmd_pool = kcalloc(num, sizeof(struct ibmvscsis_cmd),
>> +  GFP_KERNEL);
>> +if (!vscsi->cmd_pool)
>> +return -ENOMEM;
>> +
>> +for (i = 0, cmd = (struct ibmvscsis_cmd *)vscsi->cmd_pool; i < num;
>> + i++, cmd++) {
>> +cmd->adapter = vscsi;
>> +INIT_WORK(>work, ibmvscsis_scheduler);
>> +list_add_tail(>list, >free_cmd);
>> +}
>> +
>> +return 0;
>> +}
>
>Why can't you use the existing infrastructure for cmd pools in the
>target core?
>

I wasn’t aware there was existing infrastructure.



>> +rc = srp_transfer_data(cmd, _iu(iue)->srp.cmd, ibmvscsis_rdma, 1,
>> +   1);
>> +if (rc) {
>> +pr_err("srp_transfer_data failed: %d\n", rc);
>> +sd = se_cmd->sense_buffer;
>> +se_cmd->scsi_sense_length = 18;
>> +memset(se_cmd->sense_buffer, 0, se_cmd->scsi_sense_length);
>> +/* Current error */
>> +sd[0] = 0x70;
>> +/* sense key = Medium Error */
>> +sd[2] = 3;
>> +/* additional length (length - 8) */
>> +sd[7] = 10;
>> +/* asc/ascq 0x801 = Logical Unit Communication time-out */
>> +sd[12] = 8;
>> +sd[13] = 1;
>
>The Fabrics driver shouldn't generate it's own sense codes.  This
>would for example break for a lun using descriptor format sense data.

Changed to:

if (rc) {
pr_err("srp_transfer_data failed: %d\n", rc);
sd = se_cmd->sense_buffer;
se_cmd->scsi_sense_length = 18;
memset(se_cmd->sense_buffer, 0, se_cmd->scsi_sense_length);
/* Fixed/Current Error = 0
 * Sense Key

Re: [PATCH v2 1/3] block: provide helpers for reading block count

2016-06-23 Thread Sagi Grimberg 

Looks good, for the series:

Reviewed-by: Sagi Grimbeg 
--
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] snic: Fix use-after-free in case of a dma mapping error

2016-06-23 Thread Laurence Oberman 


- Original Message -
> From: "Johannes Thumshirn" 
> To: "Martin K . Petersen" , "James Bottomley" 
> 
> Cc: "Linux SCSI Mailinglist" , "Linux Kernel 
> Mailinglist" ,
> "Narsimhulu Musini" , "Sesidhar Baddela" 
> , "Johannes Thumshirn"
> 
> Sent: Thursday, June 23, 2016 8:37:20 AM
> Subject: [PATCH] snic: Fix use-after-free in case of a dma mapping error
> 
> If there is a dma mapping error snic kfree()s buf right before printing it.
> Change the order to not accidently trip on memory that's not owned by us
> anymore.
> 
> Signed-off-by: Johannes Thumshirn 
> ---
>  drivers/scsi/snic/snic_disc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/snic/snic_disc.c b/drivers/scsi/snic/snic_disc.c
> index b0fefd6..b106596 100644
> --- a/drivers/scsi/snic/snic_disc.c
> +++ b/drivers/scsi/snic/snic_disc.c
> @@ -113,11 +113,11 @@ snic_queue_report_tgt_req(struct snic *snic)
>  
>   pa = pci_map_single(snic->pdev, buf, buf_len, PCI_DMA_FROMDEVICE);
>   if (pci_dma_mapping_error(snic->pdev, pa)) {
> - kfree(buf);
> - snic_req_free(snic, rqi);
>   SNIC_HOST_ERR(snic->shost,
> "Rpt-tgt rspbuf %p: PCI DMA Mapping Failed\n",
> buf);
> + kfree(buf);
> + snic_req_free(snic, rqi);
>   ret = -EINVAL;
>  
>   goto error;
> --
> 2.8.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
> 

Looks fine to me
Reviewed-by Laurence Oberman 
--
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] block: provide helpers for reading block count

2016-06-23 Thread Christoph Hellwig 
Thanks Arnd,

the series looks fine to me:

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 v7] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-06-23 Thread Christoph Hellwig 
> -config SCSI_SRP
> - tristate "SCSI RDMA Protocol helper library"
> - depends on SCSI && PCI
> - select SCSI_TGT
> - help
> -   If you wish to use SRP target drivers, say Y.
> -
> -   To compile this driver as a module, choose M here: the
> -   module will be called libsrp.
> -

Please split that removal of libsrp into a separate patch before
adding the new driver.

> new file mode 100644
> index 000..887574d
> --- /dev/null
> +++ b/drivers/scsi/ibmvscsi_tgt/Makefile
> @@ -0,0 +1,4 @@
> +obj-$(CONFIG_SCSI_IBMVSCSIS) += ibmvscsis.o
> +
> +ibmvscsis-objs := libsrp.o ibmvscsi_tgt.o

please use module-y for adding objects.  Also what's the reason
for splitting these files?

> +/***
> + * IBM Virtual SCSI Target Driver
> + * Copyright (C) 2003-2005 Dave Boutcher (boutc...@us.ibm.com) IBM Corp.
> + *  Santiago Leon (san...@us.ibm.com) IBM Corp.
> + *  Linda Xie (l...@us.ibm.com) IBM Corp.
> + *
> + * Copyright (C) 2005-2011 FUJITA Tomonori 
> + * Copyright (C) 2010 Nicholas A. Bellinger 
> + * Copyright (C) 2016 Bryant G. Ly  IBM Corp.
> + *
> + * Authors: Bryant G. Ly 
> + * Authors: Michael Cyr 

What's the reational for the copyright vs Authors lines?

> +#include "ibmvscsi_tgt.h"
> +
> +#ifndef H_GET_PARTNER_INFO
> +#define H_GET_PARTNER_INFO  0x0008LL
> +#endif

Should this be in a header with the other hcalls?

> +static const char ibmvscsis_driver_name[] = "ibmvscsis";

I think you can just assign the name directly in the driver ops
structure.

> +static const char ibmvscsis_workq_name[] = "ibmvscsis";

This one seems unused.

> + vscsi->flags &= (~PROCESSING_MAD);

No need for the braces here. (ditto for many other places later on)

> +static long ibmvscsis_parse_command(struct scsi_info *vscsi,
> + struct viosrp_crq *crq);

Can you avoid forward declarations where easily possible, and if not
keep them all at the beginning of the file?

> +static struct ibmvscsis_tport *ibmvscsis_lookup_port(const char *name)
> +{
> + struct ibmvscsis_tport *tport = NULL;
> + struct vio_dev *vdev;
> + struct scsi_info *vscsi;
> +
> + spin_lock_bh(_dev_lock);
> + list_for_each_entry(vscsi, _dev_list, list) {
> + vdev = vscsi->dma_dev;
> + if (!strcmp(dev_name(>dev), name)) {
> + tport = >tport;
> + break;
> + }
> + }
> + spin_unlock_bh(_dev_lock);

Without grabbing a reference this looks inherently unsafe.

> +static void ibmvscsis_scheduler(struct work_struct *work)

Odd name for a work item.

> +{
> + struct ibmvscsis_cmd *cmd = container_of(work, struct ibmvscsis_cmd,
> +  work);
> + struct scsi_info *vscsi = cmd->adapter;
> +
> + spin_lock_bh(>intr_lock);
> +
> + /* Remove from schedule_q */
> + list_del(>list);

What do you need the schedule_q for as the workqueue already tracks
the commands?

> +static int ibmvscsis_alloc_cmds(struct scsi_info *vscsi, int num)
> +{
> + struct ibmvscsis_cmd *cmd;
> + int i;
> +
> + INIT_LIST_HEAD(>free_cmd);
> + vscsi->cmd_pool = kcalloc(num, sizeof(struct ibmvscsis_cmd),
> +   GFP_KERNEL);
> + if (!vscsi->cmd_pool)
> + return -ENOMEM;
> +
> + for (i = 0, cmd = (struct ibmvscsis_cmd *)vscsi->cmd_pool; i < num;
> +  i++, cmd++) {
> + cmd->adapter = vscsi;
> + INIT_WORK(>work, ibmvscsis_scheduler);
> + list_add_tail(>list, >free_cmd);
> + }
> +
> + return 0;
> +}

Why can't you use the existing infrastructure for cmd pools in the
target core?

> +static void ibmvscsis_alloc_common_locks(struct scsi_info *vscsi)
> +{
> + spin_lock_init(>intr_lock);
> +}
> +
> +static void ibmvscsis_free_common_locks(struct scsi_info *vscsi)
> +{
> + /* Nothing to do here */
> +}

No need for these wrapers.

> +static irqreturn_t ibmvscsis_interrupt(int dummy, void *data)
> +{
> + struct scsi_info *vscsi = data;
> +
> + vio_disable_interrupts(vscsi->dma_dev);
> + tasklet_schedule(>work_task);
> +
> + return IRQ_HANDLED;
> +}

Can you explain the need for the tasklet?  There shouldn't be a whole
lot of working before passing the command off to the workqueue anyway.

> + rc = srp_transfer_data(cmd, _iu(iue)->srp.cmd, ibmvscsis_rdma, 1,
> +1);
> + if (rc) {
> + pr_err("srp_transfer_data failed: %d\n", rc);
> + sd = se_cmd->sense_buffer;
> + se_cmd->scsi_sense_length = 18;
> + memset(se_cmd->sense_buffer, 0, se_cmd->scsi_sense_length);
> + /* Current error */
> + sd[0] = 0x70;

[PATCH v2 2/3] partition/efi: use bdev_logical_block_count()

2016-06-23 Thread Arnd Bergmann 
Enabling CONFIG_UBSAN_SANITIZE_ALL on ARM caused a link error:

last_lba.part.0':
:(.text+0xc3440): undefined reference to `ilog2_NaN'
:(.text+0xc3538): undefined reference to `__aeabi_uldivmod'
:(.text+0xc38e8): undefined reference to `__aeabi_uldivmod'

This is caused by gcc not behaving in the expected ways with 
__builtin_constant_p(),
but it also points to somewhat inefficient code based on a 64-bit
division.

I have introduced a better bdev_logical_block_count() now, so we
can use that here.

Signed-off-by: Arnd Bergmann 
---
 block/partitions/efi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index bcd86e5cd546..7b1a62073d34 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -149,8 +149,8 @@ static u64 last_lba(struct block_device *bdev)
 {
if (!bdev || !bdev->bd_inode)
return 0;
-   return div_u64(bdev->bd_inode->i_size,
-  bdev_logical_block_size(bdev)) - 1ULL;
+
+   return bdev_logical_block_count(bdev) - 1;
 }
 
 static inline int pmbr_part_valid(gpt_mbr_record *part)
-- 
2.9.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

[PATCH v2 3/3] target/iblock: use bdev_logical_block_count()

2016-06-23 Thread Arnd Bergmann 
Enabling CONFIG_UBSAN_SANITIZE_ALL on ARM caused a link error:

drivers/target/built-in.o: In function 
`iblock_emulate_read_cap_with_block_size.constprop.1':
target_core_iblock.c:(.text+0xc2774): undefined reference to `ilog2_NaN'
target_core_iblock.c:(.text+0xc27f8): undefined reference to `__aeabi_uldivmod'
target_core_iblock.c:(.text+0xc299c): undefined reference to `__aeabi_uldivmod'

This is caused by gcc not behaving in the expected ways with 
__builtin_constant_p(),
but it also points to somewhat inefficient code based on an expensive 64-bit
division.

I have introduced a better bdev_logical_block_count() now, so we can use that 
here.

Signed-off-by: Arnd Bergmann 
---
 drivers/target/target_core_iblock.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/target/target_core_iblock.c 
b/drivers/target/target_core_iblock.c
index 22af12f8b8eb..2dc0129553e1 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -201,9 +201,8 @@ static unsigned long long 
iblock_emulate_read_cap_with_block_size(
struct block_device *bd,
struct request_queue *q)
 {
-   unsigned long long blocks_long = (div_u64(i_size_read(bd->bd_inode),
-   bdev_logical_block_size(bd)) - 1);
u32 block_size = bdev_logical_block_size(bd);
+   unsigned long long blocks_long = bdev_logical_block_count(bd) - 1;
 
if (block_size == dev->dev_attrib.block_size)
return blocks_long;
-- 
2.9.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

[PATCH v2 1/3] block: provide helpers for reading block count

2016-06-23 Thread Arnd Bergmann 
Several drivers use an expensive do_div() to compute the number
of logical or physical blocks in a blockdev, which can be done
more efficiently using a shift, since the blocksize is always
a power of two number.

Let's introduce bdev_logical_block_count() and bdev_physical_block_count()
helper functions mirroring the bdev_logical_block_size() and
bdev_physical_block_size() interfaces for the block size.

Signed-off-by: Arnd Bergmann 
Suggested-by: Christoph Hellwig 
---
 include/linux/blkdev.h | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 9d1e0a4650dc..ae8c408f6c22 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1226,6 +1226,13 @@ static inline unsigned short 
bdev_logical_block_size(struct block_device *bdev)
return queue_logical_block_size(bdev_get_queue(bdev));
 }
 
+static inline sector_t bdev_logical_block_count(struct block_device *bdev)
+{
+   unsigned int block_shift = ilog2(bdev_logical_block_size(bdev));
+
+   return bdev->bd_inode->i_size >> block_shift;
+}
+
 static inline unsigned int queue_physical_block_size(struct request_queue *q)
 {
return q->limits.physical_block_size;
@@ -1236,6 +1243,13 @@ static inline unsigned int 
bdev_physical_block_size(struct block_device *bdev)
return queue_physical_block_size(bdev_get_queue(bdev));
 }
 
+static inline sector_t bdev_physical_block_count(struct block_device *bdev)
+{
+   unsigned int block_shift = ilog2(bdev_physical_block_size(bdev));
+
+   return bdev->bd_inode->i_size >> block_shift;
+}
+
 static inline unsigned int queue_io_min(struct request_queue *q)
 {
return q->limits.io_min;
-- 
2.9.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

[PATCH] snic: Fix use-after-free in case of a dma mapping error

2016-06-23 Thread Johannes Thumshirn 
If there is a dma mapping error snic kfree()s buf right before printing it.
Change the order to not accidently trip on memory that's not owned by us
anymore.

Signed-off-by: Johannes Thumshirn 
---
 drivers/scsi/snic/snic_disc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/snic/snic_disc.c b/drivers/scsi/snic/snic_disc.c
index b0fefd6..b106596 100644
--- a/drivers/scsi/snic/snic_disc.c
+++ b/drivers/scsi/snic/snic_disc.c
@@ -113,11 +113,11 @@ snic_queue_report_tgt_req(struct snic *snic)
 
pa = pci_map_single(snic->pdev, buf, buf_len, PCI_DMA_FROMDEVICE);
if (pci_dma_mapping_error(snic->pdev, pa)) {
-   kfree(buf);
-   snic_req_free(snic, rqi);
SNIC_HOST_ERR(snic->shost,
  "Rpt-tgt rspbuf %p: PCI DMA Mapping Failed\n",
  buf);
+   kfree(buf);
+   snic_req_free(snic, rqi);
ret = -EINVAL;
 
goto error;
-- 
2.8.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

[Bug 120921] New: target: Unconfiguring ib_srpt triggers kernel crash

2016-06-23 Thread bugzilla-daemon 
https://bugzilla.kernel.org/show_bug.cgi?id=120921

Bug ID: 120921
   Summary: target: Unconfiguring ib_srpt triggers kernel crash
   Product: IO/Storage
   Version: 2.5
Kernel Version: v4.6.2
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: SCSI
  Assignee: linux-scsi@vger.kernel.org
  Reporter: bvanass...@acm.org
Regression: No

The following command triggered the kernel crash:

# rmdir /sys/kernel/config/target/srpt/*/*/acls/*
Segmentation fault

>From the console:

[  957.515524] general protection fault:  [#1] SMP 
[  957.515638] Modules linked in: target_core_file ib_srpt target_core_iblock
target_core_mod brd dm_service_time fuse dm_multipath scsi_dh_rdac scsi_dh_emc
scsi_dh_alua netconsole xt_CHECKSUM iptable_mangle ipt_MASQUERADE
nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4
nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT xt_tcpudp tun
ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter ip_tables
x_tables af_packet bridge stp llc iscsi_ibft iscsi_boot_sysfs ib_ipoib rdma_ucm
ib_ucm ib_uverbs msr ib_umad rdma_cm configfs ib_cm iw_cm mlx4_ib ib_sa ib_mad
ib_core ib_addr x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm
dm_mod irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel
tg3 ipmi_ssif ipmi_devintf iTCO_wdt aesni_intel iTCO_vendor_support aes_x86_64
lrw ptp sb_edac mei_me gf128mul dcdbas pps_core glue_helper ablk_helper
mlx4_core pcspkr mei edac_core libphy cryptd fjes lpc_ich ipmi_si mfd_core
8250_fintek ipmi_msghandler tpm_tis tpm wmi shpchp acpi_power_meter button
hid_generic usbhid mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect
sysimgblt fb_sys_fops ttm sr_mod drm cdrom ehci_pci ehci_hcd usbcore usb_common
sg [last unloaded: scsi_transport_srp]
[  957.519948] CPU: 3 PID: 22754 Comm: rmdir Not tainted 4.6.2-dbg+ #1
[  957.519989] Hardware name: Dell Inc. PowerEdge R430/03XKDV, BIOS 1.0.2
11/17/2014
[  957.520032] task: 88006c82b140 ti: 88033eb8c000 task.ti:
88033eb8c000
[  957.520073] RIP: 0010:[]  []
__list_del_entry+0x29/0xc0
[  957.520154] RSP: 0018:88033eb8fd88  EFLAGS: 00010a83
[  957.520194] RAX: 6b6b6b6b6b6b6b6b RBX: 880372948780 RCX:
dead0200
[  957.520234] RDX: 6b6b6b6b6b6b6b6b RSI: 88006c82b980 RDI:
880372948780
[  957.520276] RBP: 88033eb8fd88 R08: 88033eb8fc88 R09:

[  957.520316] R10:  R11:  R12:
880340dc8958
[  957.520540] R13: 0001 R14: 88046db301b8 R15:
880372948740
[  957.520582] FS:  7f4e4a1f7700() GS:88046f26()
knlGS:
[  957.520623] CS:  0010 DS:  ES:  CR0: 80050033
[  957.520662] CR2: 7fefadbca728 CR3: 0003752ee000 CR4:
001406e0
[  957.520701] Stack:
[  957.520739]  88033eb8fda0 812dbb7d 6b6b6b6b6b6b6b2b
88033eb8fe00
[  957.520924]  a061f044 88033eb8fdd0 880340dc8b28
880340dc8dd8
[  957.521106]  880372948780 880372948780 880340dc8958

[  957.521283] Call Trace:
[  957.521323]  [] list_del+0xd/0x30
[  957.521379]  []
core_tpg_del_initiator_node_acl+0x134/0x210 [target_core_mod]
[  957.521434]  [] target_fabric_nacl_base_release+0x20/0x30
[target_core_mod]
[  957.521484]  [] config_item_release+0x62/0xd0 [configfs]
[  957.521532]  [] config_item_put+0x1d/0x1f [configfs]
[  957.521870]  [] configfs_rmdir+0x1e7/0x300 [configfs]
[  957.521918]  [] vfs_rmdir+0x6c/0x110
[  957.521963]  [] do_rmdir+0x158/0x1c0
[  957.522006]  [] SyS_rmdir+0x11/0x20
[  957.522049]  [] entry_SYSCALL_64_fastpath+0x1c/0xac
[  957.522093] Code: 66 90 48 b9 00 01 00 00 00

Note: this crash does not occur with kernel v4.7-rc4.

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

Re: [PATCH v2] byteswap: try to avoid __builtin_constant_p gcc bug

2016-06-23 Thread Arnd Bergmann 
On Thursday, June 23, 2016 9:27:30 AM CEST Tomas Winkler wrote:
> On Wed, Jun 22, 2016 at 3:25 PM, Arnd Bergmann  wrote:
> > On Wednesday, June 22, 2016 2:44:21 PM CEST Tomas Winkler wrote:
> >> >
> >> > There are more than 20 files that have the statement: case cpu_to_...
> >> > Sparse complains about: case __builtin_bswap, not about 
> >> > __builtin_constant_p.
> >>
> >> There is even much more in the header files used in  initializers,
> >> which also require constants.  I wonder if __builtin_bswap produces
> >> constant expression correctly under gcc?
> >
> > In gcc-4.8 or later yes. gcc-4.6/4.7 on powerpc was a special case that we
> > have worked around now, as the 16-bit byteswap there was not a constant
> > expression, unlike the 32-bit and 64-bit ones.
> 
> Can you please give any references to that?

The patch description for the last change has a good explanation:

commit 8634de6d254462e6953b7dac772a1df4f44c8030
Author: Josh Poimboeuf 
Date:   Fri May 6 09:22:25 2016 -0500

compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16()

gcc support for __builtin_bswap16() was supposedly added for powerpc in
gcc 4.6, and was then later added for other architectures in gcc 4.8.

However, Stephen Rothwell reported that attempting to use it on powerpc
in gcc 4.6 fails with:

  lib/vsprintf.c:160:2: error: initializer element is not constant
  lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]')
  lib/vsprintf.c:160:2: error: initializer element is not constant
  lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]')
  ...

I'm not entirely sure what those errors mean, but I don't see them on
gcc 4.8.  So let's consider gcc 4.8 to be the official starting point
for __builtin_bswap16().

Arnd Bergmann adds:
 "I found the commit in gcc-4.8 that replaced the powerpc-specific
  implementation of __builtin_bswap16 with an architecture-independent
  one.  Apparently the powerpc version (gcc-4.6 and 4.7) just mapped to
  the lhbrx/sthbrx instructions, so it ended up not being a constant,
  though the intent of the patch was mainly to add support for the
  builtin to x86:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52624

  has the patch that went into gcc-4.8 and more information."

Fixes: 7322dd755e7d ("byteswap: try to avoid __builtin_constant_p gcc bug")
Reported-by: Stephen Rothwell 
Tested-by: Stephen Rothwell 
Acked-by: Arnd Bergmann 
Signed-off-by: Josh Poimboeuf 
Signed-off-by: Stephen Rothwell 
Signed-off-by: Linus Torvalds 

 include/linux/compiler-gcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


> Just to make sure we are on the same line , the  bottom line is that
> sparse has to be adjusted.

Yes, that sounds right. I don't know if sparse actually tracks the
value of the constant, or if it's good enough for sparse to know that
a constant input to __builtin_bswap results in a constant output.
If that is the case, we could just #define __builtin_bswap32(x) (x)
in the kernel headers when building with sparse, and have it work
for older sparse versions too.

Arnd
--
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] MAINTAINERS: Change FCoE maintainer

2016-06-23 Thread Johannes Thumshirn 
Vasu is going to resign from his maintainer role and I'll take over.

Signed-off-by: Johannes Thumshirn 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index e1b090f..70af8c0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4661,7 +4661,7 @@ S:Maintained
 F: drivers/staging/fbtft/
 
 FCOE SUBSYSTEM (libfc, libfcoe, fcoe)
-M: Vasu Dev 
+M: Johannes Thumshirn 
 L: fcoe-de...@open-fcoe.org
 W: www.Open-FCoE.org
 S: Supported
-- 
2.8.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: Reported regressions for 4.7 as of Sunday, 2016-06-19

2016-06-23 Thread Johannes Thumshirn 
[+ Cc linux-scsi@vger.kernel.org ]

On Wed, Jun 22, 2016 at 03:57:35PM +, Quinn Tran wrote:
> Johannes,  Martin,
> 
> Based on the screen shot/call trace,  it looks like this adapter is not using 
> MSIX.  It defaulted back to MSI or INTx interrupt.  The code made an 
> assumption  of MSIX is available.  There is no point in go through that code 
> segment.
> 
> Can you try this work around?  It’s untested.  Thanks.
> 
> 
> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 5649c20..e033ecb 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -2548,7 +2548,7 @@ void qla24xx_process_response_queue(struct 
> scsi_qla_host *vha,
> if (!vha->flags.online)
> return;
>  
> -   if (rsp->msix->cpuid != smp_processor_id()) {
> +   if (rsp->msix && (rsp->msix->cpuid != smp_processor_id())) {
> /* if kernel does not notify qla of IRQ's CPU change,
>  * then set it here.
>  */
> 

But this still does not fix the race which would be possible if the HBA is
using MSI-X but triggering IRQs early enough.

Have a look at this (I admit theoretical) path:
qla24xx_enable_msix(struct qla_hw_data *ha, struct rsp_que *rsp)
{
[...]
/* Enable MSI-X vectors for the base queue */
for (i = 0; i < 2; i++) {
qentry = >msix_entries[i];
if (IS_P3P_TYPE(ha))
ret = request_irq(qentry->vector,
qla82xx_msix_entries[i].handler,
0, qla82xx_msix_entries[i].name, rsp);
else
ret = request_irq(qentry->vector,
msix_entries[i].handler,
0, msix_entries[i].name, rsp);
if (ret)
goto msix_register_fail;
<--- IRQ arrives here
qentry->have_irq = 1;
qentry->rsp = rsp;
rsp->msix = qentry;

[...]


void qla24xx_process_response_queue(struct scsi_qla_host *vha,
struct rsp_que *rsp)
{
[...]
if (rsp->msix->cpuid != smp_processor_id()) {
  ^
  \--- rsp->msix == NULL

/* if kernel does not notify qla of IRQ's CPU change,
 * then set it here.
 */
rsp->msix->cpuid = smp_processor_id();
ha->tgt.rspq_vector_cpuid = rsp->msix->cpuid;

-- 
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 v2] byteswap: try to avoid __builtin_constant_p gcc bug

2016-06-23 Thread Tomas Winkler 
On Wed, Jun 22, 2016 at 3:25 PM, Arnd Bergmann  wrote:
> On Wednesday, June 22, 2016 2:44:21 PM CEST Tomas Winkler wrote:
>> >
>> > There are more than 20 files that have the statement: case cpu_to_...
>> > Sparse complains about: case __builtin_bswap, not about 
>> > __builtin_constant_p.
>>
>> There is even much more in the header files used in  initializers,
>> which also require constants.  I wonder if __builtin_bswap produces
>> constant expression correctly under gcc?
>
> In gcc-4.8 or later yes. gcc-4.6/4.7 on powerpc was a special case that we
> have worked around now, as the 16-bit byteswap there was not a constant
> expression, unlike the 32-bit and 64-bit ones.

Can you please give any references to that?
Just to make sure we are on the same line , the  bottom line is that
sparse has to be adjusted.

Thanks
Tomas
--
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 7/8] mmc: block: register RPMB partition with the RPMB subsystem

2016-06-23 Thread Adrian Hunter 
On 02/06/16 00:41, Tomas Winkler wrote:
> Register eMMC RPMB partition with the RPMB subsystem and provide
> implementation for the RPMB access operations abstracting
> actual multi step process.
> 
> V2: resend
> V3: commit message fix
> V4: Kconfig: use select RPMB to ensure valid configuration
> Switch back to main area after RPMB access
> 
> Signed-off-by: Tomas Winkler 
> Signed-off-by: Alexander Usyskin 
> ---



> +static void mmc_blk_rpmb_add(struct mmc_card *card)
> +{
> + struct mmc_blk_data *md = dev_get_drvdata(>dev);
> + struct mmc_blk_data *part_md = mmc_blk_rpmb_part_get(md);
> + struct rpmb_dev *rdev;
> +
> + if (!part_md)
> + return;
> +
> + mmc_blk_rpmb_set_dev_id(_rpmb_dev_ops, card);
> +
> + /* RPMB blocks are written in half sectors hence '* 2' */
> + mmc_rpmb_dev_ops.reliable_wr_cnt = card->ext_csd.rel_sectors * 2;

This looks like it does not support 8KB writes added in v5.1 spec.  Can that
be supported?

> +
> + rdev = rpmb_dev_register(disk_to_dev(part_md->disk),
> +   _rpmb_dev_ops);
> + if (IS_ERR(rdev)) {
> + pr_warn("%s: cannot register to rpmb %ld\n",
> + part_md->disk->disk_name, PTR_ERR(rdev));
> + }
> +}



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