date:20171207

Re: [PATCH] libsas: flush pending destruct work in sas_unregister_domain_devices()

2017-12-07 Thread Jason Yan



On 2017/12/8 6:57, Cong Wang wrote:

On Thu, Dec 7, 2017 at 5:37 AM, John Garry  wrote:

On 28/11/2017 17:04, Cong Wang wrote:


I don't understand, the only caller of sas_unregister_domain_devices()
is sas_deform_port().



And sas_deform_port() may be called from another worker on the same queue,
right? As in sas_phye_loss_of_signal()->sas_deform_port()


Oh, good catch! I didn't notice this subtle call path.

Do you have any better idea to fix this? We saw this on 4.9 too.



We have sent a patchset to fix this and to enhance libsas hotplug.
Please refer to https://lkml.org/lkml/2017/9/6/142

And I'm going to send a new version soon.

Jason



The device destruct takes place in a separate worker from which
sas_deform_port() is called, but the same queue. So we have this queued
destruct happen after the port is fully deformed -> hence the WARN.

I guess you only tested your patch on disks attached through an expander.


I have very limited scsi hardware, so my testing is limited too.

.

Re: [PATCH] scsi_debug: Add support for injecting SCSI_MLQUEUE_HOST_BUSY

2017-12-07 Thread Martin K. Petersen


Bart,

> Although it is important to be able to trigger the code in the SCSI
> core for SCSI_MLQUEUE_HOST_BUSY handling, currently it is
> nontrivial to trigger that code. Hence this patch that adds a new
> error injection option to the scsi_debug driver for making the
> .queue_rq() implementation of this driver return
> SCSI_MLQUEUE_HOST_BUSY.

Applied to 4.16/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v2 1/3] scsi: Fix a scsi_show_rq() NULL pointer dereference

2017-12-07 Thread Martin K. Petersen


Ming,

> As I explained in [1], the use-after-free is inevitable no matter if
> clearing 'SCpnt->cmnd' before mempool_free() in sd_uninit_command() or
> not, so we need to comment the fact that cdb may point to garbage
> data, and this function(especially __scsi_format_command() has to
> survive that, so that people won't be surprised when kasan complains
> use-after-free, and guys will be careful when they try to change the
> code in future.

Longer term we really need to get rid of the separate CDB allocation. It
was a necessary evil when I did it. And not much of a concern since I
did not expect anybody sane to use Type 2 (it's designed for use inside
disk arrays).

However, I keep hearing about people using Type 2 drives. Some vendors
source drives formatted that way and use the same SKU for arrays and
standalone servers.

So we should really look into making it possible for a queue to have a
bigger than 16-byte built-in CDB. For Type 2 devices, 32-byte reads and
writes are a prerequisite. So it would be nice to be able to switch a
queue to a larger allocation post creation (we won't know the type until
after READ CAPACITY(16) has been sent).

Last I looked at this it was not entirely trivial given how we tag
things on to the end. But that really is my preferred fix.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi_debug: Add support for injecting SCSI_MLQUEUE_HOST_BUSY

2017-12-07 Thread Douglas Gilbert


On 2017-12-07 09:12 PM, Martin K. Petersen wrote:


Bart,


Although it is important to be able to trigger the code in the SCSI
core for SCSI_MLQUEUE_HOST_BUSY handling, currently it is nontrivial
to trigger that code. Hence this patch that adds a new error injection
option to the scsi_debug driver for making the .queue_rq()
implementation of this driver return SCSI_MLQUEUE_HOST_BUSY.


This looks good to me. Doug?


Acked-by: Douglas Gilbert

Re: [PATCH try #2] scsi_devinfo: apply to HP-rebranded the same flags as Hitachi

2017-12-07 Thread Martin K. Petersen


Xose,

> 627511e3e modified some Hitachi entries:
>
> Four models, OPEN-/DF400/DF500/DISK-SUBSYSTEM, can handle REPORT_LUN,
> and the BLIST_REPORTLUN2 flag needs to be set. And DF600 doesn't require
> any flags because it returns ANSI 03h (SPC).
> ~~~
>
> The same should have been done also for HP counterparts.

Applied to 4.16/scsi-queue, thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: scsi_devinfo: replace "Dell PV 650F" with "EMC CLARiiON"

2017-12-07 Thread Martin K. Petersen


Xose,

> The Dell PV650F is a re-branded CLARiiON FC5700.
> And DGC/RAID,DISK identifies all CLARiiON family.

Applied to 4.16/scsi-queue. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi_dh: add new rdac devices

2017-12-07 Thread Martin K. Petersen


Xose,

> Add IBM 3542 and 3552, arrays: FAStT200 and FAStT500.
> Add full STK OPENstorage family, arrays: 9176, D173, D178, D210, D220, D240 
> and D280.
> Add STK BladeCtlr family, arrays: B210, B220, B240 and B280.
>
> These changes were done in multipath-tools time ago.

Applied to 4.16/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi_devinfo: apply to HP XP the same flags as Hitachi VSP

2017-12-07 Thread Martin K. Petersen


Xose,

> 56f3d383f modified some Hitachi entries:
>
>HITACHI is always supporting VPD pages, even though it's claiming to
>support SCSI Revision 3 only.
> ~~~
>
> The same should have been done also for HP-rebranded.

Applied to 4.16/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: pmcraid: use correct size unit when calling find_first_zero_bit()

2017-12-07 Thread Martin K. Petersen


Niklas,

> find_first_zero_bit()'s parameter 'size' is defined in bits,
> not in bytes.

Applied to 4.16/scsi-queue, thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: fusion: clean up some indentations

2017-12-07 Thread Martin K. Petersen


Colin,

> There are several places where the source is not indented correctly
> with either too many or too few levels of intentation. Fix these.

Applied to 4.16/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] Remove scsi_dh_remove_device()

2017-12-07 Thread Martin K. Petersen


Bart,

> Remove this function since it has an empty body.

Applied to 4.16/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi_debug: Add support for injecting SCSI_MLQUEUE_HOST_BUSY

2017-12-07 Thread Martin K. Petersen


Bart,

> Although it is important to be able to trigger the code in the SCSI
> core for SCSI_MLQUEUE_HOST_BUSY handling, currently it is nontrivial
> to trigger that code. Hence this patch that adds a new error injection
> option to the scsi_debug driver for making the .queue_rq()
> implementation of this driver return SCSI_MLQUEUE_HOST_BUSY.

This looks good to me. Doug?

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] Unexport scsi_initialize_rq()

2017-12-07 Thread Martin K. Petersen


Bart,

> Commit 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for
> SMP passthrough") removed the only call to scsi_initialize_rq() from
> outside the SCSI core. Hence unexport scsi_initialize_rq().

Applied to 4.16/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v2 1/3] scsi: Fix a scsi_show_rq() NULL pointer dereference

2017-12-07 Thread Ming Lei

On Tue, Dec 05, 2017 at 04:57:51PM -0800, Bart Van Assche wrote:
> Avoid that scsi_show_rq() triggers a NULL pointer dereference if
> called after sd_uninit_command(). Swap the NULL pointer assignment
> and the mempool_free() call in sd_uninit_command() to make it less
> likely that scsi_show_rq() triggers a use-after-free. Note: even
> with these changes scsi_show_rq() can trigger a use-after-free but
> that's a lesser evil than e.g. suppressing debug information for
> T10-PI commands completely. This patch fixes the following oops:
> 
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: scsi_format_opcode_name+0x1a/0x1c0
> CPU: 1 PID: 1881 Comm: cat Not tainted 4.14.0-rc2.blk_mq_io_hang+ #516
> Call Trace:
>  __scsi_format_command+0x27/0xc0
>  scsi_show_rq+0x5c/0xc0
>  __blk_mq_debugfs_rq_show+0x116/0x130
>  blk_mq_debugfs_rq_show+0xe/0x10
>  seq_read+0xfe/0x3b0
>  full_proxy_read+0x54/0x90
>  __vfs_read+0x37/0x160
>  vfs_read+0x96/0x130
>  SyS_read+0x55/0xc0
>  entry_SYSCALL_64_fastpath+0x1a/0xa5
> 
> Fixes: 0eebd005dd07 ("scsi: Implement blk_mq_ops.show_rq()")
> Reported-by: Ming Lei 
> Signed-off-by: Bart Van Assche 
> Cc: James E.J. Bottomley 
> Cc: Martin K. Petersen 
> Cc: Ming Lei 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: Johannes Thumshirn 
> Cc: sta...@vger.kernel.org
> ---
>  drivers/scsi/scsi_debugfs.c | 6 --
>  drivers/scsi/sd.c   | 4 +++-
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_debugfs.c b/drivers/scsi/scsi_debugfs.c
> index 01f08c03f2c1..c3765d29fd3f 100644
> --- a/drivers/scsi/scsi_debugfs.c
> +++ b/drivers/scsi/scsi_debugfs.c
> @@ -8,9 +8,11 @@ void scsi_show_rq(struct seq_file *m, struct request *rq)
>  {
>   struct scsi_cmnd *cmd = container_of(scsi_req(rq), typeof(*cmd), req);
>   int msecs = jiffies_to_msecs(jiffies - cmd->jiffies_at_alloc);
> - char buf[80];
> + const u8 *const cdb = READ_ONCE(cmd->cmnd);
> + char buf[80] = "(?)";
>  
> - __scsi_format_command(buf, sizeof(buf), cmd->cmnd, cmd->cmd_len);
> + if (cdb)
> + __scsi_format_command(buf, sizeof(buf), cdb, cmd->cmd_len);
>   seq_printf(m, ", .cmd=%s, .retries=%d, allocated %d.%03d s ago", buf,
>  cmd->retries, msecs / 1000, msecs % 1000);
>  }

