[PATCH-next] scsi: libsas: dynamically allocate and free ata host

2018-05-09 Thread Jason Yan 
Commit 2623c7a5f2 ("libata: add refcounting to ata_host") v4.17+
introduced refcounting to ata_host and will increase or decrease the
refcount when adding or deleting transport ATA port.

Now the ata host for libsas is embedded in domain_device, and the ->kref
member is not initialized. Afer we add ata transport class,
ata_host_get() will be called when adding transport ATA port and a
warning will be triggered as below:

refcount_t: increment on 0; use-after-free.
WARNING: CPU: 2 PID: 103 at lib/refcount.c:153 refcount_inc+0x40/0x48
..
Call trace:
 refcount_inc+0x40/0x48
 ata_host_get+0x10/0x18
 ata_tport_add+0x40/0x120
 ata_sas_tport_add+0xc/0x14
 sas_ata_init+0x7c/0xc8
 sas_discover_domain+0x380/0x53c
 process_one_work+0x12c/0x288
 worker_thread+0x58/0x3f0
 kthread+0xfc/0x128
 ret_from_fork+0x10/0x18

And also when removing transport ATA port ata_host_put() will be called
and another similar warning will be triggered. If the refcount decreased
to zero, the ata host will be freed. But this ata host is only part of
domain_device, it cannot be freed directly.

So we have to change this embedded static ata host to a dynamically
allocated ata host and initialize the ->kref member. To use
ata_host_get() and ata_host_put() in libsas, we need to move the
declaration of these functions to the public libata.h and export them.

Fixes: b6240a4df018 ("scsi: libsas: add transport class for ATA devices")
Signed-off-by: Jason Yan 
CC: John Garry 
CC: Taras Kondratiuk 
CC: Tejun Heo 
---
 drivers/ata/libata-core.c  |  3 +++
 drivers/ata/libata.h   |  2 --
 drivers/scsi/libsas/sas_ata.c  | 40 +-
 drivers/scsi/libsas/sas_discover.c |  2 ++
 include/linux/libata.h |  2 ++
 include/scsi/libsas.h  |  2 +-
 6 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 68596bd4cf06..cdb48dccfb45 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6424,6 +6424,7 @@ void ata_host_init(struct ata_host *host, struct device 
*dev,
host->n_tags = ATA_MAX_QUEUE - 1;
host->dev = dev;
host->ops = ops;
+   kref_init(>kref);
 }
 
 void __ata_port_probe(struct ata_port *ap)
@@ -7391,3 +7392,5 @@ EXPORT_SYMBOL_GPL(ata_cable_80wire);
 EXPORT_SYMBOL_GPL(ata_cable_unknown);
 EXPORT_SYMBOL_GPL(ata_cable_ignore);
 EXPORT_SYMBOL_GPL(ata_cable_sata);
+EXPORT_SYMBOL_GPL(ata_host_get);
+EXPORT_SYMBOL_GPL(ata_host_put);
\ No newline at end of file
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 9e21c49cf6be..f953cb4bb1ba 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -100,8 +100,6 @@ extern int ata_port_probe(struct ata_port *ap);
 extern void __ata_port_probe(struct ata_port *ap);
 extern unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
  u8 page, void *buf, unsigned int sectors);
-extern void ata_host_get(struct ata_host *host);
-extern void ata_host_put(struct ata_host *host);
 
 #define to_ata_port(d) container_of(d, struct ata_port, tdev)
 
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index ff1d612f6fb9..41cdda7a926b 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -557,34 +557,46 @@ int sas_ata_init(struct domain_device *found_dev)
 {
struct sas_ha_struct *ha = found_dev->port->ha;
struct Scsi_Host *shost = ha->core.shost;
+   struct ata_host *ata_host;
struct ata_port *ap;
int rc;
 
-   ata_host_init(_dev->sata_dev.ata_host, ha->dev, _sata_ops);
-   ap = ata_sas_port_alloc(_dev->sata_dev.ata_host,
-   _port_info,
-   shost);
+   ata_host = kzalloc(sizeof(*ata_host), GFP_KERNEL);
+   if (!ata_host)  {
+   SAS_DPRINTK("ata host alloc failed.\n");
+   return -ENOMEM;
+   }
+
+   ata_host_init(ata_host, ha->dev, _sata_ops);
+
+   ap = ata_sas_port_alloc(ata_host, _port_info, shost);
if (!ap) {
SAS_DPRINTK("ata_sas_port_alloc failed.\n");
-   return -ENODEV;
+   rc = -ENODEV;
+   goto free_host;
}
 
ap->private_data = found_dev;
ap->cbl = ATA_CBL_SATA;
ap->scsi_host = shost;
rc = ata_sas_port_init(ap);
-   if (rc) {
-   ata_sas_port_destroy(ap);
-   return rc;
-   }
-   rc = ata_sas_tport_add(found_dev->sata_dev.ata_host.dev, ap);
-   if (rc) {
-   ata_sas_port_destroy(ap);
-   return rc;
-   }
+   if (rc)
+   goto destroy_port;
+
+   rc = ata_sas_tport_add(ata_host->dev, ap);
+   if (rc)
+   goto destroy_port;
+
+   found_dev->sata_dev.ata_host = ata_host;

Re: [PATCH v6 05/13] firmware_loader: enhance Kconfig documentation over FW_LOADER

2018-05-09 Thread Kees Cook 
On Wed, May 9, 2018 at 1:55 PM, Luis R. Rodriguez  wrote:
> On Tue, May 08, 2018 at 03:42:33PM -0700, Kees Cook wrote:
>> On Tue, May 8, 2018 at 11:12 AM, Luis R. Rodriguez  wrote:
>> > + This used to be the default firmware loading facility, and udev 
>> > used
>> > + to listen for uvents to load firmware for the kernel. The 
>> > firmware
>> > + loading facility functionality in udev has been removed, as such 
>> > it
>> > + can no longer be relied upon as a fallback mechanism. Linux no 
>> > longer
>> > + relies on or uses a fallback mechanism in userspace. If you need 
>> > to
>> > + rely on one refer to the permissively licensed firmwared:
>>
>> Typo: firmware
>
> Thanks fixed all typos except this one, this one is meant to be firmwared as
> that is the name of the project, the url is below.
>
>>
>> > +
>> > + https://github.com/teg/firmwared

Oh! Yes, hah. :) Thanks!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] libiscsi: add lock around task lists to fix list corruption regression

2018-05-09 Thread Ilkka Sovanto 
Hi again, Chris and others!

We're hitting this again on 4.14.32.

Looks like the path iscsi_queuecommand -> prepd_reject/prepd_fault
results in iscsi_complete_task getting called with frwd_lock held
instead of the requisite back_lock which might explain the task list
corruption under heavy loads.

An alternate possibly problematic situation seems to be
fail_mgmt_tasks which calls iscsi_complete_task while holding
frwd_lock from iscsi_start_session_recovery.

See also: https://github.com/open-iscsi/open-iscsi/issues/56

