Re: [PATCH] libsas: flush pending destruct work in sas_unregister_domain_devices()
On 2017/12/8 6:57, Cong Wang wrote: On Thu, Dec 7, 2017 at 5:37 AM, John Garrywrote: 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
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
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
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
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"
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
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
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()
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
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()
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
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()
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
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
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 Weiwrote: > 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
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()
On Thu, Dec 7, 2017 at 4:40 PM, Cong Wangwrote: > 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
> 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
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
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
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 AsscheHi 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()
On Thu, Dec 7, 2017 at 2:57 PM, Cong Wangwrote: > 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
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
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 AsscheCc: 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()
Remove this function since it has an empty body. Signed-off-by: Bart Van AsscheCc: 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()
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 AsscheCc: 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
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()
On Thu, Dec 7, 2017 at 5:37 AM, John Garrywrote: > 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
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 AsscheCc: 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
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
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
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
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
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
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
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 AsscheBTW, 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
Hi Max, > On Dec 7, 2017, at 6:46 AM, Max Kellermannwrote: > > 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
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
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
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
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
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
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()
On 28/11/2017 17:04, Cong Wang wrote: On Tue, Nov 28, 2017 at 3:18 AM, John Garrywrote: 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
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()
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: chenqilinSigned-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
Dear Li, On Thu, Dec 7, 2017 at 11:20 AM, Li Weiwrote: > 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()
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: chenqilinSigned-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
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
This enable configs for Hisilicon Hi UFS driver. Signed-off-by: Li WeiSigned-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
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
add Hisilicon ufs driver code. Signed-off-by: Li WeiSigned-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
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 WeiSigned-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
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()
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: chenqilinSigned-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
On Wed, 6 Dec 2017 15:14:18 +0100 Arnd Bergmannwrote: > 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
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