As I explained in [1], the use-after-free is inevitable no matter if
clearing 'SCpnt->cmnd' before mempool_free() in sd_uninit_command() or not,
so we need to comment the fact that cdb may point to garbage data, and this
function(especially __scsi_format_command() has to survive that, so that
people won't be surprised when kasan complains use-after-free, and guys will
be careful when they try to change the code in future.

Once this comment is added, with or without clearing 'SCpnt->cmnd' before
mempool_free(), I am fine with this patch.

[1] https://marc.info/?l=linux-block=151252302112512=2

> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index d175c5c5ccf8..d841743b2107 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1284,6 +1284,7 @@ static int sd_init_command(struct scsi_cmnd *cmd)
>  static void sd_uninit_command(struct scsi_cmnd *SCpnt)
>  {
>   struct request *rq = SCpnt->request;
> + u8 *cmnd;
>  
>   if (SCpnt->flags & SCMD_ZONE_WRITE_LOCK)
>   sd_zbc_write_unlock_zone(SCpnt);
> @@ -1292,9 +1293,10 @@ static void sd_uninit_command(struct scsi_cmnd *SCpnt)
>   __free_page(rq->special_vec.bv_page);
>  
>   if (SCpnt->cmnd != scsi_req(rq)->cmd) {
> - mempool_free(SCpnt->cmnd, sd_cdb_pool);
> + cmnd = SCpnt->cmnd;
>   SCpnt->cmnd = NULL;
>   SCpnt->cmd_len = 0;
> + mempool_free(cmnd, sd_cdb_pool);
>   }
>  }
>  
> -- 
> 2.15.0
> 

-- 
Ming

答复: [PATCH v6 1/5] scsi: ufs: add Hisilicon ufs driver code

2017-12-07 Thread liwei (CM)

Hi,Philippe,

Thank you for your suggestion, and I'll consider that next patch.

-邮件原件-
发件人: Philippe Ombredanne [mailto:pombreda...@nexb.com] 
发送时间: 2017年12月7日 18:34
收件人: liwei (CM)
抄送: Rob Herring; Mark Rutland; xuwei (O); Catalin Marinas; Will Deacon; 
vinholika...@gmail.com; James E.J. Bottomley; Martin K. Petersen; 
khil...@baylibre.com; Arnd Bergmann; gregory.clem...@free-electrons.com; Thomas 
Petazzoni; yamada.masah...@socionext.com; riku.voi...@linaro.org; 
tred...@nvidia.com; k...@kernel.org; e...@anholt.net; open list:OPEN FIRMWARE 
AND FLATTENED DEVICE TREE BINDINGS; LKML; moderated list:ARM/FREESCALE IMX / 
MXC ARM ARCHITECTURE; linux-scsi@vger.kernel.org; zangleigang; Gengjianfeng; 
guodong...@linaro.org; zhangfei@linaro.org; Fengbaopeng (kevin, Kirin 
Solution Dept)
主题: Re: [PATCH v6 1/5] scsi: ufs: add Hisilicon ufs driver code

Dear Li,

On Thu, Dec 7, 2017 at 11:20 AM, Li Wei  wrote:
> add Hisilicon ufs driver code.
>
> Signed-off-by: Li Wei 
> Signed-off-by: Geng Jianfeng 
> Signed-off-by: Zang Leigang 
> Signed-off-by: Yu Jianfeng 
[]

> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-hisi.c
> @@ -0,0 +1,624 @@
> +/*
> + *
> + * HiSilicon Hi UFS Driver
> + *
> + * Copyright (c) 2016-2017 Linaro Ltd.
> + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or 
> +modify
> + * it under the terms of the GNU General Public License as published 
> +by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */

Would you consider using the new SPDX license ids instead?
Check Thomas doc patches for instructions.
This would be great and while you are it may be this could be adopted for all 
HiSilicon and Huawei past, present and future contributions?
Thank you for your kind consideration!

--
Cordially
Philippe Ombredanne

Re: [PATCH 0/3] SCSI device blacklist handling improvements

2017-12-07 Thread Martin K. Petersen


Bart,

> Are you perhaps referring to the five __force casts? If so, do you
> have a suggestion for avoiding that sparse reports false positive
> warnings on the conversions between int and blist_flags_t? The only
> approach I can think of to reduce the number of __force casts is to
> embed these __force casts into two helper functions - one for the
> conversion from int to blist_flags_t and one for the conversion the
> other way around.

blist_flags_t is an unsigned int, you are forcing it to u32 two
places. That's a problem waiting to happen next time we add a blacklist
flag.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] libsas: flush pending destruct work in sas_unregister_domain_devices()

2017-12-07 Thread Cong Wang

On Thu, Dec 7, 2017 at 4:40 PM, Cong Wang  wrote:
> On Thu, Dec 7, 2017 at 2:57 PM, Cong Wang  wrote:
>> On Thu, Dec 7, 2017 at 5:37 AM, John Garry  wrote:
>>> On 28/11/2017 17:04, Cong Wang wrote:

 I don't understand, the only caller of sas_unregister_domain_devices()
 is sas_deform_port().

>>>
>>> And sas_deform_port() may be called from another worker on the same queue,
>>> right? As in sas_phye_loss_of_signal()->sas_deform_port()
>>
>> Oh, good catch! I didn't notice this subtle call path.
>>
>> Do you have any better idea to fix this? We saw this on 4.9 too.
>>
>
> I think we can just cancel the destruct work before calling
> sas_port_delete(). This should work even if it is called in
> another work.
>

This assumes sas_port_delete() could release resources recursively
in the hierarchy, this is true for sysfs but perhaps not true for other
resources...

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

2017-12-07 Thread Martin K. Petersen


> I'm perfectly OK with taking it through the SCSI tree. Probably the
> path of least resistance.

Applied to 4.16/scsi-queue and rebased so it sits before Bart's patch.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: bfa: fix type conversion warning

2017-12-07 Thread Martin K. Petersen


Arnd,

> This changes the code back to shost_priv() once more, but encapsulates
> it in an inline function to document the rather unusual way of using
> the private data only as a pointer to the previously allocated
> structure.

Applied to 4.15/scsi-fixes, thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] SCSI: run queue if SCSI device queue isn't ready and queue is idle

2017-12-07 Thread Martin K. Petersen


Ming,

> Jens, Martin, would any of you mind making this patch in V4.15? Since
> it fixes real use cases and this way is exact what we do before
> 0df21c86bdbf("scsi: implement .get_budget and .put_budget for blk-mq").

Applied to 4.15/scsi-fixes, thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] SCSI: run queue if SCSI device queue isn't ready and queue is idle

2017-12-07 Thread Ming Lei

On Thu, Dec 07, 2017 at 09:06:58PM +, Bart Van Assche wrote:
> On Wed, 2017-12-06 at 00:28 +0800, Ming Lei wrote:
> > On Tue, Dec 05, 2017 at 04:08:20PM +, Bart Van Assche wrote:
> > > On Tue, 2017-12-05 at 15:52 +0800, Ming Lei wrote:
> > > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > > > index db9556662e27..1816dd8259b3 100644
> > > > --- a/drivers/scsi/scsi_lib.c
> > > > +++ b/drivers/scsi/scsi_lib.c
> > > > @@ -1967,6 +1967,8 @@ static bool scsi_mq_get_budget(struct 
> > > > blk_mq_hw_ctx *hctx)
> > > >  out_put_device:
> > > > put_device(>sdev_gendev);
> > > >  out:
> > > > +   if (atomic_read(>device_busy) == 0 && 
> > > > !scsi_device_blocked(sdev))
> > > > +   blk_mq_delay_run_hw_queue(hctx, SCSI_QUEUE_DELAY);
> > > > return false;
> > > >  }
> > > 
> > > This cannot work since multiple threads can call scsi_mq_get_budget()
> > 
> > That is exactly the way we are handling these cases before 
> > 0df21c86bdbf(scsi:
> > implement .get_budget and .put_budget for blk-mq), so if it can't work,
> > that is not fault of commit 0df21c86bdbf.
> > 
> > > concurrently and hence it can happen that none of them sees
> > > atomic_read(>device_busy) == 0. BTW, the above patch is something I
> > 
> > If there is concurrent .get_budget(), one of them will see the counter
> > becoming zero finally because each sdev->device_busy is inc/dec
> > atomically. Or scsi_dev_queue_ready() return true.
> > 
> > Anyway, we need this patch to avoid possible regression. If you think
> > there are bugs in blk-mq RESTART, just root cause and and fix it.
> 
> Hello Ming,
> 
> When I looked at the patch at the start of this thread for the first time I
> got frustrated because I didn't see how this patch could fix the queue stall
> I ran into myself. Today I started realizing that what Holger reported is
> probably another issue than what I ran into myself. Since this patch by
> itself looks now useful to me:
> 
> Reviewed-by: Bart Van Assche 

Hi Bart,

Thanks for your Review!

> 
> BTW, do you think the patch at the start of this thread also fixes the issue
> that resulted in commit 826a70a08b12 ("SCSI: don't get target/host busy_count
> in scsi_mq_get_budget()")? Do you think we still need that patch?

Yeah, once the patch in this thread is in, IO hang shouldn't happen any more
even without 826a70a08b12.

But that commit is still the correct thing to do, since we let blk-mq's sbitmap
queue respect per-host queue depth, which way is much efficient than the
simple atomic operations used in scsi_host_queue_ready(). So I think that commit
is still useful.

When I was figuring out patch of 826a70a08b12, I just ignore the .get_budget() 
for
requests from hctx->dispatch_list, because we don't need to deal with queue idle
when .get_budget() is called from both blk_mq_do_dispatch_sched() and 
blk_mq_do_dispatch_ctx().

Thanks,
Ming

Re: [PATCH] libsas: flush pending destruct work in sas_unregister_domain_devices()

2017-12-07 Thread Cong Wang

On Thu, Dec 7, 2017 at 2:57 PM, Cong Wang  wrote:
> On Thu, Dec 7, 2017 at 5:37 AM, John Garry  wrote:
>> On 28/11/2017 17:04, Cong Wang wrote:
>>>
>>> I don't understand, the only caller of sas_unregister_domain_devices()
>>> is sas_deform_port().
>>>
>>
>> And sas_deform_port() may be called from another worker on the same queue,
>> right? As in sas_phye_loss_of_signal()->sas_deform_port()
>
> Oh, good catch! I didn't notice this subtle call path.
>
> Do you have any better idea to fix this? We saw this on 4.9 too.
>

I think we can just cancel the destruct work before calling
sas_port_delete(). This should work even if it is called in
another work.

So does the attached (untested) patch make any sense now?
diff --git a/drivers/scsi/libsas/sas_discover.c 
b/drivers/scsi/libsas/sas_discover.c
index 60de66252fa2..bc512d65e2ca 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -565,6 +565,21 @@ int sas_discover_event(struct asd_sas_port *port, enum 
discover_event ev)
return 0;
 }
 