Attached an ugly attempt at ensuring back_lock is held instead.

 - Ilkka

On 5 March 2017 at 22:49, Ilkka Sovanto  wrote:
> Hi!
>
> Was running pretty nicely for a week or so, until we got this one:
>
> [534630.679965] BUG: unable to handle kernel at 0078
> [534630.684035] IP: [] iscsi_xmit_task+0x29/0xc0
> [534630.685724] PGD a1fcd3067
> [534630.687346] Oops: 0002 [#1]
> [534630.688846] Modules linked in:
> [534630.690348] CPU: 17 PID: 7988 Comm: kworker/u56:6 Tainted: GW
> [534630.693327] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 41113_115728-nilsson.home.kraxel.org 04/01/2014
> [534630.696404] Workqueue: iscsi_q_8 iscsi_xmitworker
> [534630.697948] task: 8800bab62f00 ti: 880c0d9d8000 task.ti:
> 880c0d9d8000
> [534630.700837] RIP: 0010:[] []
> iscsi_xmit_task+0x29/0xc0
> [534630.703689] RSP: 0018:880c0d9dbdb0  EFLAGS: 00010246
> [534630.705209] RAX: ffc3 RBX: 880fe3dfda10 RCX:
> dead0200
> [534630.708062] RDX:  RSI: 0200 RDI:
> 880fe3dfda10
> [534630.710773] RBP: 880c0d9dbdc8 R08: 880fe3881c80 R09:
> 
> [534630.713539] R10: 880fb799f710 R11: 0260 R12:
> 880fe3dfda10
> [534630.716344] R13:  R14: 880fe3881c80 R15:
> 880fe3dfdae0
> [534630.719143] FS:  () GS:880fff42()
> [534630.722001] CS:  0010 DS:  ES:  CR0: 80050033
> [534630.723492] CR2: 0078 CR3: 000aa8e44000 CR4:
> 000406e0
> [534630.726163] Stack:
> [534630.727622]  880fe3dfdaa8 880fe3dfda10 880fe3dfdac0
> 880c0d9dbe18
> [534630.730906]  8171226d 880fe3dfdad0 880fe3881c00
> 880fe3dfdab0
> [534630.767489]  880ff7d98780 880fe3dfdae0 880ffec99400
> 880fe3dfdae8
>
> [534630.770543] Call Trace:
> [534630.771967]  [] iscsi_xmitworker+0x1dd/0x310
> [534630.773523]  [] process_one_work+0x149/0x3e0
> [534630.775084]  [] worker_thread+0x69/0x470
> [534630.776585]  [] ? process_one_work+0x3e0/0x3e0
> [534630.778131]  [] ? process_one_work+0x3e0/0x3e0
> [534630.779682]  [] kthread+0xea/0x100
> [534630.781300]  [] ? kthread_create_on_node+0x1a0/0x1a0
> [534630.782872]  [] ret_from_fork+0x3f/0x70
> [534630.784340]  [] ? kthread_create_on_node+0x1a0/0x1a0
> [534630.785885] Code:
> [534630.792699] RIP [] iscsi_xmit_task+0x29/0xc0
> [534630.794266]  RSP 
> [534630.795617] CR2: 0078
> [534630.798051] ---[ end trace 9d963f95212dfbe2 ]---
> [534630.799788] Kernel panic - not syncing: Fatal exception in interrupt
> [534630.802712] Kernel Offset: disabled
> [534630.804304] ---[ end Kernel panic - not syncing: Fatal exception
> in interrupt
>
> And here's the disassembly for iscsi_xmit_task:
>
> 8170efa0 :
> 8170efa0:   e8 0b fd 52 00  call
> 81c3ecb0 <__fentry__>
> 8170efa5:   55  push   rbp
> 8170efa6:   b8 c3 ff ff ff  moveax,0xffc3
> 8170efab:   48 89 e5movrbp,rsp
> 8170efae:   41 55   push   r13
> 8170efb0:   41 54   push   r12
> 8170efb2:   53  push   rbx
> 8170efb3:   48 8b 97 f0 00 00 00movrdx,QWORD PTR 
> [rdi+0xf0]
> 8170efba:   4c 8b af 90 00 00 00movr13,QWORD PTR 
> [rdi+0x90]
> 8170efc1:   83 e2 02andedx,0x2
> 8170efc4:   75 71   jne
> 8170f037 
> 8170efc6:   48 89 fbmovrbx,rdi
> ***
> 8170efc9:   f0 41 ff 45 78  lock inc DWORD PTR [r13+0x78]
> ***
> 8170efce:   48 8b 47 10 movrax,QWORD PTR 
> [rdi+0x10]
> 8170efd2:   48 8d b8 18 01 00 00leardi,[rax+0x118]
> 8170efd9:   e8 b2 d3 52 00  call
> 81c3c390 <_raw_spin_unlock_bh>
> 8170efde:   48 8b 43 10 movrax,QWORD PTR 
> [rbx+0x10]
> 8170efe2:   4c 89 efmovrdi,r13
> 8170efe5:   48 8b 80 00 01 00 00movrax,QWORD PTR 
> [rax+0x100]
> 8170efec:   ff 90 98 00 00 00   call   QWORD PTR [rax+0x98]
> 8170eff2:   41 89 c4movr12d,eax
> 8170eff5:   48 8b 43 10 mov

[PATCH v2] target: transport should allow st ILI reads

2018-05-09 Thread Lee Duncan 
When a tape drive is exported via LIO using the
pscsi module, a read that requests more bytes per block
than the tape can supply returns an empty buffer. This
is because the pscsi pass-through target module sees
the "ILI" illegal length bit set and thinks there
is no reason to return the data.

This is a long-standing transport issue, since it
assume that no data need be returned under a check
condition, which isn't always the case for tape.

Add in a check for tape reads with the the ILI, EOM,
or FM bits set, with a sense code of NO_SENSE,
treating such cases as if there is no sense data
and the read succeeded. The layered tape driver then
"does the right thing" when it gets such a response.

Changes from RFC:
 - Moved ugly code from transport to pscsi module
 - Added checking EOM and FM bits, as well as ILI
 - fixed malformed patch
 - Clarified description a bit

Signed-off-by: Lee Duncan 
---
 drivers/target/target_core_pscsi.c | 22 +-
 drivers/target/target_core_transport.c |  6 ++
 include/target/target_core_base.h  |  1 +
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_pscsi.c 
b/drivers/target/target_core_pscsi.c
index 0d99b242e82e..b237104af81c 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -689,8 +689,28 @@ static void pscsi_complete_cmd(struct se_cmd *cmd, u8 
scsi_status,
}
 after_mode_select:
 
-   if (scsi_status == SAM_STAT_CHECK_CONDITION)
+   if (scsi_status == SAM_STAT_CHECK_CONDITION) {
transport_copy_sense_to_cmd(cmd, req_sense);
+
+   /*
+* hack to check for TAPE device reads with
+* FM/EOM/ILI set, so that we can get data
+* back despite framework assumption that a
+* check condition means there is no data
+*/
+   if ((sd->type == TYPE_TAPE) &&
+   (cmd->data_direction == DMA_FROM_DEVICE)) {
+   /*
+* is sense data valid, fixed format,
+* and have FM, EOM, or ILI set?
+*/
+   if ((req_sense[0] == 0xf0) &&   /* valid, fixed format 
*/
+   (req_sense[2] & 0xe0) &&/* FM, EOM, or ILI */
+   ((req_sense[2] & 0xf) == 0)) { /* key==NO_SENSE */
+   cmd->se_cmd_flags |= SCF_TREAT_READ_AS_NORMAL;
+   }
+   }
+   }
 }
 
 enum {
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 74b646f165d4..56661a824266 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2192,6 +2192,11 @@ static void target_complete_ok_work(struct work_struct 
*work)
if (atomic_read(>se_dev->dev_qf_count) != 0)
schedule_work(>se_dev->qf_work_queue);
 
+   if (cmd->se_cmd_flags & SCF_TREAT_READ_AS_NORMAL) {
+   pr_debug("Tape FM/EOM/ILI status detected. Treating as OK\n");
+   goto treat_as_normal_read;
+   }
+
/*
 * Check if we need to send a sense buffer from
 * the struct se_cmd in question.
@@ -2241,6 +2246,7 @@ static void target_complete_ok_work(struct work_struct 
*work)
if (cmd->scsi_status)
goto queue_status;
 
+treat_as_normal_read:
atomic_long_add(cmd->data_length,
>se_lun->lun_stats.tx_data_octets);
/*
diff --git a/include/target/target_core_base.h 
b/include/target/target_core_base.h
index 9f9f5902af38..922a39f45abc 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -143,6 +143,7 @@ enum se_cmd_flags_table {
SCF_ACK_KREF= 0x0040,
SCF_USE_CPUID   = 0x0080,
SCF_TASK_ATTR_SET   = 0x0100,
+   SCF_TREAT_READ_AS_NORMAL= 0x0200,
 };
 
 /*
-- 
2.13.6

Re: [PATCH v6 05/13] firmware_loader: enhance Kconfig documentation over FW_LOADER

2018-05-09 Thread Luis R. Rodriguez 
On Tue, May 08, 2018 at 03:42:33PM -0700, Kees Cook wrote:
> On Tue, May 8, 2018 at 11:12 AM, Luis R. Rodriguez  wrote:
> > + This used to be the default firmware loading facility, and udev 
> > used
> > + to listen for uvents to load firmware for the kernel. The firmware
> > + loading facility functionality in udev has been removed, as such 
> > it
> > + can no longer be relied upon as a fallback mechanism. Linux no 
> > longer
> > + relies on or uses a fallback mechanism in userspace. If you need 
> > to
> > + rely on one refer to the permissively licensed firmwared:
> 
> Typo: firmware

Thanks fixed all typos except this one, this one is meant to be firmwared as
that is the name of the project, the url is below.

> 
> > +
> > + https://github.com/teg/firmwared

  Luis

[PATCH] ipr: new IOASC update

2018-05-09 Thread wenxiong 
From: Wen Xiong 

This patch adds new adapter error log for P9 system with the new
AZ SAS cable.

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

---
 drivers/scsi/ipr.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index dda1a64..6615ad8 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -435,6 +435,8 @@ struct ipr_error_table_t ipr_error_table[] = {
"4080: IOA exceeded maximum operating temperature"},
{0x060B8000, 0, IPR_DEFAULT_LOG_LEVEL,
"4085: Service required"},
+   {0x060B8100, 0, IPR_DEFAULT_LOG_LEVEL,
+   "4086: SAS Adapter Hardware Configuration Error"},
{0x06288000, 0, IPR_DEFAULT_LOG_LEVEL,
"3140: Device bus not ready to ready transition"},
{0x0629, 0, IPR_DEFAULT_LOG_LEVEL,
-- 
1.6.0.2

Re: [PATCH v6 12/13] Documentation: remove stale firmware API reference

2018-05-09 Thread Luis R. Rodriguez 
On Wed, May 09, 2018 at 12:12:09PM -0300, Mauro Carvalho Chehab wrote:
> Em Tue,  8 May 2018 11:12:46 -0700
> "Luis R. Rodriguez"  escreveu:
> 
> > It refers to a pending patch, but this was merged eons ago.
> 
> Didn't know that such patch was already merged. Great!
> 
> Regards,
> Mauro
> 
> > 
> > Signed-off-by: Luis R. Rodriguez 
> > ---
> >  Documentation/dell_rbu.txt | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/Documentation/dell_rbu.txt b/Documentation/dell_rbu.txt
> > index 0fdb6aa2704c..077fdc29a0d0 100644
> > --- a/Documentation/dell_rbu.txt
> > +++ b/Documentation/dell_rbu.txt
> > @@ -121,9 +121,6 @@ read back the image downloaded.
> >  
> >  .. note::
> >  
> > -   This driver requires a patch for firmware_class.c which has the modified
> > -   request_firmware_nowait function.
> > -
> 
> > Also after updating the BIOS image a user mode application needs to 
> > execute
> > code which sends the BIOS update request to the BIOS. So on the next 
> > reboot
> > the BIOS knows about the new image downloaded and it updates itself.
> 
> You should likely remove the "Also" here.

Good catch. Will adjust.

> 
> With that,
> 
> Reviewed-by: Mauro Carvalho Chehab 

Great, thanks.

  Luis

Re: simplify procfs code for seq_file instances V2

2018-05-09 Thread Alexey Dobriyan 
On Sun, May 06, 2018 at 06:45:31PM +0100, Al Viro wrote:
> On Sun, May 06, 2018 at 08:19:49PM +0300, Alexey Dobriyan wrote:

> > @@ -62,9 +62,9 @@ struct proc_dir_entry {
> > umode_t mode;
> > u8 namelen;
> >  #ifdef CONFIG_64BIT
> > -#define SIZEOF_PDE_INLINE_NAME (192-139)
> > +#define SIZEOF_PDE_INLINE_NAME (192-155)
> >  #else
> > -#define SIZEOF_PDE_INLINE_NAME (128-87)
> > +#define SIZEOF_PDE_INLINE_NAME (128-95)
> 
> >  #endif
> > char inline_name[SIZEOF_PDE_INLINE_NAME];
> >  } __randomize_layout;
> 
> *UGH*

I agree. Maintaining these numbers is a pain point.
Who knew people were going to start adding fields right away.

> Both to the original state and that kind of "adjustments".
> Incidentally, with __bugger_layout in there these expressions
> are simply wrong.

Struct randomization is exempt from maintaining sizeof as they are already
screwing cachelines and everything. But if patch works with lockdep and
randomization -- even better.

> If nothing else, I would suggest turning the last one into
>   char inline_name[];
> in hope that layout won't get... randomized that much and
> used
> 
> #ifdef CONFIG_64BIT
> #define PDE_SIZE 192
> #else
> #define PDE_SIZE 128
> #endif
> 
> union __proc_dir_entry {
>   char pad[PDE_SIZE];
>   struct proc_dir_entry real;
> };
> 
> #define SIZEOF_PDE_INLINE_NAME (PDE_SIZE - offsetof(struct proc_dir_entry, 
> inline_name))
> 
> for constants, adjusted sizeof and sizeof_field when creating
> proc_dir_entry_cache and turned proc_root into
> 
> union __proc_dir_entry __proc_root = { .real = {
> .low_ino= PROC_ROOT_INO,
> .namelen= 5,
> .mode   = S_IFDIR | S_IRUGO | S_IXUGO,
> .nlink  = 2,
> .refcnt = REFCOUNT_INIT(1),
> .proc_iops  = _root_inode_operations,
> .proc_fops  = _root_operations,
> .parent = &__proc_root.real,
> .subdir = RB_ROOT,
> .name   = __proc_root.real.inline_name,
> .inline_name= "/proc",
> }};
> 
> #define proc_root __proc_root.real
> (or actually used __proc_root.real in all of a 6 places where it remains).
> 
> > -   size_t state_size = PDE(inode)->state_size;
> > +   unsigned int state_size = PDE(inode)->state_size;
> 
> 
> You and your "size_t is evil" crusade...

[nods]

unsigned long   flags;  /* error bits */
unsigned long   i_state;
unsigned long   s_blocksize;
unsigned long   s_flags;
unsigned long   s_iflags;   /* internal SB_I_* flags */

[PATCH 2/6] scsi: hisi_sas: make return type of prep functions void

2018-05-09 Thread John Garry 
From: Xiang Chen 

Since the task prep functions now should not fail, adjust
the return types to void.

In addition, some checks in the task prep functions are
relocated to the main module; this is specifically the
check for the number of elements in an sg list exceeded
the HW SGE limit.

Signed-off-by: Xiang Chen 
Signed-off-by: John Garry 
---
 drivers/scsi/hisi_sas/hisi_sas.h   |  8 +++---
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 45 ++
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 28 +
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 45 +-
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 43 +---
 5 files changed, 51 insertions(+), 118 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 4410538..3a1caa04 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -214,14 +214,14 @@ struct hisi_sas_hw {
void (*sl_notify)(struct hisi_hba *hisi_hba, int phy_no);
int (*get_free_slot)(struct hisi_hba *hisi_hba, struct hisi_sas_dq *dq);
void (*start_delivery)(struct hisi_sas_dq *dq);
-   int (*prep_ssp)(struct hisi_hba *hisi_hba,
+   void (*prep_ssp)(struct hisi_hba *hisi_hba,
struct hisi_sas_slot *slot, int is_tmf,
struct hisi_sas_tmf_task *tmf);
-   int (*prep_smp)(struct hisi_hba *hisi_hba,
+   void (*prep_smp)(struct hisi_hba *hisi_hba,
struct hisi_sas_slot *slot);
-   int (*prep_stp)(struct hisi_hba *hisi_hba,
+   void (*prep_stp)(struct hisi_hba *hisi_hba,
struct hisi_sas_slot *slot);
-   int (*prep_abort)(struct hisi_hba *hisi_hba,
+   void (*prep_abort)(struct hisi_hba *hisi_hba,
  struct hisi_sas_slot *slot,
  int device_id, int abort_flag, int tag_to_abort);
int (*slot_complete)(struct hisi_hba *hisi_hba,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 0ce7c71..2772e920 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -243,30 +243,30 @@ void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, 
struct sas_task *task,
 }
 EXPORT_SYMBOL_GPL(hisi_sas_slot_task_free);
 
-static int hisi_sas_task_prep_smp(struct hisi_hba *hisi_hba,
+static void hisi_sas_task_prep_smp(struct hisi_hba *hisi_hba,
  struct hisi_sas_slot *slot)
 {
-   return hisi_hba->hw->prep_smp(hisi_hba, slot);
+   hisi_hba->hw->prep_smp(hisi_hba, slot);
 }
 
-static int hisi_sas_task_prep_ssp(struct hisi_hba *hisi_hba,
+static void hisi_sas_task_prep_ssp(struct hisi_hba *hisi_hba,
  struct hisi_sas_slot *slot, int is_tmf,
  struct hisi_sas_tmf_task *tmf)
 {
-   return hisi_hba->hw->prep_ssp(hisi_hba, slot, is_tmf, tmf);
+   hisi_hba->hw->prep_ssp(hisi_hba, slot, is_tmf, tmf);
 }
 
-static int hisi_sas_task_prep_ata(struct hisi_hba *hisi_hba,
+static void hisi_sas_task_prep_ata(struct hisi_hba *hisi_hba,
  struct hisi_sas_slot *slot)
 {
-   return hisi_hba->hw->prep_stp(hisi_hba, slot);
+   hisi_hba->hw->prep_stp(hisi_hba, slot);
 }
 
-static int hisi_sas_task_prep_abort(struct hisi_hba *hisi_hba,
+static void hisi_sas_task_prep_abort(struct hisi_hba *hisi_hba,
struct hisi_sas_slot *slot,
int device_id, int abort_flag, int tag_to_abort)
 {
-   return hisi_hba->hw->prep_abort(hisi_hba, slot,
+   hisi_hba->hw->prep_abort(hisi_hba, slot,
device_id, abort_flag, tag_to_abort);
 }
 
@@ -395,6 +395,13 @@ static int hisi_sas_task_prep(struct sas_task *task, 
struct hisi_sas_dq
} else
n_elem = task->num_scatter;
 
+   if (n_elem > HISI_SAS_SGE_PAGE_CNT) {
+   dev_err(dev, "task prep: n_elem(%d) > HISI_SAS_SGE_PAGE_CNT",
+   n_elem);
+   rc = -EINVAL;
+   goto err_out_dma_unmap;
+   }
+
spin_lock_irqsave(_hba->lock, flags);
if (hisi_hba->hw->slot_index_alloc)
rc = hisi_hba->hw->slot_index_alloc(hisi_hba, _idx,
@@ -439,28 +446,22 @@ static int hisi_sas_task_prep(struct sas_task *task, 
struct hisi_sas_dq
 
switch (task->task_proto) {
case SAS_PROTOCOL_SMP:
-   rc = hisi_sas_task_prep_smp(hisi_hba, slot);
+   hisi_sas_task_prep_smp(hisi_hba, slot);
break;
case SAS_PROTOCOL_SSP:
-   rc = hisi_sas_task_prep_ssp(hisi_hba, slot, is_tmf, tmf);
+   hisi_sas_task_prep_ssp(hisi_hba, slot, is_tmf, tmf);
break;
case SAS_PROTOCOL_SATA:
case SAS_PROTOCOL_STP:

[PATCH 6/6] scsi: hisi_sas: add check of device in hisi_sas_task_exec()

2018-05-09 Thread John Garry 
From: Xiaofei Tan 

Currently we don't check that device is not gone before
dereferencing it's elements in the function
hisi_sas_task_exec() (specifically, the DQ pointer).

This patch fixes this issue by filling in the DQ
pointer in hisi_sas_task_prep(), after we check
that the device pointer is still safe to
reference.

Signed-off-by: Xiaofei Tan 
Signed-off-by: John Garry 
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index a451625..39f694e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -310,12 +310,13 @@ static void hisi_sas_slot_abort(struct work_struct *work)
task->task_done(task);
 }
 
-static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq *dq,
+static int hisi_sas_task_prep(struct sas_task *task,
+ struct hisi_sas_dq **dq_pointer,
  int is_tmf, struct hisi_sas_tmf_task *tmf,
  int *pass)
 {
-   struct hisi_hba *hisi_hba = dq->hisi_hba;
struct domain_device *device = task->dev;
+   struct hisi_hba *hisi_hba = dev_to_hisi_hba(device);
struct hisi_sas_device *sas_dev = device->lldd_dev;
struct hisi_sas_port *port;
struct hisi_sas_slot *slot;
@@ -323,8 +324,9 @@ static int hisi_sas_task_prep(struct sas_task *task, struct 
hisi_sas_dq *dq,
struct asd_sas_port *sas_port = device->port;
struct device *dev = hisi_hba->dev;
int dlvry_queue_slot, dlvry_queue, rc, slot_idx;
-   int  n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
+   int n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
unsigned long flags, flags_dq;
+   struct hisi_sas_dq *dq;
int wr_q_index;
 
if (!sas_port) {
@@ -352,6 +354,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct 
hisi_sas_dq *dq,
return -ECOMM;
}
 
+   *dq_pointer = dq = sas_dev->dq;
+
port = to_hisi_sas_port(sas_port);
if (port && !port->port_attached) {
dev_info(dev, "task prep: %s port%d not attach device\n",
@@ -520,22 +524,21 @@ static int hisi_sas_task_exec(struct sas_task *task, 
gfp_t gfp_flags,
unsigned long flags;
struct hisi_hba *hisi_hba = dev_to_hisi_hba(task->dev);
struct device *dev = hisi_hba->dev;
-   struct domain_device *device = task->dev;
-   struct hisi_sas_device *sas_dev = device->lldd_dev;
-   struct hisi_sas_dq *dq = sas_dev->dq;
+   struct hisi_sas_dq *dq = NULL;
 
if (unlikely(test_bit(HISI_SAS_REJECT_CMD_BIT, _hba->flags)))
return -EINVAL;
 
/* protect task_prep and start_delivery sequence */
-   rc = hisi_sas_task_prep(task, dq, is_tmf, tmf, );
+   rc = hisi_sas_task_prep(task, , is_tmf, tmf, );
if (rc)
dev_err(dev, "task exec: failed[%d]!\n", rc);
 
-   spin_lock_irqsave(>lock, flags);
-   if (likely(pass))
+   if (likely(pass)) {
+   spin_lock_irqsave(>lock, flags);
hisi_hba->hw->start_delivery(dq);
-   spin_unlock_irqrestore(>lock, flags);
+   spin_unlock_irqrestore(>lock, flags);
+   }
 
return rc;
 }
-- 
1.9.1

[PATCH 3/6] scsi: hisi_sas: allocate slot buffer earlier

2018-05-09 Thread John Garry 
From: Xiang Chen 

Currently we allocate the slot's memory buffer
after allocating the DQ slot.

To aid DQ lockout reduction, and allow slots to be
built in parallel, move this step (which can fail)
prior to allocating the slot.

Also a stray spin_unlock_irqrestore() is removed
from internal task exec function.

Signed-off-by: Xiang Chen 
Signed-off-by: John Garry 
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 55 ---
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 2772e920..58cbe1f 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -412,14 +412,22 @@ static int hisi_sas_task_prep(struct sas_task *task, 
struct hisi_sas_dq
if (rc)
goto err_out_dma_unmap;
 
+   slot = _hba->slot_info[slot_idx];
+   memset(slot, 0, sizeof(struct hisi_sas_slot));
+
+   slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
+  GFP_ATOMIC, >buf_dma);
+   if (!slot->buf) {
+   rc = -ENOMEM;
+   goto err_out_tag;
+   }
+
rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
if (rc)
-   goto err_out_tag;
+   goto err_out_buf;
 
dlvry_queue = dq->id;
dlvry_queue_slot = dq->wr_point;
-   slot = _hba->slot_info[slot_idx];
-   memset(slot, 0, sizeof(struct hisi_sas_slot));
 
slot->idx = slot_idx;
slot->n_elem = n_elem;
@@ -434,12 +442,6 @@ static int hisi_sas_task_prep(struct sas_task *task, 
struct hisi_sas_dq
task->lldd_task = slot;
INIT_WORK(>abort_slot, hisi_sas_slot_abort);
 
-   slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
-  GFP_ATOMIC, >buf_dma);
-   if (!slot->buf) {
-   rc = -ENOMEM;
-   goto err_out_slot_buf;
-   }
memset(slot->cmd_hdr, 0, sizeof(struct hisi_sas_cmd_hdr));
memset(hisi_sas_cmd_hdr_addr_mem(slot), 0, HISI_SAS_COMMAND_TABLE_SZ);
memset(hisi_sas_status_buf_addr_mem(slot), 0, HISI_SAS_STATUS_BUF_SZ);
@@ -474,8 +476,9 @@ static int hisi_sas_task_prep(struct sas_task *task, struct 
hisi_sas_dq
 
return 0;
 
-err_out_slot_buf:
-   /* Nothing to be done */
+err_out_buf:
+   dma_pool_free(hisi_hba->buffer_pool, slot->buf,
+ slot->buf_dma);
 err_out_tag:
spin_lock_irqsave(_hba->lock, flags);
hisi_sas_slot_index_free(hisi_hba, slot_idx);
@@ -1519,17 +1522,26 @@ static int hisi_sas_query_task(struct sas_task *task)
}
spin_unlock_irqrestore(_hba->lock, flags);
 
+   slot = _hba->slot_info[slot_idx];
+   memset(slot, 0, sizeof(struct hisi_sas_slot));
+
+   slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
+   GFP_ATOMIC, >buf_dma);
+   if (!slot->buf) {
+   rc = -ENOMEM;
+   goto err_out_tag;
+   }
spin_lock_irqsave(>lock, flags_dq);
rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
-   if (rc)
-   goto err_out_tag;
+   if (rc) {
+   rc = -ENOMEM;
+   spin_unlock_irqrestore(>lock, flags_dq);
+   goto err_out_buf;
+   }
 
dlvry_queue = dq->id;
dlvry_queue_slot = dq->wr_point;
 
-   slot = _hba->slot_info[slot_idx];
-   memset(slot, 0, sizeof(struct hisi_sas_slot));
-
slot->idx = slot_idx;
slot->n_elem = n_elem;
slot->dlvry_queue = dlvry_queue;
@@ -1541,13 +1553,6 @@ static int hisi_sas_query_task(struct sas_task *task)
slot->is_internal = true;
task->lldd_task = slot;
 
-   slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
-   GFP_ATOMIC, >buf_dma);
-   if (!slot->buf) {
-   rc = -ENOMEM;
-   goto err_out_tag;
-   }
-
memset(slot->cmd_hdr, 0, sizeof(struct hisi_sas_cmd_hdr));
memset(hisi_sas_cmd_hdr_addr_mem(slot), 0, HISI_SAS_COMMAND_TABLE_SZ);
memset(hisi_sas_status_buf_addr_mem(slot), 0, HISI_SAS_STATUS_BUF_SZ);
@@ -1570,11 +1575,13 @@ static int hisi_sas_query_task(struct sas_task *task)
 
return 0;
 
+err_out_buf:
+   dma_pool_free(hisi_hba->buffer_pool, slot->buf,
+ slot->buf_dma);
 err_out_tag:
spin_lock_irqsave(_hba->lock, flags);
hisi_sas_slot_index_free(hisi_hba, slot_idx);
spin_unlock_irqrestore(_hba->lock, flags);
-   spin_unlock_irqrestore(>lock, flags_dq);
 err_out:
dev_err(dev, "internal abort task prep: failed[%d]!\n", rc);
 
-- 
1.9.1

[PATCH 4/6] scsi: hisi_sas: Don't lock DQ for complete task sending

2018-05-09 Thread John Garry 
From: Xiang Chen 

Currently we lock the DQ to protect whole delivery process.
So this stops us building slots for the same queue in
parallel, and can affect performance.

To optimise it, only lock the DQ during special periods,
specifically when allocating a slot from the DQ and when
delivering a slot to the HW.

This approach is now safe, thanks to the previous patches
to ensure that we always deliver a slot to the HW once
allocated.

Signed-off-by: Xiang Chen 
Signed-off-by: John Garry 
---
 drivers/scsi/hisi_sas/hisi_sas.h   |  4 ++-
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 49 --
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 27 ++-
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 29 +++-
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 28 ++-
 5 files changed, 96 insertions(+), 41 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 3a1caa04..52fc709 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -161,7 +161,7 @@ struct hisi_sas_cq {
 
 struct hisi_sas_dq {
struct hisi_hba *hisi_hba;
-   struct hisi_sas_slot*slot_prep;
+   struct list_head list;
spinlock_t lock;
int wr_point;
int id;
@@ -181,6 +181,7 @@ struct hisi_sas_device {
 
 struct hisi_sas_slot {
struct list_head entry;
+   struct list_head delivery;
struct sas_task *task;
struct hisi_sas_port*port;
u64 n_elem;
@@ -190,6 +191,7 @@ struct hisi_sas_slot {
int cmplt_queue_slot;
int idx;
int abort;
+   int ready;
void*buf;
dma_addr_t buf_dma;
void*cmd_hdr;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 58cbe1f..bf374a7 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -307,9 +307,9 @@ static void hisi_sas_slot_abort(struct work_struct *work)
task->task_done(task);
 }
 
-static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
-   *dq, int is_tmf, struct hisi_sas_tmf_task *tmf,
-   int *pass)
+static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq *dq,
+ int is_tmf, struct hisi_sas_tmf_task *tmf,
+ int *pass)
 {
struct hisi_hba *hisi_hba = dq->hisi_hba;
struct domain_device *device = task->dev;
@@ -321,7 +321,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct 
hisi_sas_dq
struct device *dev = hisi_hba->dev;
int dlvry_queue_slot, dlvry_queue, rc, slot_idx;
int  n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
-   unsigned long flags;
+   unsigned long flags, flags_dq;
+   int wr_q_index;
 
if (!sas_port) {
struct task_status_struct *ts = >task_status;
@@ -422,12 +423,18 @@ static int hisi_sas_task_prep(struct sas_task *task, 
struct hisi_sas_dq
goto err_out_tag;
}
 
-   rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
-   if (rc)
+   spin_lock_irqsave(>lock, flags_dq);
+   wr_q_index = hisi_hba->hw->get_free_slot(hisi_hba, dq);
+   if (wr_q_index < 0) {
+   spin_unlock_irqrestore(>lock, flags_dq);
goto err_out_buf;
+   }
+
+   list_add_tail(>delivery, >list);
+   spin_unlock_irqrestore(>lock, flags_dq);
 
dlvry_queue = dq->id;
-   dlvry_queue_slot = dq->wr_point;
+   dlvry_queue_slot = wr_q_index;
 
slot->idx = slot_idx;
slot->n_elem = n_elem;
@@ -471,8 +478,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct 
hisi_sas_dq
task->task_state_flags |= SAS_TASK_AT_INITIATOR;
spin_unlock_irqrestore(>task_state_lock, flags);
 
-   dq->slot_prep = slot;
++(*pass);
+   slot->ready = 1;
 
return 0;
 
@@ -518,11 +525,11 @@ static int hisi_sas_task_exec(struct sas_task *task, 
gfp_t gfp_flags,
return -EINVAL;
 
/* protect task_prep and start_delivery sequence */
-   spin_lock_irqsave(>lock, flags);
rc = hisi_sas_task_prep(task, dq, is_tmf, tmf, );
if (rc)
dev_err(dev, "task exec: failed[%d]!\n", rc);
 
+   spin_lock_irqsave(>lock, flags);
if (likely(pass))
hisi_hba->hw->start_delivery(dq);
spin_unlock_irqrestore(>lock, flags);
@@ -1503,7 +1510,8 @@ static int hisi_sas_query_task(struct sas_task *task)
struct hisi_sas_cmd_hdr *cmd_hdr_base;
struct hisi_sas_dq *dq = sas_dev->dq;
int dlvry_queue_slot, dlvry_queue, n_elem = 0, rc, slot_idx;
-   unsigned long flags, flags_dq;
+   unsigned long flags, flags_dq = 0;
+   int wr_q_index;
 
if

Re: [PATCH 06/10] lpfc: Check if SCSI scanning is complete, before allowing I/O commands

2018-05-09 Thread Ewan D. Milne 
On Fri, 2018-05-04 at 20:37 -0700, James Smart wrote:
> A race condition is being seen. I/O is being issued to scsi targets
> simultaneous to a short connectivity loss. The rport block/unblock is
> occurring, but the unblock is allowing pending I/O commands to intermix
> with the commands being used to scan the scsi bus. Thus the target
> device is getting a read or write prior to transitioning to an ready
> state, thus the device errors the io. The io error propagatest all the
> way back to the application.
> 
> Resolve by augmenting the io submission with a check to see whether
> transport is scanning the remote port or not. If not, then the io is
> failed such that it will be retried. If so, then normal processing resumes.

This seems like a hack to me.  What exactly is the problem, is the
target rejecting the READ and/or WRITE commands for some reason?
Can you explain what you mean by "..transitioning to an ready state"?

Perhaps this would be better handled by some kind of interlock in
the transport layer with the scan_work?

-Ewan

Re: [PATCH v6 12/13] Documentation: remove stale firmware API reference

2018-05-09 Thread Mauro Carvalho Chehab 
Em Tue,  8 May 2018 11:12:46 -0700
"Luis R. Rodriguez"  escreveu:

> It refers to a pending patch, but this was merged eons ago.

Didn't know that such patch was already merged. Great!

Regards,
Mauro

> 
> Signed-off-by: Luis R. Rodriguez 
> ---
>  Documentation/dell_rbu.txt | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/Documentation/dell_rbu.txt b/Documentation/dell_rbu.txt
> index 0fdb6aa2704c..077fdc29a0d0 100644
> --- a/Documentation/dell_rbu.txt
> +++ b/Documentation/dell_rbu.txt
> @@ -121,9 +121,6 @@ read back the image downloaded.
>  
>  .. note::
>  
> -   This driver requires a patch for firmware_class.c which has the modified
> -   request_firmware_nowait function.
> -

> Also after updating the BIOS image a user mode application needs to 
> execute
> code which sends the BIOS update request to the BIOS. So on the next 
> reboot
> the BIOS knows about the new image downloaded and it updates itself.

You should likely remove the "Also" here.

With that,

Reviewed-by: Mauro Carvalho Chehab 

Regards,
Mauro

[PATCH 5/6] scsi: hisi_sas: Use device lock to protect slot alloc/free

2018-05-09 Thread John Garry 
From: Xiang Chen 

The IPTT of a slot is unique, and we currently use hisi_hba lock
to protect it.

Now slot is managed on hisi_sas_device.list, so use DQ lock to
protect for allocating and freeing the slot.

Signed-off-by: Xiang Chen 
Signed-off-by: John Garry 
---
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 56 ++
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c |  3 --
 2 files changed, 16 insertions(+), 43 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index bf374a7..a451625 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -214,6 +214,8 @@ static void hisi_sas_slot_index_init(struct hisi_hba 
*hisi_hba)
 void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, struct sas_task *task,
 struct hisi_sas_slot *slot)
 {
+   struct hisi_sas_dq *dq = _hba->dq[slot->dlvry_queue];
+   unsigned long flags;
 
if (task) {
struct device *dev = hisi_hba->dev;
@@ -233,11 +235,15 @@ void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, 
struct sas_task *task,
if (slot->buf)
dma_pool_free(hisi_hba->buffer_pool, slot->buf, slot->buf_dma);
 
+   spin_lock_irqsave(>lock, flags);
list_del_init(>entry);
+   spin_unlock_irqrestore(>lock, flags);
slot->buf = NULL;
slot->task = NULL;
slot->port = NULL;
+   spin_lock_irqsave(_hba->lock, flags);
hisi_sas_slot_index_free(hisi_hba, slot->idx);
+   spin_unlock_irqrestore(_hba->lock, flags);
 
/* slot memory is fully zeroed when it is reused */
 }
@@ -286,7 +292,6 @@ static void hisi_sas_slot_abort(struct work_struct *work)
struct scsi_lun lun;
struct device *dev = hisi_hba->dev;
int tag = abort_slot->idx;
-   unsigned long flags;
 
if (!(task->task_proto & SAS_PROTOCOL_SSP)) {
dev_err(dev, "cannot abort slot for non-ssp task\n");
@@ -300,9 +305,7 @@ static void hisi_sas_slot_abort(struct work_struct *work)
hisi_sas_debug_issue_ssp_tmf(task->dev, lun.scsi_lun, _task);
 out:
/* Do cleanup for this task */
-   spin_lock_irqsave(_hba->lock, flags);
hisi_sas_slot_task_free(hisi_hba, task, abort_slot);
-   spin_unlock_irqrestore(_hba->lock, flags);
if (task->task_done)
task->task_done(task);
 }
@@ -471,9 +474,9 @@ static int hisi_sas_task_prep(struct sas_task *task, struct 
hisi_sas_dq *dq,
break;
}
 
-   spin_lock_irqsave(_hba->lock, flags);
+   spin_lock_irqsave(>lock, flags);
list_add_tail(>entry, _dev->list);
-   spin_unlock_irqrestore(_hba->lock, flags);
+   spin_unlock_irqrestore(>lock, flags);
spin_lock_irqsave(>task_state_lock, flags);
task->task_state_flags |= SAS_TASK_AT_INITIATOR;
spin_unlock_irqrestore(>task_state_lock, flags);
@@ -1047,7 +1050,6 @@ static int hisi_sas_softreset_ata_disk(struct 
domain_device *device)
struct hisi_hba *hisi_hba = dev_to_hisi_hba(device);
struct device *dev = hisi_hba->dev;
int s = sizeof(struct host_to_dev_fis);
-   unsigned long flags;
 
ata_for_each_link(link, ap, EDGE) {
int pmp = sata_srst_pmp(link);
@@ -1072,11 +1074,8 @@ static int hisi_sas_softreset_ata_disk(struct 
domain_device *device)
dev_err(dev, "ata disk reset failed\n");
}
 
-   if (rc == TMF_RESP_FUNC_COMPLETE) {
-   spin_lock_irqsave(_hba->lock, flags);
+   if (rc == TMF_RESP_FUNC_COMPLETE)
hisi_sas_release_task(hisi_hba, device);
-   spin_unlock_irqrestore(_hba->lock, flags);
-   }
 
return rc;
 }
@@ -1173,7 +1172,6 @@ static int hisi_sas_controller_reset(struct hisi_hba 
*hisi_hba)
struct device *dev = hisi_hba->dev;
struct Scsi_Host *shost = hisi_hba->shost;
u32 old_state, state;
-   unsigned long flags;
int rc;
 
if (!hisi_hba->hw->soft_reset)
@@ -1197,9 +1195,7 @@ static int hisi_sas_controller_reset(struct hisi_hba 
*hisi_hba)
scsi_unblock_requests(shost);
goto out;
}
-   spin_lock_irqsave(_hba->lock, flags);
hisi_sas_release_tasks(hisi_hba);
-   spin_unlock_irqrestore(_hba->lock, flags);
 
clear_bit(HISI_SAS_REJECT_CMD_BIT, _hba->flags);
 
@@ -1274,11 +1270,8 @@ static int hisi_sas_abort_task(struct sas_task *task)
 * will have already been completed
 */
if (rc == TMF_RESP_FUNC_COMPLETE && rc2 != TMF_RESP_FUNC_SUCC) {
-   if (task->lldd_task) {
-   spin_lock_irqsave(_hba->lock, flags);
+   if (task->lldd_task)
hisi_sas_do_release_task(hisi_hba, task, slot);
-

[PATCH 1/6] scsi: hisi_sas: relocate smp sg map

2018-05-09 Thread John Garry 
From: Xiang Chen 

Currently we use DQ lock to protect delivery of DQ
entry one by one.

To optimise to allow more than one slot to be built
for a single DQ in parallel, we need to remove the DQ
lock when preparing slots, prior to delivery.

To achieve this, we rearrange the slot build order to
ensure that once we allocate a slot for a task, we do
cannot fail to deliver the task.

In this patch, we rearrange the slot building for SMP
tasks to ensure that sg mapping part (which can fail)
happens before we allocate the slot in the DQ.

Signed-off-by: Xiang Chen 
Signed-off-by: John Garry 
---
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 56 ++
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 33 ++--
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 35 ++---
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 33 ++--
 4 files changed, 51 insertions(+), 106 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index ff5b8d7..0ce7c71 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -319,7 +319,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct 
hisi_sas_dq
struct hisi_sas_cmd_hdr *cmd_hdr_base;
struct asd_sas_port *sas_port = device->port;
struct device *dev = hisi_hba->dev;
-   int dlvry_queue_slot, dlvry_queue, n_elem = 0, rc, slot_idx;
+   int dlvry_queue_slot, dlvry_queue, rc, slot_idx;
+   int  n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
unsigned long flags;
 
if (!sas_port) {
@@ -358,6 +359,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct 
hisi_sas_dq
}
 
if (!sas_protocol_ata(task->task_proto)) {
+   unsigned int req_len, resp_len;
+
if (task->num_scatter) {
n_elem = dma_map_sg(dev, task->scatter,
task->num_scatter, task->data_dir);
@@ -365,6 +368,29 @@ static int hisi_sas_task_prep(struct sas_task *task, 
struct hisi_sas_dq
rc = -ENOMEM;
goto prep_out;
}
+   } else if (task->task_proto & SAS_PROTOCOL_SMP) {
+   n_elem_req = dma_map_sg(dev, >smp_task.smp_req,
+   1, DMA_TO_DEVICE);
+   if (!n_elem_req) {
+   rc = -ENOMEM;
+   goto prep_out;
+   }
+   req_len = sg_dma_len(>smp_task.smp_req);
+   if (req_len & 0x3) {
+   rc = -EINVAL;
+   goto err_out_dma_unmap;
+   }
+   n_elem_resp = dma_map_sg(dev, >smp_task.smp_resp,
+1, DMA_FROM_DEVICE);
+   if (!n_elem_req) {
+   rc = -ENOMEM;
+   goto err_out_dma_unmap;
+   }
+   resp_len = sg_dma_len(>smp_task.smp_resp);
+   if (resp_len & 0x3) {
+   rc = -EINVAL;
+   goto err_out_dma_unmap;
+   }
}
} else
n_elem = task->num_scatter;
@@ -375,11 +401,9 @@ static int hisi_sas_task_prep(struct sas_task *task, 
struct hisi_sas_dq
device);
else
rc = hisi_sas_slot_index_alloc(hisi_hba, _idx);
-   if (rc) {
-   spin_unlock_irqrestore(_hba->lock, flags);
-   goto err_out;
-   }
spin_unlock_irqrestore(_hba->lock, flags);
+   if (rc)
+   goto err_out_dma_unmap;
 
rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
if (rc)
@@ -458,14 +482,22 @@ static int hisi_sas_task_prep(struct sas_task *task, 
struct hisi_sas_dq
spin_lock_irqsave(_hba->lock, flags);
hisi_sas_slot_index_free(hisi_hba, slot_idx);
spin_unlock_irqrestore(_hba->lock, flags);
-err_out:
-   dev_err(dev, "task prep: failed[%d]!\n", rc);
-   if (!sas_protocol_ata(task->task_proto))
-   if (n_elem)
-   dma_unmap_sg(dev, task->scatter,
-task->num_scatter,
-task->data_dir);
+err_out_dma_unmap:
+   if (!sas_protocol_ata(task->task_proto)) {
+   if (task->num_scatter) {
+   dma_unmap_sg(dev, task->scatter, task->num_scatter,
+task->data_dir);
+   } else if (task->task_proto & SAS_PROTOCOL_SMP) {
+   if (n_elem_req)
+

Re: [PATCH v6 13/13] Documentation: clarify firmware_class provenance and why we can't rename the module

2018-05-09 Thread Mauro Carvalho Chehab 
Em Tue,  8 May 2018 11:12:47 -0700
"Luis R. Rodriguez"  escreveu:

> Clarify the provenance of the firmware loader firmware_class module name
> and why we cannot rename the module in the future.
> 
> Signed-off-by: Luis R. Rodriguez 
> ---
>  .../driver-api/firmware/fallback-mechanisms.rst  | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst 
> b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> index a39323ef7d29..a8047be4a96e 100644
> --- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
> +++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> @@ -72,9 +72,12 @@ the firmware requested, and establishes it in the device 
> hierarchy by
>  associating the device used to make the request as the device's parent.
>  The sysfs directory's file attributes are defined and controlled through
>  the new device's class (firmware_class) and group (fw_dev_attr_groups).
> -This is actually where the original firmware_class.c file name comes from,
> -as originally the only firmware loading mechanism available was the
> -mechanism we now use as a fallback mechanism.
> +This is actually where the original firmware_class module name came from,
> +given that originally the only firmware loading mechanism available was the
> +mechanism we now use as a fallback mechanism, which which registers a
> +struct class firmware_class. Because the attributes exposed are part of the
> +module name, the module name firmware_class cannot be renamed in the future, 
> to
> +ensure backward compatibilty with old userspace.

Ah, now the explanation makes a lot more sense to me :-)

Reviewed-by: Mauro Carvalho Chehab 

>  
>  To load firmware using the sysfs interface we expose a loading indicator,
>  and a file upload firmware into:



Thanks,
Mauro

Re: [PATCH 0/3] scsi: arcmsr: Add driver parameter cmd_timeout for scsi command timeout setting

2018-05-09 Thread Steffen Maier 


On 05/08/2018 08:43 AM, Ching Huang wrote:

On Tue, 2018-05-08 at 14:32 +0800, Ching Huang wrote:

On Tue, 2018-05-08 at 01:41 -0400, Martin K. Petersen wrote:

Hello Ching,


1. Add driver parameter cmd_timeout, default value is ARCMSR_DEFAULT_TIMEOUT.
2. Add slave_configure callback function to set device command timeout value.
3. Update driver version to v1.40.00.06-20180504.


I am not so keen on arcmsr overriding the timeout set by the admin or
application.

Also, instead of introducing this module parameter, why not simply ask
the user to change rq_timeout?


This timeout setting only after device has been inquiry successfully.
Of course, user can set timeout value to /sys/block/sdX/device/timeout.
But user does not like to set this value once command timeout occurred.
They rather like timeout never happen.


This timeout setting apply to all devices, its better than user has to
set one bye one for each device.


Udev rules?

Something roughly like bottom of:
https://www.ibm.com/support/knowledgecenter/ST3FR7_8.1.2/com.ibm.storwize.v7000.812.doc/svc_linux_settings.html
or better doing the assignment with udev builtins (fix the syntax error 
with model):

https://www.ibm.com/support/knowledgecenter/ST3FR7_8.1.2/com.ibm.storwize.v7000.812.doc/svc_zs_statechange_3fgeri.html

--
Mit freundlichen Grüßen / Kind regards
Steffen Maier

Linux on z Systems Development

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294