+static void sas_cancel_work(struct sas_work *sw)
+{
+   cancel_work_sync(>work);
+}
+
+void sas_cancel_event(struct asd_sas_port *port, enum discover_event ev)
+{
+   struct sas_discovery *disc;
+
+   if (!port)
+   return;
+   disc = >disc;
+   sas_cancel_work(>disc_work[ev].work);
+}
+
 /**
  * sas_init_disc -- initialize the discovery struct in the port
  * @port: pointer to struct port
diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c
index d3c5297c6c89..89e37640e26c 100644
--- a/drivers/scsi/libsas/sas_port.c
+++ b/drivers/scsi/libsas/sas_port.c
@@ -219,6 +219,7 @@ void sas_deform_port(struct asd_sas_phy *phy, int gone)
 
if (port->num_phys == 1) {
sas_unregister_domain_devices(port, gone);
+   sas_cancel_event(port, DISCE_DESTRUCT);
sas_port_delete(port->port);
port->port = NULL;
} else {
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 6df6fe0c2198..5b8a7fadd9b4 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -680,6 +680,7 @@ int  sas_ex_revalidate_domain(struct domain_device *);
 void sas_unregister_domain_devices(struct asd_sas_port *port, int gone);
 void sas_init_disc(struct sas_discovery *disc, struct asd_sas_port *);
 int  sas_discover_event(struct asd_sas_port *, enum discover_event ev);
+void sas_cancel_event(struct asd_sas_port *port, enum discover_event ev);
 
 int  sas_discover_sata(struct domain_device *);
 int  sas_discover_end_dev(struct domain_device *);

Re: [PATCH] SCSI: run queue if SCSI device queue isn't ready and queue is idle

2017-12-07 Thread Ming Lei

On Thu, Dec 07, 2017 at 09:11:54PM +, Bart Van Assche wrote:
> On Thu, 2017-12-07 at 09:31 +0800, Ming Lei wrote:
> > But if you always call blk_mq_sched_mark_restart_hctx() before a new
> > dispatch, that may affect performance on NVMe which may never trigger
> > BLK_STS_RESOURCE.
> 
> Hmm ... only the SCSI core implements .get_budget() and .put_budget() and
> I proposed to insert a blk_mq_sched_mark_restart_hctx() call under "if
> (q->mq_ops->get_budget)". In other words, I proposed to insert a
> blk_mq_sched_mark_restart_hctx() call in a code path that is never triggered
> by the NVMe driver. So I don't see how the change I proposed could affect
> the performance of the NVMe driver?

You only add the check on none scheduler, right?

But this race isn't related with scheduler, that means it can't fix the
race with other schedulers.

I have test case to trigger this issue on both none and mq-deadline, and
my patch fixes them all.

Thanks,
Ming

[PATCH] qla2xxx: Suppress gcc 7 fall-through warnings

2017-12-07 Thread Bart Van Assche

Avoid that building with gcc 7 and W=1 triggers warnings similar
to the following:

drivers/scsi/qla2xxx/qla_isr.c:1189:27: warning: this statement may fall 
through [-Wimplicit-fallthrough=]

Signed-off-by: Bart Van Assche 
Cc: Himanshu Madhani 
Cc: Quinn Tran 
Cc: Hannes Reinecke 
---
 drivers/scsi/qla2xxx/qla_gs.c | 2 +-
 drivers/scsi/qla2xxx/qla_isr.c| 5 +++--
 drivers/scsi/qla2xxx/qla_sup.c| 1 +
 drivers/scsi/qla2xxx/qla_target.c | 3 ++-
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index 7d715e58901f..07fe17a986b0 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -177,7 +177,7 @@ qla2x00_chk_ms_status(scsi_qla_host_t *vha, ms_iocb_entry_t 
*ms_pkt,
break;
case CS_TIMEOUT:
rval = QLA_FUNCTION_TIMEOUT;
-   /* drop through */
+   /* fall through */
default:
ql_dbg(ql_dbg_disc, vha, 0x2033,
"%s failed, completion status (%x) on port_id: "
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 85382387a52b..a55bfaa790a3 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1202,6 +1202,7 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que 
*rsp, uint16_t *mb)
qla2xxx_wake_dpc(vha);
}
}
+   /* fall through */
case MBA_IDC_COMPLETE:
if (ha->notify_lb_portup_comp && !vha->vp_idx)
complete(>lb_portup_comp);
@@ -1769,7 +1770,7 @@ qla24xx_logio_entry(scsi_qla_host_t *vha, struct req_que 
*req,
set_bit(ISP_ABORT_NEEDED, >dpc_flags);
qla2xxx_wake_dpc(vha);
}
-   /* drop through */
+   /* fall through */
default:
data[0] = MBS_COMMAND_ERROR;
break;
@@ -2967,9 +2968,9 @@ void qla24xx_process_response_queue(struct scsi_qla_host 
*vha,
(response_t *)pkt);
break;
} else {
-   /* drop through */
qlt_24xx_process_atio_queue(vha, 1);
}
+   /* fall through */
case ABTS_RESP_24XX:
case CTIO_TYPE7:
case CTIO_CRC2:
diff --git a/drivers/scsi/qla2xxx/qla_sup.c b/drivers/scsi/qla2xxx/qla_sup.c
index b4336e0cd85f..d2db86ea06b2 100644
--- a/drivers/scsi/qla2xxx/qla_sup.c
+++ b/drivers/scsi/qla2xxx/qla_sup.c
@@ -2461,6 +2461,7 @@ qla2x00_write_optrom_data(struct scsi_qla_host *vha, 
uint8_t *buf,
sec_mask = 0x1e000;
break;
}
+   /* fall through */
default:
/* Default to 16 kb sector size. */
rest_addr = 0x3fff;
diff --git a/drivers/scsi/qla2xxx/qla_target.c 
b/drivers/scsi/qla2xxx/qla_target.c
index a23107b1ec06..067bcc57a9ad 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -450,6 +450,7 @@ void qlt_response_pkt_all_vps(struct scsi_qla_host *vha,
ql_dbg(ql_dbg_tgt, vha, 0xe073,
"qla_target(%d):%s: CRC2 Response pkt\n",
vha->vp_idx, __func__);
+   /* fall through */
case CTIO_TYPE7:
{
struct ctio7_from_24xx *entry = (struct ctio7_from_24xx *)pkt;
@@ -4889,7 +4890,7 @@ static int qlt_24xx_handle_els(struct scsi_qla_host *vha,
res = 1;
break;
}
-   /* drop through */
+   /* fall through */
case ELS_LOGO:
case ELS_PRLO:
spin_lock_irqsave(>tgt.sess_lock, flags);
-- 
2.15.0

[PATCH] Remove scsi_dh_remove_device()

2017-12-07 Thread Bart Van Assche

Remove this function since it has an empty body.

Signed-off-by: Bart Van Assche 
Cc: Christoph Hellwig 
Cc: Hannes Reinecke 
Cc: Johannes Thumshirn 
---
 drivers/scsi/scsi_priv.h  | 1 -
 drivers/scsi/scsi_sysfs.c | 3 ---
 2 files changed, 4 deletions(-)

diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 61024db5953d..99f1db5e467e 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -186,7 +186,6 @@ void scsi_dh_release_device(struct scsi_device *sdev);
 static inline void scsi_dh_add_device(struct scsi_device *sdev) { }
 static inline void scsi_dh_release_device(struct scsi_device *sdev) { }
 #endif
-static inline void scsi_dh_remove_device(struct scsi_device *sdev) { }
 
 /* 
  * internal scsi timeout functions: for use by mid-layer and transport
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 6ee964643531..1bce5c7305ff 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1277,7 +1277,6 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
if (error) {
sdev_printk(KERN_INFO, sdev,
"failed to add device: %d\n", error);
-   scsi_dh_remove_device(sdev);
return error;
}
 
@@ -1286,7 +1285,6 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
if (error) {
sdev_printk(KERN_INFO, sdev,
"failed to add class device: %d\n", error);
-   scsi_dh_remove_device(sdev);
device_del(>sdev_gendev);
return error;
}
@@ -1353,7 +1351,6 @@ void __scsi_remove_device(struct scsi_device *sdev)
bsg_unregister_queue(sdev->request_queue);
device_unregister(>sdev_dev);
transport_remove_device(dev);
-   scsi_dh_remove_device(sdev);
device_del(dev);
} else
put_device(>sdev_dev);
-- 
2.15.0

[PATCH] Unexport scsi_initialize_rq()

2017-12-07 Thread Bart Van Assche

Commit 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib
for SMP passthrough") removed the only call to scsi_initialize_rq()
from outside the SCSI core. Hence unexport scsi_initialize_rq().

Signed-off-by: Bart Van Assche 
Cc: Christoph Hellwig 
Cc: Hannes Reinecke 
Cc: Johannes Thumshirn 
---
 drivers/scsi/scsi_lib.c  | 3 +--
 include/scsi/scsi_cmnd.h | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1827956b33c9..c3fc4353af3c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1145,7 +1145,7 @@ EXPORT_SYMBOL(scsi_init_io);
  * Called from inside blk_get_request() for pass-through requests and from
  * inside scsi_init_command() for filesystem requests.
  */
-void scsi_initialize_rq(struct request *rq)
+static void scsi_initialize_rq(struct request *rq)
 {
struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
 
@@ -1153,7 +1153,6 @@ void scsi_initialize_rq(struct request *rq)
cmd->jiffies_at_alloc = jiffies;
cmd->retries = 0;
 }
-EXPORT_SYMBOL(scsi_initialize_rq);
 
 /* Add a command to the list used by the aacraid and dpt_i2o drivers */
 void scsi_add_cmd_to_list(struct scsi_cmnd *cmd)
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 7fb57e905526..949a016dd7fa 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -171,7 +171,6 @@ extern void *scsi_kmap_atomic_sg(struct scatterlist *sg, 
int sg_count,
 extern void scsi_kunmap_atomic_sg(void *virt);
 
 extern int scsi_init_io(struct scsi_cmnd *cmd);
-extern void scsi_initialize_rq(struct request *rq);
 
 extern int scsi_dma_map(struct scsi_cmnd *cmd);
 extern void scsi_dma_unmap(struct scsi_cmnd *cmd);
-- 
2.15.0

Re: [PATCH v6 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

2017-12-07 Thread Rob Herring

On Thu, Dec 07, 2017 at 06:20:23PM +0800, Li Wei wrote:
> add ufs node document for Hisilicon.
> 
> Signed-off-by: Li Wei 
> ---

Version history?

>  Documentation/devicetree/bindings/ufs/ufs-hisi.txt | 38 
> ++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ufs/ufs-hisi.txt
> 
> diff --git a/Documentation/devicetree/bindings/ufs/ufs-hisi.txt 
> b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
> new file mode 100644
> index ..73e10698960e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
> @@ -0,0 +1,38 @@
> +* Hisilicon Universal Flash Storage (UFS) Host Controller
> +
> +UFS nodes are defined to describe on-chip UFS hardware macro.
> +Each UFS Host Controller should have its own node.
> +
> +Required properties:
> +- compatible: compatible list, contains one of the following -
> + "hisilicon,hi3660-ufs", "jedec,ufs-1.1" 
> for hisi ufs
> + host controller present on Hi36xx 
> chipset.
> +- reg   : should contain UFS register address space & UFS SYS 
> CTRL register address,
> +- interrupt-parent  : interrupt device
> +- interrupts: interrupt number
> +- clocks : List of phandle and clock specifier pairs
> +- clock-names   : List of clock input name strings sorted in the same
> + order as the clocks property. 
> "ref_clk", "phy_clk" is optional
> +- resets: reset node register, one reset the clk and the other 
> reset the controller
> +- reset-names   : describe reset node register
> +
> +Example:
> +
> + ufs: ufs@ff3b {
> + compatible = "hisilicon,hi3660-ufs", "jedec,ufs-1.1";
> + /* 0: HCI standard */
> + /* 1: UFS SYS CTRL */
> + reg = <0x0 0xff3b 0x0 0x1000>,
> + <0x0 0xff3b1000 0x0 0x1000>;
> + interrupt-parent = <>;
> + interrupts = ;
> + clocks = <_ctrl HI3660_CLK_GATE_UFSIO_REF>,
> + <_ctrl HI3660_CLK_GATE_UFSPHY_CFG>;
> + clock-names = "ref_clk", "phy_clk";
> + freq-table-hz = <0 0>, <0 0>;

? Not documented.

> + /* offset: 0x84; bit: 12 */
> + /* offset: 0x84; bit: 7  */
> + resets = <_rst 0x84 12>,
> + <_rst 0x84 7>;
> + reset-names = "rst", "assert";
> + };
> -- 
> 2.15.0
>

Re: [PATCH] libsas: flush pending destruct work in sas_unregister_domain_devices()

2017-12-07 Thread Cong Wang

On Thu, Dec 7, 2017 at 5:37 AM, John Garry  wrote:
> On 28/11/2017 17:04, Cong Wang wrote:
>>
>> I don't understand, the only caller of sas_unregister_domain_devices()
>> is sas_deform_port().
>>
>
> And sas_deform_port() may be called from another worker on the same queue,
> right? As in sas_phye_loss_of_signal()->sas_deform_port()

Oh, good catch! I didn't notice this subtle call path.

Do you have any better idea to fix this? We saw this on 4.9 too.

>
> The device destruct takes place in a separate worker from which
> sas_deform_port() is called, but the same queue. So we have this queued
> destruct happen after the port is fully deformed -> hence the WARN.
>
> I guess you only tested your patch on disks attached through an expander.

I have very limited scsi hardware, so my testing is limited too.

[PATCH] scsi_debug: Add support for injecting SCSI_MLQUEUE_HOST_BUSY

2017-12-07 Thread Bart Van Assche

Although it is important to be able to trigger the code in the SCSI
core for SCSI_MLQUEUE_HOST_BUSY handling, currently it is
nontrivial to trigger that code. Hence this patch that adds a new
error injection option to the scsi_debug driver for making the
.queue_rq() implementation of this driver return
SCSI_MLQUEUE_HOST_BUSY.

Signed-off-by: Bart Van Assche 
Cc: Douglas Gilbert 
Cc: Christoph Hellwig 
Cc: Hannes Reinecke 
Cc: Johannes Thumshirn 
---
 drivers/scsi/scsi_debug.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 9833850a53df..4b87e94e6611 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -161,12 +161,14 @@ static const char *sdebug_version_date = "20160430";
 #define SDEBUG_OPT_N_WCE   0x1000
 #define SDEBUG_OPT_RESET_NOISE 0x2000
 #define SDEBUG_OPT_NO_CDB_NOISE0x4000
+#define SDEBUG_OPT_HOST_BUSY   0x8000
 #define SDEBUG_OPT_ALL_NOISE (SDEBUG_OPT_NOISE | SDEBUG_OPT_Q_NOISE | \
  SDEBUG_OPT_RESET_NOISE)
 #define SDEBUG_OPT_ALL_INJECTING (SDEBUG_OPT_RECOVERED_ERR | \
  SDEBUG_OPT_TRANSPORT_ERR | \
  SDEBUG_OPT_DIF_ERR | SDEBUG_OPT_DIX_ERR | \
- SDEBUG_OPT_SHORT_TRANSFER)
+ SDEBUG_OPT_SHORT_TRANSFER | \
+ SDEBUG_OPT_HOST_BUSY)
 /* When "every_nth" > 0 then modulo "every_nth" commands:
  *   - a missing response is simulated if SDEBUG_OPT_TIMEOUT is set
  *   - a RECOVERED_ERROR is simulated on successful read and write
@@ -282,6 +284,7 @@ struct sdebug_queued_cmd {
unsigned int inj_dif:1;
unsigned int inj_dix:1;
unsigned int inj_short:1;
+   unsigned int inj_host_busy:1;
 };
 
 struct sdebug_queue {
@@ -3995,6 +3998,7 @@ static void setup_inject(struct sdebug_queue *sqp,
sqcp->inj_dif = !!(SDEBUG_OPT_DIF_ERR & sdebug_opts);
sqcp->inj_dix = !!(SDEBUG_OPT_DIX_ERR & sdebug_opts);
sqcp->inj_short = !!(SDEBUG_OPT_SHORT_TRANSFER & sdebug_opts);
+   sqcp->inj_host_busy = !!(SDEBUG_OPT_HOST_BUSY & sdebug_opts);
 }
 
 /* Complete the processing of the thread that queued a SCSI command to this
@@ -5278,6 +5282,12 @@ static bool fake_timeout(struct scsi_cmnd *scp)
return false;
 }
 
+static bool fake_host_busy(struct scsi_cmnd *scp)
+{
+   return (sdebug_opts & SDEBUG_OPT_HOST_BUSY) &&
+   (atomic_read(_cmnd_count) % abs(sdebug_every_nth)) == 0;
+}
+
 static int scsi_debug_queuecommand(struct Scsi_Host *shost,
   struct scsi_cmnd *scp)
 {
@@ -5320,6 +5330,8 @@ static int scsi_debug_queuecommand(struct Scsi_Host 
*shost,
sdev_printk(KERN_INFO, sdp, "%s: cmd %s\n", my_name,
b);
}
+   if (fake_host_busy(scp))
+   return SCSI_MLQUEUE_HOST_BUSY;
has_wlun_rl = (sdp->lun == SCSI_W_LUN_REPORT_LUNS);
if (unlikely((sdp->lun >= sdebug_max_luns) && !has_wlun_rl))
goto err_out;
-- 
2.15.0

Re: [PATCH] drivers/scsi/qla2xxx: fix double free bug after firmware timeout

2017-12-07 Thread Max Kellermann

On 2017/12/07 21:38, "Madhani, Himanshu"  wrote:
> NACK
> 
> These calls are asynchronous calls and free should be called by
> completion.

I don't understand the NACK, and your text doesn't explain it.  It
only describes a second bug that is orthogonal to mine.

[PATCH] scsi: libiscsi: Allow sd_shutdown on bad transport

2017-12-07 Thread Rafael David Tinoco

If, for any reason, userland shuts down iscsi transport interfaces
before proper logouts - like when logging in to LUNs manually,
without logging out on server shutdown, or when automated scripts
can't umount/logout from logged LUNs - kernel will hang forever on
its sd_sync_cache() logic, after issuing the SYNCHRONIZE_CACHE cmd
to all still existent paths.

PID: 1 TASK: 8801a69b8000 CPU: 1 COMMAND: "systemd-shutdow"
 #0 [8801a69c3a30] __schedule at 8183e9ee
 #1 [8801a69c3a80] schedule at 8183f0d5
 #2 [8801a69c3a98] schedule_timeout at 81842199
 #3 [8801a69c3b40] io_schedule_timeout at 8183e604
 #4 [8801a69c3b70] wait_for_completion_io_timeout at 8183fc6c
 #5 [8801a69c3bd0] blk_execute_rq at 813cfe10
 #6 [8801a69c3c88] scsi_execute at 815c3fc7
 #7 [8801a69c3cc8] scsi_execute_req_flags at 815c60fe
 #8 [8801a69c3d30] sd_sync_cache at 815d37d7
 #9 [8801a69c3da8] sd_shutdown at 815d3c3c

This happens because iscsi_eh_cmd_timed_out(), the transport layer
timeout helper, would tell the queue timeout function (scsi_times_out)
to reset the request timer over and over, until the session state is
back to logged in state. Unfortunately, during server shutdown, this
might never happen again.

Other option would be "not to handle" the issue in the transport
layer. That would trigger the error handler logic, which would also
need the session state to be logged in again.

Best option, for such case, is to tell upper layers that the command
was handled during the transport layer error handler helper, marking
it as DID_NO_CONNECT, which will allow completion and inform about
the problem.

After the session was marked as ISCSI_STATE_FAILED, due to the first
timeout during the server shutdown phase, all subsequent cmds will
fail to be queued, allowing upper logic to fail faster.

Signed-off-by: Rafael David Tinoco 
---
 drivers/scsi/libiscsi.c | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 9c50d2d9f27c..785d1c55d152 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1696,6 +1696,15 @@ int iscsi_queuecommand(struct Scsi_Host *host, struct 
scsi_cmnd *sc)
 */
switch (session->state) {
case ISCSI_STATE_FAILED:
+   /*
+* cmds should fail during shutdown, if the session
+* state is bad, allowing completion to happen
+*/
+   if (unlikely(system_state != SYSTEM_RUNNING)) {
+   reason = FAILURE_SESSION_FAILED;
+   sc->result = DID_NO_CONNECT << 16;
+   break;
+   }
case ISCSI_STATE_IN_RECOVERY:
reason = FAILURE_SESSION_IN_RECOVERY;
sc->result = DID_IMM_RETRY << 16;
@@ -1978,6 +1987,19 @@ enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct 
scsi_cmnd *sc)
}
 
if (session->state != ISCSI_STATE_LOGGED_IN) {
+   /*
+* During shutdown, if session is prematurely disconnected,
+* recovery won't happen and there will be hung cmds. Not
+* handling cmds would trigger EH, also bad in this case.
+* Instead, handle cmd, allow completion to happen and let
+* upper layer to deal with the result.
+*/
+   if (unlikely(system_state != SYSTEM_RUNNING)) {
+   sc->result = DID_NO_CONNECT << 16;
+   ISCSI_DBG_EH(session, "sc on shutdown, handled\n");
+   rc = BLK_EH_HANDLED;
+   goto done;
+   }
/*
 * We are probably in the middle of iscsi recovery so let
 * that complete and handle the error.
@@ -2082,7 +2104,7 @@ enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct 
scsi_cmnd *sc)
task->last_timeout = jiffies;
spin_unlock(>frwd_lock);
ISCSI_DBG_EH(session, "return %s\n", rc == BLK_EH_RESET_TIMER ?
-"timer reset" : "nh");
+"timer reset" : "shutdown or nh");
return rc;
 }
 EXPORT_SYMBOL_GPL(iscsi_eh_cmd_timed_out);
-- 
2.14.2

[Bug 197875] Processes hang on attempted access of WDC WD30-EZRX 3TB HDD on HP Z420 Workstation

2017-12-07 Thread bugzilla-daemon

https://bugzilla.kernel.org/show_bug.cgi?id=197875

--- Comment #9 from Bart Van Assche (bvanass...@acm.org) ---
My hope was that the list of PCI devices would show a PCI HBA of which the
driver has been modified recently. Since that's not the case I'm out of ideas
about what could be the root cause of this bug. Unless someone else has an idea
about how to find the root cause of this issue I think your only option is to
perform a bisect of the Linux kernel.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

2017-12-07 Thread Martin K. Petersen


Stephen,

>> I have to defer to you guys on that one.  Left to myself, I will just
>> push it into the next merge window (as opposed to using my normal
>> process, which at this point would get it into the one following).
>> 
>> So please let me know how you would like to proceed.
>
> Clearly, it needs to go via Martin's tree as otherwise his tree will
> not build in some circumstances ... or if it going to cause problems
> for Paul, then it should be in a separate non-rebasing branch (probably
> of Paul's tree) that is merged into Pauls main branch and Marin's tree.

I'm perfectly OK with taking it through the SCSI tree. Probably the path
of least resistance.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] SCSI: run queue if SCSI device queue isn't ready and queue is idle

2017-12-07 Thread Bart Van Assche

On Thu, 2017-12-07 at 09:31 +0800, Ming Lei wrote:
> But if you always call blk_mq_sched_mark_restart_hctx() before a new
> dispatch, that may affect performance on NVMe which may never trigger
> BLK_STS_RESOURCE.

Hmm ... only the SCSI core implements .get_budget() and .put_budget() and
I proposed to insert a blk_mq_sched_mark_restart_hctx() call under "if
(q->mq_ops->get_budget)". In other words, I proposed to insert a
blk_mq_sched_mark_restart_hctx() call in a code path that is never triggered
by the NVMe driver. So I don't see how the change I proposed could affect
the performance of the NVMe driver?

Bart.

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

2017-12-07 Thread Paul E. McKenney

On Fri, Dec 08, 2017 at 07:34:39AM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> On Thu, 7 Dec 2017 09:40:38 -0800 "Paul E. McKenney" 
>  wrote:
> >
> > On Thu, Dec 07, 2017 at 05:30:03PM +, Bart Van Assche wrote:
> > > However, what's not clear to me is through which tree this patch should be
> > > sent to Linus? Should the above patch be sent as a v4.15-rc fix or should
> > > Martin perhaps queue it in his tree for v4.16-rc1?  
> > 
> > I have to defer to you guys on that one.  Left to myself, I will just
> > push it into the next merge window (as opposed to using my normal process,
> > which at this point would get it into the one following).
> > 
> > So please let me know how you would like to proceed.
> 
> Clearly, it needs to go via Martin's tree as otherwise his tree will
> not build in some circumstances ... or if it going to cause problems
> for Paul, then it should be in a separate non-rebasing branch (probably
> of Paul's tree) that is merged into Pauls main branch and Marin's tree.

It is unlikely to cause problems, so please let it go up where convenient.

Just please let me know.

Thanx, Paul

Re: [PATCH] SCSI: run queue if SCSI device queue isn't ready and queue is idle

2017-12-07 Thread Bart Van Assche

On Wed, 2017-12-06 at 00:28 +0800, Ming Lei wrote:
> On Tue, Dec 05, 2017 at 04:08:20PM +, Bart Van Assche wrote:
> > On Tue, 2017-12-05 at 15:52 +0800, Ming Lei wrote:
> > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > > index db9556662e27..1816dd8259b3 100644
> > > --- a/drivers/scsi/scsi_lib.c
> > > +++ b/drivers/scsi/scsi_lib.c
> > > @@ -1967,6 +1967,8 @@ static bool scsi_mq_get_budget(struct blk_mq_hw_ctx 
> > > *hctx)
> > >  out_put_device:
> > >   put_device(>sdev_gendev);
> > >  out:
> > > + if (atomic_read(>device_busy) == 0 && !scsi_device_blocked(sdev))
> > > + blk_mq_delay_run_hw_queue(hctx, SCSI_QUEUE_DELAY);
> > >   return false;
> > >  }
> > 
> > This cannot work since multiple threads can call scsi_mq_get_budget()
> 
> That is exactly the way we are handling these cases before 0df21c86bdbf(scsi:
> implement .get_budget and .put_budget for blk-mq), so if it can't work,
> that is not fault of commit 0df21c86bdbf.
> 
> > concurrently and hence it can happen that none of them sees
> > atomic_read(>device_busy) == 0. BTW, the above patch is something I
> 
> If there is concurrent .get_budget(), one of them will see the counter
> becoming zero finally because each sdev->device_busy is inc/dec
> atomically. Or scsi_dev_queue_ready() return true.
> 
> Anyway, we need this patch to avoid possible regression. If you think
> there are bugs in blk-mq RESTART, just root cause and and fix it.

Hello Ming,

When I looked at the patch at the start of this thread for the first time I
got frustrated because I didn't see how this patch could fix the queue stall
I ran into myself. Today I started realizing that what Holger reported is
probably another issue than what I ran into myself. Since this patch by
itself looks now useful to me:

Reviewed-by: Bart Van Assche 

BTW, do you think the patch at the start of this thread also fixes the issue
that resulted in commit 826a70a08b12 ("SCSI: don't get target/host busy_count
in scsi_mq_get_budget()")? Do you think we still need that patch?

Bart.

Re: [PATCH] drivers/scsi/qla2xxx: fix double free bug after firmware timeout

2017-12-07 Thread Madhani, Himanshu

Hi Max,

> On Dec 7, 2017, at 6:46 AM, Max Kellermann  wrote:
> 
> When the qla2xxx firmware is unavailable, eventually
> qla2x00_sp_timeout() is reached, which calls the timeout function and
> frees the srb_t instance.
> 
> The timeout function always resolves to qla2x00_async_iocb_timeout(),
> which invokes another callback function called "done".  All of these
> qla2x00_*_sp_done() callbacks also free the srb_t instance; after
> returning to qla2x00_sp_timeout(), it is freed again.
> 
> The fix is to remove the "sp->free(sp)" call from qla2x00_sp_timeout()
> and add it to those code paths in qla2x00_async_iocb_timeout() which
> do not already free the object.
> 
> This is how it looks like with KASAN:
> 
> BUG: KASAN: use-after-free in qla2x00_sp_timeout+0x228/0x250
> Read of size 8 at addr 88278147a590 by task swapper/2/0
> 
> Allocated by task 1502:
>  save_stack+0x33/0xa0
>  kasan_kmalloc+0xa0/0xd0
>  kmem_cache_alloc+0xb8/0x1c0
>  mempool_alloc+0xd6/0x260
>  qla24xx_async_gnl+0x3c5/0x1100
> 
> Freed by task 0:
>  save_stack+0x33/0xa0
>  kasan_slab_free+0x72/0xc0
>  kmem_cache_free+0x75/0x200
>  qla24xx_async_gnl_sp_done+0x556/0x9e0
>  qla2x00_async_iocb_timeout+0x1c7/0x420
>  qla2x00_sp_timeout+0x16d/0x250
>  call_timer_fn+0x36/0x200
> 
> The buggy address belongs to the object at 88278147a440
>  which belongs to the cache qla2xxx_srbs of size 344
> The buggy address is located 336 bytes inside of
>  344-byte region [88278147a440, 88278147a598)
> 
> Signed-off-by: Max Kellermann 
> ---
> drivers/scsi/qla2xxx/qla_init.c |3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index b5b48ddca962..801890564e00 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -58,7 +58,6 @@ qla2x00_sp_timeout(unsigned long __data)
>   req->outstanding_cmds[sp->handle] = NULL;
>   iocb = >u.iocb_cmd;
>   iocb->timeout(sp);
> - sp->free(sp);
>   spin_unlock_irqrestore(>hw->hardware_lock, flags);
> }
> 
> @@ -121,9 +120,11 @@ qla2x00_async_iocb_timeout(void *data)
>   ea.data[1] = lio->u.logio.data[1];
>   ea.sp = sp;
>   qla24xx_handle_plogi_done_event(fcport->vha, );
> + sp->free(sp);
>   break;
>   case SRB_LOGOUT_CMD:
>   qlt_logo_completion_handler(fcport, QLA_FUNCTION_TIMEOUT);
> + sp->free(sp);
>   break;
>   case SRB_CT_PTHRU_CMD:
>   case SRB_MB_IOCB:
> 


NACK

These calls are asynchronous calls and free should be called by completion.

I am going to send updates to driver which we have fixed similar issue for 4.16

Thanks,
- Himanshu

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

2017-12-07 Thread Stephen Rothwell

Hi all,

On Thu, 7 Dec 2017 09:40:38 -0800 "Paul E. McKenney" 
 wrote:
>
> On Thu, Dec 07, 2017 at 05:30:03PM +, Bart Van Assche wrote:
> > However, what's not clear to me is through which tree this patch should be
> > sent to Linus? Should the above patch be sent as a v4.15-rc fix or should
> > Martin perhaps queue it in his tree for v4.16-rc1?  
> 
> I have to defer to you guys on that one.  Left to myself, I will just
> push it into the next merge window (as opposed to using my normal process,
> which at this point would get it into the one following).
> 
> So please let me know how you would like to proceed.

Clearly, it needs to go via Martin's tree as otherwise his tree will
not build in some circumstances ... or if it going to cause problems
for Paul, then it should be in a separate non-rebasing branch (probably
of Paul's tree) that is merged into Pauls main branch and Marin's tree.
-- 
Cheers,
Stephen Rothwell

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

2017-12-07 Thread Paul E. McKenney

On Thu, Dec 07, 2017 at 05:30:03PM +, Bart Van Assche wrote:
> On Wed, 2017-12-06 at 20:42 -0800, Paul E. McKenney wrote:
> > On Thu, Dec 07, 2017 at 03:25:21PM +1100, Stephen Rothwell wrote:
> > > On Thu, 7 Dec 2017 03:59:30 + Bart Van Assche 
> > >  wrote:

[ . . . ]

> > commit cde4691a3a4591e7355295dd62610e3262159002
> > Author: Paul E. McKenney 
> > Date:   Wed Dec 6 20:39:38 2017 -0800
> > 
> > rcu: Export init_rcu_head() and destroy_rcu_head() to GPL modules
> > 
> > Use of init_rcu_head() and destroy_rcu_head() from modules results in
> > the following build-time error:
> > 
> > ERROR: "init_rcu_head" [drivers/scsi/scsi_mod.ko] undefined!
> > ERROR: "destroy_rcu_head" [drivers/scsi/scsi_mod.ko] undefined!
> > 
> > This commit therefore adds EXPORT_SYMBOL_GPL() for each to allow them
> > to be used by GPL-licensed kernel modules.
> > 
> > Reported-by: Bart Van Assche 
> > Reported-by: Stephen Rothwell 
> > Signed-off-by: Paul E. McKenney 
> > 
> > diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
> > index 8d591d8411fe..4c4d26e9a67b 100644
> > --- a/kernel/rcu/update.c
> > +++ b/kernel/rcu/update.c
> > @@ -422,11 +422,13 @@ void init_rcu_head(struct rcu_head *head)
> >  {
> > debug_object_init(head, _debug_descr);
> >  }
> > +EXPORT_SYMBOL_GPL(init_rcu_head);
> >  
> >  void destroy_rcu_head(struct rcu_head *head)
> >  {
> > debug_object_free(head, _debug_descr);
> >  }
> > +EXPORT_SYMBOL_GPL(destroy_rcu_head);
> >  
> >  static bool rcuhead_is_static_object(void *addr)
> >  {
> 
> (+linux-scsi)
> 
> Hello Paul,
> 
> How about changing the commit message into "... fixes a build failure with
> CONFIG_DEBUG_OBJECTS_RCU_HEAD=y"? Otherwise the above patch looks fine to me
> and fixes the reported build failure on my setup.

I have updated it as shown below.

> However, what's not clear to me is through which tree this patch should be
> sent to Linus? Should the above patch be sent as a v4.15-rc fix or should
> Martin perhaps queue it in his tree for v4.16-rc1?

I have to defer to you guys on that one.  Left to myself, I will just
push it into the next merge window (as opposed to using my normal process,
which at this point would get it into the one following).

So please let me know how you would like to proceed.

Thanx, Paul



commit 193dffdf4354f14b4f3369a85128817e5ba74e58
Author: Paul E. McKenney 
Date:   Wed Dec 6 20:39:38 2017 -0800

rcu: Export init_rcu_head() and destroy_rcu_head() to GPL modules

Use of init_rcu_head() and destroy_rcu_head() from modules results in
the following build-time error with CONFIG_DEBUG_OBJECTS_RCU_HEAD=y:

ERROR: "init_rcu_head" [drivers/scsi/scsi_mod.ko] undefined!
ERROR: "destroy_rcu_head" [drivers/scsi/scsi_mod.ko] undefined!

This commit therefore adds EXPORT_SYMBOL_GPL() for each to allow them
to be used by GPL-licensed kernel modules.

Reported-by: Bart Van Assche 
Reported-by: Stephen Rothwell 
Signed-off-by: Paul E. McKenney 

diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index 8d591d8411fe..4c4d26e9a67b 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -422,11 +422,13 @@ void init_rcu_head(struct rcu_head *head)
 {
debug_object_init(head, _debug_descr);
 }
+EXPORT_SYMBOL_GPL(init_rcu_head);
 
 void destroy_rcu_head(struct rcu_head *head)
 {
debug_object_free(head, _debug_descr);
 }
+EXPORT_SYMBOL_GPL(destroy_rcu_head);
 
 static bool rcuhead_is_static_object(void *addr)
 {

Re: [PATCH 0/3] SCSI device blacklist handling improvements

2017-12-07 Thread Bart Van Assche

On Wed, 2017-12-06 at 21:03 -0500, Martin K. Petersen wrote:
> > These three patches is what I came up with after having reviewed
> > recent changes in the code for handling blacklist flags
> > handling. Please consider these patches for kernel v4.16.
> 
> I applied 1 and 3 to 4.16/scsi-queue. I am still not a fan of forcing
> u32. That's a recipe for disaster when we add the next flag.

Hello Martin,

Are you perhaps referring to the five __force casts? If so, do you have a
suggestion for avoiding that sparse reports false positive warnings on the
conversions between int and blist_flags_t? The only approach I can think of
to reduce the number of __force casts is to embed these __force casts into
two helper functions - one for the conversion from int to blist_flags_t and
one for the conversion the other way around.

Thanks,

Bart.

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

2017-12-07 Thread Bart Van Assche

On Wed, 2017-12-06 at 20:42 -0800, Paul E. McKenney wrote:
> On Thu, Dec 07, 2017 at 03:25:21PM +1100, Stephen Rothwell wrote:
> > On Thu, 7 Dec 2017 03:59:30 + Bart Van Assche  
> > wrote:
> > > On Thu, 2017-12-07 at 14:57 +1100, Stephen Rothwell wrote:
> > > > After merging the scsi-mkp tree, today's linux-next build (x86_64
> > > > allmodconfig) failed like this:
> > > > 
> > > > ERROR: "init_rcu_head" [drivers/scsi/scsi_mod.ko] undefined!
> > > > ERROR: "destroy_rcu_head" [drivers/scsi/scsi_mod.ko] undefined!
> > > > 
> > > > Caused by commit
> > > > 
> > > >   ac90420f17c9 ("scsi: core: Ensure that the SCSI error handler gets 
> > > > woken up")
> > > > 
> > > > I have used the scsi-mkp tree from next-20171206 for today.  
> > > 
> > > Does that mean I'm the first one who added RCU code to the SCSI core?
> > 
> > The only other uses of init_rcu_head() are in drivers/iommu/intel-svm.c
> > and kernel/irq/irqdesc.c.  destroy_rcu_head() appears to not be used
> > anywhere ...
> 
> The key point is that Bart appears to be the first to try using them in
> a module, for which exports are needed.  Does the patch below help?
> 
>   Thanx, Paul
> 
> 
> 
> commit cde4691a3a4591e7355295dd62610e3262159002
> Author: Paul E. McKenney 
> Date:   Wed Dec 6 20:39:38 2017 -0800
> 
> rcu: Export init_rcu_head() and destroy_rcu_head() to GPL modules
> 
> Use of init_rcu_head() and destroy_rcu_head() from modules results in
> the following build-time error:
> 
>   ERROR: "init_rcu_head" [drivers/scsi/scsi_mod.ko] undefined!
>   ERROR: "destroy_rcu_head" [drivers/scsi/scsi_mod.ko] undefined!
> 
> This commit therefore adds EXPORT_SYMBOL_GPL() for each to allow them
> to be used by GPL-licensed kernel modules.
> 
> Reported-by: Bart Van Assche 
> Reported-by: Stephen Rothwell 
> Signed-off-by: Paul E. McKenney 
> 
> diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
> index 8d591d8411fe..4c4d26e9a67b 100644
> --- a/kernel/rcu/update.c
> +++ b/kernel/rcu/update.c
> @@ -422,11 +422,13 @@ void init_rcu_head(struct rcu_head *head)
>  {
>   debug_object_init(head, _debug_descr);
>  }
> +EXPORT_SYMBOL_GPL(init_rcu_head);
>  
>  void destroy_rcu_head(struct rcu_head *head)
>  {
>   debug_object_free(head, _debug_descr);
>  }
> +EXPORT_SYMBOL_GPL(destroy_rcu_head);
>  
>  static bool rcuhead_is_static_object(void *addr)
>  {

(+linux-scsi)

Hello Paul,

How about changing the commit message into "... fixes a build failure with
CONFIG_DEBUG_OBJECTS_RCU_HEAD=y"? Otherwise the above patch looks fine to me
and fixes the reported build failure on my setup.

However, what's not clear to me is through which tree this patch should be
sent to Linus? Should the above patch be sent as a v4.15-rc fix or should
Martin perhaps queue it in his tree for v4.16-rc1?

Thanks,

Bart.

Re: [PATCH v3 0/2] Ensure that the SCSI error handler gets woken up

2017-12-07 Thread Bart Van Assche

On Thu, 2017-12-07 at 12:02 +, John Garry wrote:
> On 07/12/2017 01:55, Martin K. Petersen wrote:
> > 
> > Bart,
> > 
> > > As reported by Pavel Tikhomirov it can happen that the SCSI error
> > > handler does not get woken up. This is very annoying because it
> > > results in a queue stall. The two patches in this series address this
> > > issue without acquiring the SCSI host lock in the hot path. Please
> > > consider these patches for kernel v4.16.
> > 
> > Applied to 4.16/scsi-queue. Thank you!
> > 
> 
> Is anyone finding a build error for this patch? I built allmodconfig on 
> Martin's 4.16/scsi-queue and I get this:
> ERROR: "init_rcu_head" [drivers/scsi/scsi_mod.ko] undefined!
> ERROR: "destroy_rcu_head" [drivers/scsi/scsi_mod.ko] undefined!
> 
> Reverting fixes it.
> 
>  From a quick check, it seems the rcu funcitons are not exported, and 
> SCSI=m means hosts.c cannot reference it.

Hello John,

A discussion is ongoing about this issue on the linux-next mailing list.
Paul E. McKenney proposed to export both the init_rcu_head() and
destroy_rcu_head() functions. See also https://lkml.org/lkml/2017/12/6/1150.

Bart.

[PATCH] drivers/scsi/qla2xxx: fix double free bug after firmware timeout

2017-12-07 Thread Max Kellermann

When the qla2xxx firmware is unavailable, eventually
qla2x00_sp_timeout() is reached, which calls the timeout function and
frees the srb_t instance.

The timeout function always resolves to qla2x00_async_iocb_timeout(),
which invokes another callback function called "done".  All of these
qla2x00_*_sp_done() callbacks also free the srb_t instance; after
returning to qla2x00_sp_timeout(), it is freed again.

The fix is to remove the "sp->free(sp)" call from qla2x00_sp_timeout()
and add it to those code paths in qla2x00_async_iocb_timeout() which
do not already free the object.

This is how it looks like with KASAN:

 BUG: KASAN: use-after-free in qla2x00_sp_timeout+0x228/0x250
 Read of size 8 at addr 88278147a590 by task swapper/2/0

 Allocated by task 1502:
  save_stack+0x33/0xa0
  kasan_kmalloc+0xa0/0xd0
  kmem_cache_alloc+0xb8/0x1c0
  mempool_alloc+0xd6/0x260
  qla24xx_async_gnl+0x3c5/0x1100

 Freed by task 0:
  save_stack+0x33/0xa0
  kasan_slab_free+0x72/0xc0
  kmem_cache_free+0x75/0x200
  qla24xx_async_gnl_sp_done+0x556/0x9e0
  qla2x00_async_iocb_timeout+0x1c7/0x420
  qla2x00_sp_timeout+0x16d/0x250
  call_timer_fn+0x36/0x200

 The buggy address belongs to the object at 88278147a440
  which belongs to the cache qla2xxx_srbs of size 344
 The buggy address is located 336 bytes inside of
  344-byte region [88278147a440, 88278147a598)

Signed-off-by: Max Kellermann 
---
 drivers/scsi/qla2xxx/qla_init.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index b5b48ddca962..801890564e00 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -58,7 +58,6 @@ qla2x00_sp_timeout(unsigned long __data)
req->outstanding_cmds[sp->handle] = NULL;
iocb = >u.iocb_cmd;
iocb->timeout(sp);
-   sp->free(sp);
spin_unlock_irqrestore(>hw->hardware_lock, flags);
 }
 
@@ -121,9 +120,11 @@ qla2x00_async_iocb_timeout(void *data)
ea.data[1] = lio->u.logio.data[1];
ea.sp = sp;
qla24xx_handle_plogi_done_event(fcport->vha, );
+   sp->free(sp);
break;
case SRB_LOGOUT_CMD:
qlt_logo_completion_handler(fcport, QLA_FUNCTION_TIMEOUT);
+   sp->free(sp);
break;
case SRB_CT_PTHRU_CMD:
case SRB_MB_IOCB:

Re: [PATCH] libsas: flush pending destruct work in sas_unregister_domain_devices()

2017-12-07 Thread John Garry


On 28/11/2017 17:04, Cong Wang wrote:

On Tue, Nov 28, 2017 at 3:18 AM, John Garry  wrote:

On 28/11/2017 08:20, Johannes Thumshirn wrote:


On Mon, Nov 27, 2017 at 04:24:45PM -0800, Cong Wang wrote:


We saw dozens of the following kernel waring:

 WARNING: CPU: 0 PID: 705 at fs/sysfs/group.c:224
sysfs_remove_group+0x54/0x88()
 sysfs group 81ab7670 not found for kobject '6:0:3:0'
 Modules linked in: cpufreq_ondemand x86_pkg_temp_thermal coretemp
kvm_intel kvm microcode raid0 iTCO_wdt iTCO_vendor_support sb_edac edac_core
lpc_ich mfd_core ioatdma i2c_i801 shpchp wmi hed acpi_cpufreq lp parport
tcp_diag inet_diag ipmi_si ipmi_devintf ipmi_msghandler sch_fq_codel igb ptp
pps_core i2c_algo_bit i2c_core crc32c_intel isci libsas scsi_transport_sas
dca ipv6
 CPU: 0 PID: 705 Comm: kworker/u240:0 Not tainted 4.1.35.el7.x86_64 #1



This should by now be fixed with commit fbce4d97fd43 ("scsi: fixup kernel
warning during rmmod()" which went into v4.14-rc6.



Is that the same issue? I think Cong Wang is just trying to deal with the
longstanding libsas hotplug WARN.


Right, we saw it on both 4.1 and 3.14, clearly an old bug.




We at Huawei are still working to fix it. Our patchset is under internal
test at the moment.

As for this patch:

 drivers/scsi/libsas/sas_discover.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/libsas/sas_discover.c
b/drivers/scsi/libsas/sas_discover.c
index 60de66252fa2..27c11fc7aa2b 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -388,6 +388,11 @@ void sas_unregister_dev(struct asd_sas_port *port,
struct domain_device *dev)
  }
 }

+static void sas_flush_work(struct asd_sas_port *port)
+{
+ scsi_flush_work(port->ha->core.shost);
+}
+
 void sas_unregister_domain_devices(struct asd_sas_port *port, int gone)
 {
  struct domain_device *dev, *n;
@@ -401,8 +406,8 @@ void sas_unregister_domain_devices(struct asd_sas_port
*port, int gone)
  list_for_each_entry_safe(dev, n, >disco_list, disco_list_node)
  sas_unregister_dev(port, dev);

+ sas_flush_work(port);


How can this work as sas_unregister_domain_devices() may be called from the
same workqueue which you're trying to flush?




Sorry for slow reply, just remembered this now.



I don't understand, the only caller of sas_unregister_domain_devices()
is sas_deform_port().



And sas_deform_port() may be called from another worker on the same 
queue, right? As in sas_phye_loss_of_signal()->sas_deform_port()


As I see today, this is the problem callchain:
sas_deform_port()
sas_unregister_domain_devices()
sas_unregister_dev()
sas_discover_event(DISCE_DESTRUCT)

The device destruct takes place in a separate worker from which 
sas_deform_port() is called, but the same queue. So we have this queued 
destruct happen after the port is fully deformed -> hence the WARN.


I guess you only tested your patch on disks attached through an expander.

Thanks,
John









.

Re: [PATCH v3 0/2] Ensure that the SCSI error handler gets woken up

2017-12-07 Thread John Garry


On 07/12/2017 01:55, Martin K. Petersen wrote:


Bart,


As reported by Pavel Tikhomirov it can happen that the SCSI error
handler does not get woken up. This is very annoying because it
results in a queue stall. The two patches in this series address this
issue without acquiring the SCSI host lock in the hot path. Please
consider these patches for kernel v4.16.


Applied to 4.16/scsi-queue. Thank you!



Is anyone finding a build error for this patch? I built allmodconfig on 
Martin's 4.16/scsi-queue and I get this:

ERROR: "init_rcu_head" [drivers/scsi/scsi_mod.ko] undefined!
ERROR: "destroy_rcu_head" [drivers/scsi/scsi_mod.ko] undefined!

Reverting fixes it.

From a quick check, it seems the rcu funcitons are not exported, and 
SCSI=m means hosts.c cannot reference it.


Cheers,
John

[PATCH v2] scsi: libsas: fix length error in sas_smp_handler()

2017-12-07 Thread Jason Yan

The bsg_job_done() requires the length of payload received, but we give
it the untransferred residual.

Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for SMP 
passthrough")
Reported-and-tested-by: chenqilin 
Signed-off-by: Jason Yan 
CC: Christoph Hellwig 
---
 drivers/scsi/libsas/sas_expander.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c 
b/drivers/scsi/libsas/sas_expander.c
index 50cb0f3..6c40ecc 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2143,7 +2143,7 @@ void sas_smp_handler(struct bsg_job *job, struct 
Scsi_Host *shost,
struct sas_rphy *rphy)
 {
struct domain_device *dev;
-   unsigned int reslen = 0;
+   unsigned int rcvlen = 0;
int ret = -EINVAL;
 
/* no rphy means no smp target support (ie aic94xx host) */
@@ -2177,12 +2177,12 @@ void sas_smp_handler(struct bsg_job *job, struct 
Scsi_Host *shost,
 
ret = smp_execute_task_sg(dev, job->request_payload.sg_list,
job->reply_payload.sg_list);
-   if (ret > 0) {
-   /* positive number is the untransferred residual */
-   reslen = ret;
+   if (ret >= 0) {
+   /* bsg_job_done() requires the length received  */
+   rcvlen = job->reply_payload.payload_len - ret;
ret = 0;
}
 
 out:
-   bsg_job_done(job, ret, reslen);
+   bsg_job_done(job, ret, rcvlen);
 }
-- 
2.9.5

Re: [PATCH v6 1/5] scsi: ufs: add Hisilicon ufs driver code

2017-12-07 Thread Philippe Ombredanne

Dear Li,

On Thu, Dec 7, 2017 at 11:20 AM, Li Wei  wrote:
> add Hisilicon ufs driver code.
>
> Signed-off-by: Li Wei 
> Signed-off-by: Geng Jianfeng 
> Signed-off-by: Zang Leigang 
> Signed-off-by: Yu Jianfeng 
[]

> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-hisi.c
> @@ -0,0 +1,624 @@
> +/*
> + *
> + * HiSilicon Hi UFS Driver
> + *
> + * Copyright (c) 2016-2017 Linaro Ltd.
> + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */

Would you consider using the new SPDX license ids instead?
Check Thomas doc patches for instructions.
This would be great and while you are it may be this could be adopted
for all HiSilicon and Huawei past, present and future contributions?
Thank you for your kind consideration!

-- 
Cordially
Philippe Ombredanne

Re: [PATCH] scsi: libsas: fix length error in sas_smp_handler()

2017-12-07 Thread Jason Yan



On 2017/12/7 17:27, John Garry wrote:

On 07/12/2017 01:41, Jason Yan wrote:

Can anybody review this patch? Our test of SG_IO all failed because of
this bug.

On 2017/12/5 17:39, Jason Yan wrote:

The bsg_job_done() requires the length of payload received, but we give
it the untransferred residual.

Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for
SMP")
Reported-and-tested-by: chenqilin 
Signed-off-by: Jason Yan 
CC: Christoph Hellwig 
---
  drivers/scsi/libsas/sas_expander.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 50cb0f3..8323dc1 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2177,9 +2177,9 @@ void sas_smp_handler(struct bsg_job *job, struct
Scsi_Host *shost,

  ret = smp_execute_task_sg(dev, job->request_payload.sg_list,
  job->reply_payload.sg_list);
-if (ret > 0) {
+if (ret >= 0) {
  /* positive number is the untransferred residual */
-reslen = ret;
+reslen = job->reply_payload.payload_len - ret;


Hi Jason,

If we really want the length of the payload received, then should you
change the reslen variable name? The name implies "residual length",
which is not really what it is holding according to your change.

Thanks,
John



Thanks a lot. I will correct it.

Jason


  ret = 0;
  }





.





.

[PATCH v6 0/5] scsi: ufs: add ufs driver code for Hisilicon Hi3660 SoC

2017-12-07 Thread Li Wei

This patchset adds driver support for UFS for Hi3660 SoC. It is verified on 
HiKey960 board.

Li Wei (5):
  scsi: ufs: add Hisilicon ufs driver code
  dt-bindings: scsi: ufs: add document for hisi-ufs
  arm64: dts: add ufs dts node
  arm64: defconfig: enable configs for Hisilicon ufs
  arm64: defconfig: enable f2fs and squashfs

 Documentation/devicetree/bindings/ufs/ufs-hisi.txt |  38 ++
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi  |  20 +
 arch/arm64/configs/defconfig   |  11 +
 drivers/scsi/ufs/Kconfig   |   9 +
 drivers/scsi/ufs/Makefile  |   1 +
 drivers/scsi/ufs/ufs-hisi.c| 624 +
 drivers/scsi/ufs/ufs-hisi.h| 122 
 7 files changed, 825 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ufs/ufs-hisi.txt
 create mode 100644 drivers/scsi/ufs/ufs-hisi.c
 create mode 100644 drivers/scsi/ufs/ufs-hisi.h

-- 
2.15.0

[PATCH v6 4/5] arm64: defconfig: enable configs for Hisilicon ufs

2017-12-07 Thread Li Wei

This enable configs for Hisilicon Hi UFS driver.

Signed-off-by: Li Wei 
Signed-off-by: Zhangfei Gao 
Signed-off-by: Guodong Xu 
---
 arch/arm64/configs/defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 6356c6da34ea..fa6f921eed86 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -174,6 +174,9 @@ CONFIG_BLK_DEV_SD=y
 CONFIG_SCSI_SAS_ATA=y
 CONFIG_SCSI_HISI_SAS=y
 CONFIG_SCSI_HISI_SAS_PCI=y
+CONFIG_SCSI_UFSHCD=y
+CONFIG_SCSI_UFSHCD_PLATFORM=y
+CONFIG_SCSI_UFS_HISI=y
 CONFIG_ATA=y
 CONFIG_SATA_AHCI=y
 CONFIG_SATA_AHCI_PLATFORM=y
-- 
2.15.0

[PATCH v6 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs

2017-12-07 Thread Li Wei

add ufs node document for Hisilicon.

Signed-off-by: Li Wei 
---
 Documentation/devicetree/bindings/ufs/ufs-hisi.txt | 38 ++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ufs/ufs-hisi.txt

diff --git a/Documentation/devicetree/bindings/ufs/ufs-hisi.txt 
b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
new file mode 100644
index ..73e10698960e
--- /dev/null
+++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
@@ -0,0 +1,38 @@
+* Hisilicon Universal Flash Storage (UFS) Host Controller
+
+UFS nodes are defined to describe on-chip UFS hardware macro.
+Each UFS Host Controller should have its own node.
+
+Required properties:
+- compatible: compatible list, contains one of the following -
+   "hisilicon,hi3660-ufs", "jedec,ufs-1.1" 
for hisi ufs
+   host controller present on Hi36xx 
chipset.
+- reg   : should contain UFS register address space & UFS SYS CTRL 
register address,
+- interrupt-parent  : interrupt device
+- interrupts: interrupt number
+- clocks   : List of phandle and clock specifier pairs
+- clock-names   : List of clock input name strings sorted in the same
+   order as the clocks property. 
"ref_clk", "phy_clk" is optional
+- resets: reset node register, one reset the clk and the other 
reset the controller
+- reset-names   : describe reset node register
+
+Example:
+
+   ufs: ufs@ff3b {
+   compatible = "hisilicon,hi3660-ufs", "jedec,ufs-1.1";
+   /* 0: HCI standard */
+   /* 1: UFS SYS CTRL */
+   reg = <0x0 0xff3b 0x0 0x1000>,
+   <0x0 0xff3b1000 0x0 0x1000>;
+   interrupt-parent = <>;
+   interrupts = ;
+   clocks = <_ctrl HI3660_CLK_GATE_UFSIO_REF>,
+   <_ctrl HI3660_CLK_GATE_UFSPHY_CFG>;
+   clock-names = "ref_clk", "phy_clk";
+   freq-table-hz = <0 0>, <0 0>;
+   /* offset: 0x84; bit: 12 */
+   /* offset: 0x84; bit: 7  */
+   resets = <_rst 0x84 12>,
+   <_rst 0x84 7>;
+   reset-names = "rst", "assert";
+   };
-- 
2.15.0

[PATCH v6 1/5] scsi: ufs: add Hisilicon ufs driver code

2017-12-07 Thread Li Wei

add Hisilicon ufs driver code.

Signed-off-by: Li Wei 
Signed-off-by: Geng Jianfeng 
Signed-off-by: Zang Leigang 
Signed-off-by: Yu Jianfeng 
---
 drivers/scsi/ufs/Kconfig|   9 +
 drivers/scsi/ufs/Makefile   |   1 +
 drivers/scsi/ufs/ufs-hisi.c | 624 
 drivers/scsi/ufs/ufs-hisi.h | 122 +
 4 files changed, 756 insertions(+)
 create mode 100644 drivers/scsi/ufs/ufs-hisi.c
 create mode 100644 drivers/scsi/ufs/ufs-hisi.h

diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index e27b4d4e6ae2..e09fe6ab3572 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -100,3 +100,12 @@ config SCSI_UFS_QCOM
 
  Select this if you have UFS controller on QCOM chipset.
  If unsure, say N.
+
+config SCSI_UFS_HISI
+   tristate "Hisilicon specific hooks to UFS controller platform driver"
+   depends on (ARCH_HISI || COMPILE_TEST) && SCSI_UFSHCD_PLATFORM
+   ---help---
+ This selects the Hisilicon specific additions to UFSHCD platform 
driver.
+
+ Select this if you have UFS controller on Hisilicon chipset.
+ If unsure, say N.
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 9310c6c83041..e1ebf1031437 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
 obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
 obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
 obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
+obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
diff --git a/drivers/scsi/ufs/ufs-hisi.c b/drivers/scsi/ufs/ufs-hisi.c
new file mode 100644
index ..0c9551b3bfb2
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-hisi.c
@@ -0,0 +1,624 @@
+/*
+ *
+ * HiSilicon Hi UFS Driver
+ *
+ * Copyright (c) 2016-2017 Linaro Ltd.
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ufshcd.h"
+#include "ufshcd-pltfrm.h"
+#include "unipro.h"
+#include "ufs-hisi.h"
+#include "ufshci.h"
+
+static int ufs_hisi_check_hibern8(struct ufs_hba *hba)
+{
+   int err = 0;
+   u32 tx_fsm_val_0 = 0;
+   u32 tx_fsm_val_1 = 0;
+   unsigned long timeout = jiffies + msecs_to_jiffies(HBRN8_POLL_TOUT_MS);
+
+   do {
+   err = ufshcd_dme_get(hba, UIC_ARG_MIB_SEL(MPHY_TX_FSM_STATE, 0),
+ _fsm_val_0);
+   err |= ufshcd_dme_get(hba,
+   UIC_ARG_MIB_SEL(MPHY_TX_FSM_STATE, 1), _fsm_val_1);
+   if (err || (tx_fsm_val_0 == TX_FSM_HIBERN8 &&
+   tx_fsm_val_1 == TX_FSM_HIBERN8))
+   break;
+
+   /* sleep for max. 200us */
+   usleep_range(100, 200);
+   } while (time_before(jiffies, timeout));
+
+   /*
+* we might have scheduled out for long during polling so
+* check the state again.
+*/
+   if (time_after(jiffies, timeout)) {
+   err = ufshcd_dme_get(hba, UIC_ARG_MIB_SEL(MPHY_TX_FSM_STATE, 0),
+_fsm_val_0);
+   err |= ufshcd_dme_get(hba,
+UIC_ARG_MIB_SEL(MPHY_TX_FSM_STATE, 1), _fsm_val_1);
+   }
+
+   if (err) {
+   dev_err(hba->dev, "%s: unable to get TX_FSM_STATE, err %d\n",
+   __func__, err);
+   } else if (tx_fsm_val_0 != TX_FSM_HIBERN8 ||
+tx_fsm_val_1 != TX_FSM_HIBERN8) {
+   err = -1;
+   dev_err(hba->dev, "%s: invalid TX_FSM_STATE, lane0 = %d, lane1 
= %d\n",
+   __func__, tx_fsm_val_0, tx_fsm_val_1);
+   }
+
+   return err;
+}
+
+static void ufs_hi3660_clk_init(struct ufs_hba *hba)
+{
+   struct ufs_hisi_host *host = ufshcd_get_variant(hba);
+
+   ufs_sys_ctrl_clr_bits(host, BIT_SYSCTRL_REF_CLOCK_EN, PHY_CLK_CTRL);
+   if (ufs_sys_ctrl_readl(host, PHY_CLK_CTRL) & BIT_SYSCTRL_REF_CLOCK_EN)
+   mdelay(1);
+   /* use abb clk */
+   ufs_sys_ctrl_clr_bits(host, BIT_UFS_REFCLK_SRC_SEl, UFS_SYSCTRL);
+   ufs_sys_ctrl_clr_bits(host, BIT_UFS_REFCLK_ISO_EN, PHY_ISO_EN);
+   /* open mphy ref clk */
+   ufs_sys_ctrl_set_bits(host, BIT_SYSCTRL_REF_CLOCK_EN, PHY_CLK_CTRL);
+}
+
+static void ufs_hi3660_soc_init(struct ufs_hba *hba)
+{
+   struct ufs_hisi_host *host = ufshcd_get_variant(hba);
+   u32 reg;
+
+   if (!IS_ERR(host->rst))
+   reset_control_assert(host->rst);
+
+   /* HC_PSW powerup */
+   ufs_sys_ctrl_set_bits(host,

[PATCH v6 5/5] arm64: defconfig: enable f2fs and squashfs

2017-12-07 Thread Li Wei

Partitions in HiKey960 are formatted as f2fs and squashfs.
f2fs is for userdata; squashfs is for system. Both partitions are required
by Android.

Signed-off-by: Li Wei 
Signed-off-by: Zhangfei Gao 
Signed-off-by: Guodong Xu 
---
 arch/arm64/configs/defconfig | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index fa6f921eed86..7be4ee2ac680 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -572,6 +572,7 @@ CONFIG_ACPI_APEI_GHES=y
 CONFIG_ACPI_APEI_PCIEAER=y
 CONFIG_EXT2_FS=y
 CONFIG_EXT3_FS=y
+CONFIG_F2FS_FS=y
 CONFIG_EXT4_FS_POSIX_ACL=y
 CONFIG_BTRFS_FS=m
 CONFIG_BTRFS_FS_POSIX_ACL=y
@@ -587,6 +588,13 @@ CONFIG_HUGETLBFS=y
 CONFIG_CONFIGFS_FS=y
 CONFIG_EFIVAR_FS=y
 CONFIG_SQUASHFS=y
+CONFIG_SQUASHFS_FILE_DIRECT=y
+CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU=y
+CONFIG_SQUASHFS_XATTR=y
+CONFIG_SQUASHFS_LZ4=y
+CONFIG_SQUASHFS_LZO=y
+CONFIG_SQUASHFS_XZ=y
+CONFIG_SQUASHFS_4K_DEVBLK_SIZE=y
 CONFIG_NFS_FS=y
 CONFIG_NFS_V4=y
 CONFIG_NFS_V4_1=y
-- 
2.15.0

[PATCH v6 3/5] arm64: dts: add ufs dts node

2017-12-07 Thread Li Wei

arm64: dts: add ufs node for Hisilicon.

Signed-off-by: Li Wei 
---
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 20 
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi 
b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index ab0b95ba5ae5..3c57346366ad 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -904,6 +904,26 @@
reset-gpios = < 1 0 >;
};
 
+   /* UFS */
+   ufs: ufs@ff3b {
+   compatible = "hisilicon,hi3660-ufs", "jedec,ufs-1.1";
+   /* 0: HCI standard */
+   /* 1: UFS SYS CTRL */
+   reg = <0x0 0xff3b 0x0 0x1000>,
+   <0x0 0xff3b1000 0x0 0x1000>;
+   interrupt-parent = <>;
+   interrupts = ;
+   clocks = <_ctrl HI3660_CLK_GATE_UFSIO_REF>,
+   <_ctrl HI3660_CLK_GATE_UFSPHY_CFG>;
+   clock-names = "ref_clk", "phy_clk";
+   freq-table-hz = <0 0>, <0 0>;
+   /* offset: 0x84; bit: 12 */
+   /* offset: 0x84; bit: 7  */
+   resets = <_rst 0x84 12>,
+   <_rst 0x84 7>;
+   reset-names = "rst", "assert";
+   };
+
/* SD */
dwmmc1: dwmmc1@ff37f000 {
#address-cells = <1>;
-- 
2.15.0

Re: [PATCH] scsi: libsas: fix length error in sas_smp_handler()

2017-12-07 Thread John Garry


On 07/12/2017 01:41, Jason Yan wrote:

Can anybody review this patch? Our test of SG_IO all failed because of
this bug.

On 2017/12/5 17:39, Jason Yan wrote:

The bsg_job_done() requires the length of payload received, but we give
it the untransferred residual.

Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for
SMP")
Reported-and-tested-by: chenqilin 
Signed-off-by: Jason Yan 
CC: Christoph Hellwig 
---
  drivers/scsi/libsas/sas_expander.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 50cb0f3..8323dc1 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2177,9 +2177,9 @@ void sas_smp_handler(struct bsg_job *job, struct
Scsi_Host *shost,

  ret = smp_execute_task_sg(dev, job->request_payload.sg_list,
  job->reply_payload.sg_list);
-if (ret > 0) {
+if (ret >= 0) {
  /* positive number is the untransferred residual */
-reslen = ret;
+reslen = job->reply_payload.payload_len - ret;


Hi Jason,

If we really want the length of the payload received, then should you 
change the reslen variable name? The name implies "residual length", 
which is not really what it is holding according to your change.


Thanks,
John


  ret = 0;
  }





.

Re: [PATCH] scsi: bfa: fix type conversion warning

2017-12-07 Thread Hannes Reinecke

On Wed,  6 Dec 2017 15:14:18 +0100
Arnd Bergmann  wrote:

> A regression fix introduced a harmless type mismatch warning:
> 
> drivers/scsi/bfa/bfad_bsg.c: In function 'bfad_im_bsg_vendor_request':
> drivers/scsi/bfa/bfad_bsg.c:3137:35: error: initialization of 'struct
> bfad_im_port_s *' from 'long unsigned int' makes pointer from integer
> without a cast [-Werror=int-conversion] struct bfad_im_port_s
> *im_port = shost->hostdata[0]; ^ drivers/scsi/bfa/bfad_bsg.c: In
> function 'bfad_im_bsg_els_ct_request':
> drivers/scsi/bfa/bfad_bsg.c:3353:35: error: initialization of 'struct
> bfad_im_port_s *' from 'long unsigned int' makes pointer from integer
> without a cast [-Werror=int-conversion] struct bfad_im_port_s
> *im_port = shost->hostdata[0];
> 
> This changes the code back to shost_priv() once more, but encapsulates
> it in an inline function to document the rather unusual way of
> using the private data only as a pointer to the previously allocated
> structure.
> 
> I did not try to get rid of the extra indirection level entirely,
> which would have been rather invasive and required reworking the
> entire initialization sequence.
> 
> Fixes: 45349821ab3a ("scsi: bfa: fix access to bfad_im_port_s")
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/scsi/bfa/bfad_bsg.c |  4 ++--
>  drivers/scsi/bfa/bfad_im.c  |  6 --
>  drivers/scsi/bfa/bfad_im.h  | 10 ++
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes

Re: [PATCH] scsi: bfa: fix type conversion warning

2017-12-07 Thread Johannes Thumshirn

Thanks Arnd,
Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

57 matches

Mail list logo