Re: [PATCH 06/10] xen-scsifront: remove DISABLE_CLUSTERING
On 13/12/2018 16:17, Christoph Hellwig wrote: > There is no such limitation in the protocol or implementation, so > remove it. > > Signed-off-by: Christoph Hellwig Reviewed-by: Juergen Gross Juergen
Re: [PATCH V2] blk-mq: Set request mapping to NULL in blk_mq_put_driver_tag
On 12/18/18 8:08 AM, Kashyap Desai wrote:
V1 -> V2
Added fix in __blk_mq_finish_request around blk_mq_put_tag() for
non-internal tags
Problem statement :
Whenever try to get outstanding request via scsi_host_find_tag,
block layer will return stale entries instead of actual outstanding
request. Kernel panic if stale entry is inaccessible or memory is reused.
Fix :
Undo request mapping in blk_mq_put_driver_tag nce request is return.
More detail :
Whenever each SDEV entry is created, block layer allocate separate tags
and static requestis.Those requests are not valid after SDEV is deleted
from the system. On the fly, block layer maps static rqs to rqs as below
from blk_mq_get_driver_tag()
data.hctx->tags->rqs[rq->tag] = rq;
Above mapping is active in-used requests and it is the same mapping which
is referred in function scsi_host_find_tag().
After running some IOs, “data.hctx->tags->rqs[rq->tag]” will have some
entries which will never be reset in block layer.
There would be a kernel panic, If request pointing to
“data.hctx->tags->rqs[rq->tag]” is part of “sdev” which is removed
and as part of that all the memory allocation of request associated with
that sdev might be reused or inaccessible to the driver.
Kernel panic snippet -
BUG: unable to handle kernel paging request at ff800010
IP: [] mpt3sas_scsih_scsi_lookup_get+0x6c/0xc0 [mpt3sas]
PGD aa4414067 PUD 0
Oops: [#1] SMP
Call Trace:
[] mpt3sas_get_st_from_smid+0x1f/0x60 [mpt3sas]
[] scsih_shutdown+0x55/0x100 [mpt3sas]
Cc:
Signed-off-by: Kashyap Desai
Signed-off-by: Sreekanth Reddy
---
block/blk-mq.c | 4 +++-
block/blk-mq.h | 1 +
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 6a75662..88d1e92 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -477,8 +477,10 @@ static void __blk_mq_free_request(struct request *rq)
const int sched_tag = rq->internal_tag;
blk_pm_mark_last_busy(rq);
-if (rq->tag != -1)
+if (rq->tag != -1) {
+hctx->tags->rqs[rq->tag] = NULL;
blk_mq_put_tag(hctx, hctx->tags, ctx, rq->tag);
+}
if (sched_tag != -1)
blk_mq_put_tag(hctx, hctx->sched_tags, ctx, sched_tag);
blk_mq_sched_restart(hctx);
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 9497b47..57432be 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -175,6 +175,7 @@ static inline bool
blk_mq_get_dispatch_budget(struct blk_mq_hw_ctx *hctx)
static inline void __blk_mq_put_driver_tag(struct blk_mq_hw_ctx *hctx,
struct request *rq)
{
+hctx->tags->rqs[rq->tag] = NULL;
blk_mq_put_tag(hctx, hctx->tags, rq->mq_ctx, rq->tag);
rq->tag = -1;
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
[PATCH 1/2] megaraid_sas: Use 63-bit DMA addressing
Although MegaRAID controllers support 64-bit DMA addressing,
as per hardware design, DMA address with all 64-bits set
(0x-) results in a firmware fault.
Fix -
Driver will set 63-bit DMA mask to ensure the above address
will not be used.
Cc: sta...@vger.kernel.org
Signed-off-by: Shivasharan S
---
drivers/scsi/megaraid/megaraid_sas_base.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
b/drivers/scsi/megaraid/megaraid_sas_base.c
index b2d11e8f56b5..634cc8aed1c7 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -6185,13 +6185,13 @@ static int megasas_io_attach(struct megasas_instance
*instance)
* @instance: Adapter soft state
* Description:
*
- * For Ventura, driver/FW will operate in 64bit DMA addresses.
+ * For Ventura, driver/FW will operate in 63bit DMA addresses.
*
* For invader-
* By default, driver/FW will operate in 32bit DMA addresses
* for consistent DMA mapping but if 32 bit consistent
- * DMA mask fails, driver will try with 64 bit consistent
- * mask provided FW is true 64bit DMA capable
+ * DMA mask fails, driver will try with 63 bit consistent
+ * mask provided FW is true 63bit DMA capable
*
* For older controllers(Thunderbolt and MFI based adapters)-
* driver/FW will operate in 32 bit consistent DMA addresses.
@@ -6205,14 +6205,14 @@ megasas_set_dma_mask(struct megasas_instance *instance)
pdev = instance->pdev;
consistent_mask = (instance->adapter_type >= VENTURA_SERIES) ?
- DMA_BIT_MASK(64) : DMA_BIT_MASK(32);
+ DMA_BIT_MASK(63) : DMA_BIT_MASK(32);
if (IS_DMA64) {
- if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
+ if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(63)) &&
dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
goto fail_set_dma_mask;
- if ((*pdev->dev.dma_mask == DMA_BIT_MASK(64)) &&
+ if ((*pdev->dev.dma_mask == DMA_BIT_MASK(63)) &&
(dma_set_coherent_mask(&pdev->dev, consistent_mask) &&
dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32 {
/*
@@ -6225,7 +6225,7 @@ megasas_set_dma_mask(struct megasas_instance *instance)
if (!(scratch_pad_1 & MR_CAN_HANDLE_64_BIT_DMA_OFFSET))
goto fail_set_dma_mask;
else if (dma_set_mask_and_coherent(&pdev->dev,
- DMA_BIT_MASK(64)))
+ DMA_BIT_MASK(63)))
goto fail_set_dma_mask;
}
} else if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
@@ -6237,8 +6237,8 @@ megasas_set_dma_mask(struct megasas_instance *instance)
instance->consistent_mask_64bit = true;
dev_info(&pdev->dev, "%s bit DMA mask and %s bit consistent mask\n",
-((*pdev->dev.dma_mask == DMA_BIT_MASK(64)) ? "64" : "32"),
-(instance->consistent_mask_64bit ? "64" : "32"));
+((*pdev->dev.dma_mask == DMA_BIT_MASK(64)) ? "63" : "32"),
+(instance->consistent_mask_64bit ? "63" : "32"));
return 0;
--
2.16.1
[PATCH 2/2] megaraid_sas: driver version update
Signed-off-by: Shivasharan S --- drivers/scsi/megaraid/megaraid_sas.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h index 8bfe4c54e4ae..2099c8e9d629 100644 --- a/drivers/scsi/megaraid/megaraid_sas.h +++ b/drivers/scsi/megaraid/megaraid_sas.h @@ -33,8 +33,8 @@ /* * MegaRAID SAS Driver meta data */ -#define MEGASAS_VERSION"07.707.03.00-rc1" -#define MEGASAS_RELDATE"August 30, 2018" +#define MEGASAS_VERSION"07.707.50.00-rc1" +#define MEGASAS_RELDATE"December 18, 2018" /* * Device IDs -- 2.16.1
[PATCH 0/2] megaraid_sas: Fix 64-bit DMA addressing
This patchset fixes an issue with enabling 64-bit DMA for megaraid controllers. The second patch updates the driver version for tracking. Shivasharan S (2): megaraid_sas: Use 63-bit DMA addressing megaraid_sas: driver version update drivers/scsi/megaraid/megaraid_sas.h | 4 ++-- drivers/scsi/megaraid/megaraid_sas_base.c | 18 +- 2 files changed, 11 insertions(+), 11 deletions(-) -- 2.16.1
Re: [PATCH 0/7] v4.19-stable randconfig fixes
On Mon, Dec 17, 2018 at 07:20:28PM -0500, Sasha Levin wrote:
> On Fri, Dec 14, 2018 at 11:10:05PM +0100, Arnd Bergmann wrote:
> > Hi Greg,
> >
> > I did some randconfig testing on linux-4.19 arm/arm64/x86. So far I needed
> > 27 patches, most of which are also still needed in mainline Linux. I
> > had submitted some before, and others were not submitted previously
> > for some reason. I'll try to get those fixed in mainline and then
> > make sure we get them into 4.19 as well.
> >
> > This series for now contains four patches that did make it into mainline:
> >
> > 2e6ae11dd0d1 ("slimbus: ngd: mark PM functions as __maybe_unused")
> > 33f49571d750 ("staging: olpc_dcon: add a missing dependency")
> > 0eeec01488da ("scsi: raid_attrs: fix unused variable warning")
> > 11d4afd4ff66 ("sched/pelt: Fix warning and clean up IRQ PELT config")
> >
> > Feel free to either cherry-pick those from mainline or apply the
> > patch from this series, whichever works best for you.
> >
> > The other three patches are for warnings in code that got removed in
> > mainline kernels:
> >
> > 3e9efc3299dd ("i2c: aspeed: Handle master/slave combined irq events
> > properly")
> > 972910948fb6 ("ARM: dts: qcom: Remove Arrow SD600 eval board")
> > effec874792f ("drm/msm/dpu: Remove dpu_dbg")
> >
> > My feeling was that it's safer to just address the warning by fixing
> > the code correctly in each of these cases, but if you disagree,
> > applying the mainline change should work equally well, so decide
> > for yourself.
>
> Thanks Arnd, I took the series as is.
>
> We really need to discuss how -stable deals with removed code upstream.
> For some cases, we should probably follow suit and remove it from
> -stable as well (I'm mostly thinking dodgy code with potential security
> issues).
It would be nice to do that at times (like lustre and ipx), but it's
good to keep that code around as maybe someone is using it? I don't
know, it's a tough call...
thanks,
greg k-h
Re: [PATCH 00/41] scsi: Mark expected switch fall-throughs
Hi Martin, Friendly ping: Only 8 out the 41 patches in this series have been applied so far. I wonder if you could apply the rest of this series, except: [PATCH 02/41] scsi: NCR5380: Mark expected switch fall-through (I'll send a v2 of this patch) Thanks -- Gustavo On 11/27/18 10:18 PM, Gustavo A. R. Silva wrote: In preparation to enabling -Wimplicit-fallthrough, this patchset aims to mark switch cases where we are expecting to fall through. I reviewed case by case and concluded that each of them is an intentional fall-through. However, it doesn't hurt that the maintainers and supporters of each driver take a look. :) Each commit log contains the particular details for the changes in the corresponding file. This series fix a total of 110 of the following type of warnings in drivers/scsi: drivers/scsi/aic7xxx/aic7xxx_core.c:4921:3: warning: this statement may fall through [-Wimplicit-fallthrough=] ahc_dma_tag_destroy(ahc, scb_data->sg_dmat); ^~~ drivers/scsi/aic7xxx/aic7xxx_core.c:4923:2: note: here case 6: ^~~~ Thanks! Gustavo A. R. Silva (41): scsi: BusLogic: mark expected switch fall-through scsi: NCR5380: Mark expected switch fall-through scsi: aacraid: aachba: Mark expected switch fall-throughs scsi: aacraid: linit: Mark expected switch fall-through scsi: aic7xxx: aic79xx: mark expected switch fall-through scsi: aic7xxx: mark expected switch fall-throughs scsi: be2iscsi: be_iscsi: Mark expected switch fall-through scsi: be2iscsi: be_main: Mark expected switch fall-through scsi: bfa: bfa_fcpim: Mark expected switch fall-throughs scsi: bfa: bfa_fcs_lport: Mark expected switch fall-throughs scsi: bfa: bfa_fcs_rport: Mark expected switch fall-throughs scsi: bfa: bfa_ioc: Mark expected switch fall-throughs scsi: csiostor: csio_wr: mark expected switch fall-through scsi: esas2r: esas2r_init: mark expected switch fall-throughs scsi: hpsa: mark expected switch fall-throughs scsi: imm: mark expected switch fall-throughs scsi: isci: phy: Mark expected switch fall-through scsi: isci: remote_device: Mark expected switch fall-throughs scsi: isci: remote_node_context: mark expected switch fall-throughs scsi: isci: request: mark expected switch fall-through scsi: libfc: fc_rport: Mark expected switch fall-through scsi: lpfc: lpfc_ct: Mark expected switch fall-throughs scsi: lpfc: lpfc_els: Mark expected switch fall-throughs scsi: lpfc: lpfc_hbadisc: Mark expected switch fall-throughs scsi: lpfc: lpfc_nportdisc: Mark expected switch fall-through scsi: lpfc: lpfc_nvme: Mark expected switch fall-through scsi: lpfc: lpfc_scsi: Mark expected switch fall-throughs scsi: lpfc: lpfc_sli: Mark expected switch fall-throughs scsi: megaraid: megaraid_sas_base: Mark expected switch fall-through scsi: megaraid_sas_fusion: Mark expected switch fall-through scsi: mpt3sas: mpt3sas_scsih: Mark expected switch fall-through scsi: myrb: Mark expected switch fall-throughs scsi: osd: osd_initiator: mark expected switch fall-throughs scsi: osst: mark expected switch fall-throughs scsi: ppa: mark expected switch fall-through scsi: qla4xxx: ql4_os: mark expected switch fall-through scsi: st: mark expected switch fall-throughs scsi: sym53c8xx_2: sym_hipd: mark expected switch fall-throughs scsi: sym53c8xx_2: sym_nvram: Mark expected switch fall-through scsi: ufs: ufshcd: mark expected switch fall-throughs scsi: xen-scsifront: mark expected switch fall-through drivers/scsi/BusLogic.c | 1 + drivers/scsi/NCR5380.c | 3 +- drivers/scsi/aacraid/aachba.c | 5 +++- drivers/scsi/aacraid/linit.c| 1 + drivers/scsi/aic7xxx/aic79xx_core.c | 14 + drivers/scsi/aic7xxx/aic7xxx_core.c | 12 ++-- drivers/scsi/be2iscsi/be_iscsi.c| 1 + drivers/scsi/be2iscsi/be_main.c | 1 + drivers/scsi/bfa/bfa_fcpim.c| 6 ++-- drivers/scsi/bfa/bfa_fcs_lport.c| 8 ++--- drivers/scsi/bfa/bfa_fcs_rport.c| 19 +--- drivers/scsi/bfa/bfa_ioc.c | 9 ++ drivers/scsi/csiostor/csio_wr.c | 1 + drivers/scsi/esas2r/esas2r_init.c | 3 +- drivers/scsi/hpsa.c | 5 drivers/scsi/imm.c | 33 +++-- drivers/scsi/isci/phy.c | 1 + drivers/scsi/isci/remote_device.c | 4 +-- drivers/scsi/isci/remote_node_context.c | 4 +-- drivers/scsi/isci/request.c | 2 +- drivers/scsi/libfc/fc_rport.c | 1 + drivers/scsi/lpfc/lpfc_ct.c | 2 ++ drivers/scsi/lpfc/lpfc_els.c| 1 + drivers/scsi/lpfc/lpfc_hbadisc.c| 4 ++- drivers/scsi/lpfc/lpfc_nportdisc.c | 1 + drivers/scsi/lpfc/lp
[GIT PULL] SCSI fixes for 4.20-rc7
Three fixes: The t10-pi one is a regression from the 4.19 release, the
qla2xxx one is a 4.20 merge window regression and the bnx2fc is a very
old bug.
The patch is available here:
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes
The short changelog is:
Dan Carpenter (1):
scsi: bnx2fc: Fix NULL dereference in error handling
Himanshu Madhani (1):
Revert "scsi: qla2xxx: Fix NVMe Target discovery"
Martin K. Petersen (1):
scsi: t10-pi: Return correct ref tag when queue has no integrity profile
And the diffstat:
drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 2 +-
drivers/scsi/qla2xxx/qla_os.c | 4 ++--
include/linux/t10-pi.h| 9 +
3 files changed, 8 insertions(+), 7 deletions(-)
With full diff below.
James
---
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index cd160f2ec75d..bcd30e2374f1 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -2364,7 +2364,7 @@ static int _bnx2fc_create(struct net_device *netdev,
if (!interface) {
printk(KERN_ERR PFX "bnx2fc_interface_create failed\n");
rc = -ENOMEM;
- goto ifput_err;
+ goto netdev_err;
}
if (is_vlan_dev(netdev)) {
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index b658b9a5eb1e..d0ecc729a90a 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -4886,10 +4886,10 @@ void qla24xx_create_new_sess(struct scsi_qla_host *vha,
struct qla_work_evt *e)
fcport->d_id = e->u.new_sess.id;
fcport->flags |= FCF_FABRIC_DEVICE;
fcport->fw_login_state = DSC_LS_PLOGI_PEND;
- if (e->u.new_sess.fc4_type & FS_FC4TYPE_FCP)
+ if (e->u.new_sess.fc4_type == FS_FC4TYPE_FCP)
fcport->fc4_type = FC4_TYPE_FCP_SCSI;
- if (e->u.new_sess.fc4_type & FS_FC4TYPE_NVME) {
+ if (e->u.new_sess.fc4_type == FS_FC4TYPE_NVME) {
fcport->fc4_type = FC4_TYPE_OTHER;
fcport->fc4f_nvme = FC4_TYPE_NVME;
}
diff --git a/include/linux/t10-pi.h b/include/linux/t10-pi.h
index b9626aa7e90c..3e2a80cc7b56 100644
--- a/include/linux/t10-pi.h
+++ b/include/linux/t10-pi.h
@@ -39,12 +39,13 @@ struct t10_pi_tuple {
static inline u32 t10_pi_ref_tag(struct request *rq)
{
+ unsigned int shift = ilog2(queue_logical_block_size(rq->q));
+
#ifdef CONFIG_BLK_DEV_INTEGRITY
- return blk_rq_pos(rq) >>
- (rq->q->integrity.interval_exp - 9) & 0x;
-#else
- return -1U;
+ if (rq->q->integrity.interval_exp)
+ shift = rq->q->integrity.interval_exp;
#endif
+ return blk_rq_pos(rq) >> (shift - SECTOR_SHIFT) & 0x;
}
extern const struct blk_integrity_profile t10_pi_type1_crc;
Re: [PATCH V2] blk-mq: Set request mapping to NULL in blk_mq_put_driver_tag
On Tue, 2018-12-18 at 12:38 +0530, Kashyap Desai wrote: > V1 -> V2 > Added fix in __blk_mq_finish_request around blk_mq_put_tag() for > non-internal tags > > Problem statement : > Whenever try to get outstanding request via scsi_host_find_tag, > block layer will return stale entries instead of actual outstanding > request. Kernel panic if stale entry is inaccessible or memory is reused. > Fix : > Undo request mapping in blk_mq_put_driver_tag nce request is return. > > More detail : > Whenever each SDEV entry is created, block layer allocate separate tags > and static requestis.Those requests are not valid after SDEV is deleted > from the system. On the fly, block layer maps static rqs to rqs as below > from blk_mq_get_driver_tag() > > data.hctx->tags->rqs[rq->tag] = rq; > > Above mapping is active in-used requests and it is the same mapping which > is referred in function scsi_host_find_tag(). > After running some IOs, “data.hctx->tags->rqs[rq->tag]” will have some > entries which will never be reset in block layer. > > There would be a kernel panic, If request pointing to > “data.hctx->tags->rqs[rq->tag]” is part of “sdev” which is removed > and as part of that all the memory allocation of request associated with > that sdev might be reused or inaccessible to the driver. > Kernel panic snippet - > > BUG: unable to handle kernel paging request at ff800010 > IP: [] mpt3sas_scsih_scsi_lookup_get+0x6c/0xc0 [mpt3sas] > PGD aa4414067 PUD 0 > Oops: [#1] SMP > Call Trace: > [] mpt3sas_get_st_from_smid+0x1f/0x60 [mpt3sas] > [] scsih_shutdown+0x55/0x100 [mpt3sas] Other block drivers (e.g. ib_srp, skd) do not need this to work reliably. It has been explained to you that the bug that you reported can be fixed by modifying the mpt3sas driver. So why to fix this by modifying the block layer? Additionally, what prevents that a race condition occurs between the block layer clearing hctx->tags->rqs[rq->tag] and scsi_host_find_tag() reading that same array element? I'm afraid that this is an attempt to paper over a real problem instead of fixing the root cause. Bart.
Re: [PATCH V2] blk-mq: Set request mapping to NULL in blk_mq_put_driver_tag
On 12/18/18 9:15 AM, Bart Van Assche wrote: > On Tue, 2018-12-18 at 12:38 +0530, Kashyap Desai wrote: >> V1 -> V2 >> Added fix in __blk_mq_finish_request around blk_mq_put_tag() for >> non-internal tags >> >> Problem statement : >> Whenever try to get outstanding request via scsi_host_find_tag, >> block layer will return stale entries instead of actual outstanding >> request. Kernel panic if stale entry is inaccessible or memory is reused. >> Fix : >> Undo request mapping in blk_mq_put_driver_tag nce request is return. >> >> More detail : >> Whenever each SDEV entry is created, block layer allocate separate tags >> and static requestis.Those requests are not valid after SDEV is deleted >> from the system. On the fly, block layer maps static rqs to rqs as below >> from blk_mq_get_driver_tag() >> >> data.hctx->tags->rqs[rq->tag] = rq; >> >> Above mapping is active in-used requests and it is the same mapping which >> is referred in function scsi_host_find_tag(). >> After running some IOs, “data.hctx->tags->rqs[rq->tag]” will have some >> entries which will never be reset in block layer. >> >> There would be a kernel panic, If request pointing to >> “data.hctx->tags->rqs[rq->tag]” is part of “sdev” which is removed >> and as part of that all the memory allocation of request associated with >> that sdev might be reused or inaccessible to the driver. >> Kernel panic snippet - >> >> BUG: unable to handle kernel paging request at ff800010 >> IP: [] mpt3sas_scsih_scsi_lookup_get+0x6c/0xc0 [mpt3sas] >> PGD aa4414067 PUD 0 >> Oops: [#1] SMP >> Call Trace: >> [] mpt3sas_get_st_from_smid+0x1f/0x60 [mpt3sas] >> [] scsih_shutdown+0x55/0x100 [mpt3sas] > > Other block drivers (e.g. ib_srp, skd) do not need this to work reliably. > It has been explained to you that the bug that you reported can be fixed > by modifying the mpt3sas driver. So why to fix this by modifying the block > layer? Additionally, what prevents that a race condition occurs between > the block layer clearing hctx->tags->rqs[rq->tag] and scsi_host_find_tag() > reading that same array element? I'm afraid that this is an attempt to > paper over a real problem instead of fixing the root cause. I have to agree with Bart here, I just don't see how the mpt3sas use case is special. The change will paper over the issue in any case. -- Jens Axboe
Re: [PATCH 0/7] v4.19-stable randconfig fixes
On Tue, Dec 18, 2018 at 04:12:30PM +0100, Greg Kroah-Hartman wrote:
On Mon, Dec 17, 2018 at 07:20:28PM -0500, Sasha Levin wrote:
On Fri, Dec 14, 2018 at 11:10:05PM +0100, Arnd Bergmann wrote:
> Hi Greg,
>
> I did some randconfig testing on linux-4.19 arm/arm64/x86. So far I needed
> 27 patches, most of which are also still needed in mainline Linux. I
> had submitted some before, and others were not submitted previously
> for some reason. I'll try to get those fixed in mainline and then
> make sure we get them into 4.19 as well.
>
> This series for now contains four patches that did make it into mainline:
>
> 2e6ae11dd0d1 ("slimbus: ngd: mark PM functions as __maybe_unused")
> 33f49571d750 ("staging: olpc_dcon: add a missing dependency")
> 0eeec01488da ("scsi: raid_attrs: fix unused variable warning")
> 11d4afd4ff66 ("sched/pelt: Fix warning and clean up IRQ PELT config")
>
> Feel free to either cherry-pick those from mainline or apply the
> patch from this series, whichever works best for you.
>
> The other three patches are for warnings in code that got removed in
> mainline kernels:
>
> 3e9efc3299dd ("i2c: aspeed: Handle master/slave combined irq events properly")
> 972910948fb6 ("ARM: dts: qcom: Remove Arrow SD600 eval board")
> effec874792f ("drm/msm/dpu: Remove dpu_dbg")
>
> My feeling was that it's safer to just address the warning by fixing
> the code correctly in each of these cases, but if you disagree,
> applying the mainline change should work equally well, so decide
> for yourself.
Thanks Arnd, I took the series as is.
We really need to discuss how -stable deals with removed code upstream.
For some cases, we should probably follow suit and remove it from
-stable as well (I'm mostly thinking dodgy code with potential security
issues).
It would be nice to do that at times (like lustre and ipx), but it's
good to keep that code around as maybe someone is using it? I don't
know, it's a tough call...
Yeah, it's really a case-by-case basis. I'm really concerned about an
unmaintained piece of code in stable kernels, no one actually fixes bug
in it.
With the example here where that eval board was removed because no one
was using it is a great example of code we should be removing from
stable trees as well. If no one is using it - great! but if someone
does, then the removal should be reverted upstream as well.
--
Thanks,
Sasha
Re: remove the "clustering" flag V2
On Tue, Dec 18, 2018 at 08:47:48AM +0100, Hannes Reinecke wrote: > On 12/18/18 8:08 AM, Christoph Hellwig wrote: >> Martin and others: >> >> can we get this reviewed and merge before the end of the merge window? >> That gets the clustering put of the way for the multipage-biovec work >> from Ming which we want to land in the block tree early in the next >> merge window. >> > Have we reviewed the crash in iscsi target? > It might be something completely different, but the fact still stands that > the bug is exposed after removing the clustering flag... There is no change in behavior for the drivers, it just uses a different know to archive the same thing. The crashes in the iSCSI target are if we remove the nob entirely on the initiator, but that is not what this series does.
Re: scsi: ufs: Problem at init on msm8998
On 17/12/2018 10:28, Marc Gonzalez wrote: > On 17/12/2018 08:14, Kyuho Choi wrote: > >> On Thursday, December 13, 2018, Marc Gonzalez wrote: >> >> I'm having trouble getting UFS working on an APQ8098 MEDIABOX dev board. >> (I'm running v4.20-rc4 with a few UFS patches taken off the MSM list.) >> >> I'm hoping someone with experience with the UFSHC will spot the one thing >> missing that will make everything work! >> >> [ 4.929036] ufshcd-qcom 1da4000.ufshc: ufshcd_wait_for_dev_cmd: >> dev_cmd request timedout, tag 31 >> [ 4.929229] ufshcd_upiu: query_complete_err: 1da4000.ufshc: HDR:16 00 >> 00 1f 00 81 00 00 00 00 00 00, CDB:06 01 00 00 00 00 00 00 00 00 00 00 00 00 >> 00 00 >> [ 4.937060] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending >> flag query for idn 1 failed, err = -11 >> [ 4.950633] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: >> failed with error -11, retries 0 >> [ 4.960131] ufshcd_upiu: query_send: 1da4000.ufshc: HDR:16 00 00 1f >> 00 81 00 00 00 00 00 00, CDB:06 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> [ 4.968909] ufshcd_command: send: 1da4000.ufshc: tag: 31, DB: >> 0x8000, size: -1, IS: 0, LBA: 18446744073709551615, opcode: 0x0 >> [ 5.034505] ufshcd-qcom 1da4000.ufshc: ufshcd_update_uic_error: UIC >> error flags = 0x >> [ 5.098107] ufshcd-qcom 1da4000.ufshc: ufshcd_update_uic_error: UIC >> error flags = 0x >> [ 5.168571] ufshcd-qcom 1da4000.ufshc: ufshcd_update_uic_error: UIC >> error flags = 0x0001 >> >> Here, your ufs has UIC layer error. Maybe it'll be >> affected (or not) fDeviceInit set query cmnd. >> >> Does this error reproduced every bootup? > > Yes, the error occurs every single time. > >> Just for test, Increase query timeout value to over 1.5sec. And how about >> another kernel versions like APQ808's reference kernel env? > > Indeed, this is something I forgot to mention! > > UFS does work on the downstream kernel (v4.4) using the qcom driver. > > So this is purely a software initialization issue. > > I will test your suggestion, thanks. I am also making a minimal vendor kernel, > with all drivers removed except UFS, so I can dump all memory accesses at init > for clock, regulators, UFS PHY, and UFSHC :-) Below, the raw boot log for the downstream kernel. Now the fun begins, to find the tiny difference that makes/breaks everything :-) (P.S. I just noticed that my mainline kernel now times out on the very first UFS command. I need to revert a bunch of patches to figure out what happened.) [0.00] Booting Linux on physical CPU 0x0 [0.00] Initializing cgroup subsys cpuset [0.00] Initializing cgroup subsys cpu [0.00] Initializing cgroup subsys cpuacct [0.00] Initializing cgroup subsys schedtune [0.00] Linux version 4.4.78 (mgonzalez@venus) (gcc version 7.3.1 20180425 [linaro-7.3-2018.05 revision d29120a424ecfbc167ef90065c0eeb7f91977701] (Linaro GCC 7.3-2018.05) ) #4 SMP PREEMPT Tue Dec 18 16:44:01 CET 2018 [0.00] Boot CPU: AArch64 Processor [51af8014] [0.00] Machine: Qualcomm Technologies, Inc. APQ 8098 V2.1 mediabox [0.00] debug: ignoring loglevel setting. [0.00] Reserved memory: reserved region for node 'removed_regions@8580': base 0x8580, size 55 MiB [0.00] Reserved memory: reserved region for node 'pil_ipa_gpu_region@9520': base 0x9520, size 1 MiB [0.00] Reserved memory: reserved region for node 'pil_slpi_region@9430': base 0x9430, size 15 MiB [0.00] Reserved memory: reserved region for node 'pil_mba_region@9410': base 0x9410, size 2 MiB [0.00] Reserved memory: reserved region for node 'pil_video_region@93c0': base 0x93c0, size 5 MiB [0.00] Reserved memory: reserved region for node 'modem_region@8cc0': base 0x8cc0, size 112 MiB [0.00] Reserved memory: reserved region for node 'pil_adsp_region@0x8b20': base 0x8b20, size 26 MiB [0.00] Reserved memory: reserved region for node 'spss_region@8ab0': base 0x8ab0, size 7 MiB [0.00] Reserved memory: reserved region for node 'splash_region@9d60': base 0x9d40, size 36 MiB [0.00] Reserved memory: allocated memory for 'qseecom_region' node: base 0xfe80, size 20 MiB [0.00] Reserved memory: created CMA memory pool at 0xfe80, size 20 MiB [0.00] Reserved memory: initialized node qseecom_region, compatible id shared-dma-pool [0.00] Reserved memory: allocated memory for 'adsp_region' node: base 0xfe00, size 8 MiB [0.00] Reserved memory: created CMA memory pool at 0xfe00, size 8 MiB [0.00] Reserved memory: initialized node adsp_region, compatible id shared-dma-pool [0.00] Reserved memory: allocated memory fo
RE: [PATCH V2] blk-mq: Set request mapping to NULL in blk_mq_put_driver_tag
> >
> > Other block drivers (e.g. ib_srp, skd) do not need this to work
> > reliably.
> > It has been explained to you that the bug that you reported can be
> > fixed by modifying the mpt3sas driver. So why to fix this by modifying
> > the block layer? Additionally, what prevents that a race condition
> > occurs between the block layer clearing hctx->tags->rqs[rq->tag] and
> > scsi_host_find_tag() reading that same array element? I'm afraid that
> > this is an attempt to paper over a real problem instead of fixing the
> > root
> cause.
>
> I have to agree with Bart here, I just don't see how the mpt3sas use case
> is
> special. The change will paper over the issue in any case.
Hi Jens, Bart
One of the key requirement for iterating whole tagset using
scsi_host_find_tag is to block scsi host. Once we are done that, we should
be good. No race condition is possible if that part is taken care.
Without this patch, if driver still receive scsi command from the
hctx->tags->rqs which is really not outstanding. I am finding this is
common issue for many scsi low level drivers.
Just for example - fnic_is_abts_pending() function has below code -
for (tag = 0; tag < fnic->fnic_max_tag_id; tag++) {
sc = scsi_host_find_tag(fnic->lport->host, tag);
/*
* ignore this lun reset cmd or cmds that do not belong to
* this lun
*/
if (!sc || (lr_sc && (sc->device != lun_dev || sc ==
lr_sc)))
continue;
Above code also has similar exposure of kernel panic like driver
while accessing sc->device.
Panic is more obvious if we have add/removal of scsi device before looping
through scsi_host_find_tag().
Avoiding block layer changes is also attempted in but our problem
is to convert that code common for non-mq and mq.
Temporary to unblock this issue, We have fixed using driver
internals scsiio_tracker() instead of piggy back in scsi_command.
Kashyap
>
> --
> Jens Axboe
Re: [PATCH V2] blk-mq: Set request mapping to NULL in blk_mq_put_driver_tag
On 12/18/18 10:08 AM, Kashyap Desai wrote:
>>>
>>> Other block drivers (e.g. ib_srp, skd) do not need this to work
>>> reliably.
>>> It has been explained to you that the bug that you reported can be
>>> fixed by modifying the mpt3sas driver. So why to fix this by modifying
>>> the block layer? Additionally, what prevents that a race condition
>>> occurs between the block layer clearing hctx->tags->rqs[rq->tag] and
>>> scsi_host_find_tag() reading that same array element? I'm afraid that
>>> this is an attempt to paper over a real problem instead of fixing the
>>> root
>> cause.
>>
>> I have to agree with Bart here, I just don't see how the mpt3sas use case
>> is
>> special. The change will paper over the issue in any case.
>
> Hi Jens, Bart
>
> One of the key requirement for iterating whole tagset using
> scsi_host_find_tag is to block scsi host. Once we are done that, we should
> be good. No race condition is possible if that part is taken care.
> Without this patch, if driver still receive scsi command from the
> hctx->tags->rqs which is really not outstanding. I am finding this is
> common issue for many scsi low level drivers.
>
> Just for example - fnic_is_abts_pending() function has below code -
>
> for (tag = 0; tag < fnic->fnic_max_tag_id; tag++) {
> sc = scsi_host_find_tag(fnic->lport->host, tag);
> /*
> * ignore this lun reset cmd or cmds that do not belong to
> * this lun
> */
> if (!sc || (lr_sc && (sc->device != lun_dev || sc ==
> lr_sc)))
> continue;
>
> Above code also has similar exposure of kernel panic like driver
> while accessing sc->device.
>
> Panic is more obvious if we have add/removal of scsi device before looping
> through scsi_host_find_tag().
>
> Avoiding block layer changes is also attempted in but our problem
> is to convert that code common for non-mq and mq.
> Temporary to unblock this issue, We have fixed using driver
> internals scsiio_tracker() instead of piggy back in scsi_command.
For mq, the requests never go out of scope, they are always valid. So
the key question here is WHY they have been freed. If the queue gets killed,
then one potential solution would be to clear pointers in the tag map
belonging to that queue. That also takes it out of the hot path.
--
Jens Axboe
Re: [PATCH 33/41] scsi: osd: osd_initiator: mark expected switch fall-throughs
On 28/11/18 06:32, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Signed-off-by: Gustavo A. R. Silva
ACK-by: Boaz Harrosh
> ---
> drivers/scsi/osd/osd_initiator.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/osd/osd_initiator.c
> b/drivers/scsi/osd/osd_initiator.c
> index 60cf7c5eb880..cb26f26d5ec1 100644
> --- a/drivers/scsi/osd/osd_initiator.c
> +++ b/drivers/scsi/osd/osd_initiator.c
> @@ -1849,6 +1849,7 @@ int osd_req_decode_sense_full(struct osd_request *or,
> 32, 1, dump, sizeof(dump), true);
> OSD_SENSE_PRINT2("response_integrity [%s]\n", dump);
> }
> + /* fall through */
> case osd_sense_attribute_identification:
> {
> struct osd_sense_attributes_data_descriptor
> @@ -1879,7 +1880,7 @@ int osd_req_decode_sense_full(struct osd_request *or,
> attr_page, attr_id);
> }
> }
> - /*These are not legal for OSD*/
> + /* fall through - These are not legal for OSD */
> case scsi_sense_field_replaceable_unit:
> OSD_SENSE_PRINT2("scsi_sense_field_replaceable_unit\n");
> break;
>
Re: [PATCH 33/41] scsi: osd: osd_initiator: mark expected switch fall-throughs
On 12/18/18 11:13 AM, Boaz Harrosh wrote: On 28/11/18 06:32, Gustavo A. R. Silva wrote: In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva ACK-by: Boaz Harrosh Thank you, Boaz. -- Gustavo
RE: [PATCH V2] blk-mq: Set request mapping to NULL in blk_mq_put_driver_tag
>
> On 12/18/18 10:08 AM, Kashyap Desai wrote:
> >>>
> >>> Other block drivers (e.g. ib_srp, skd) do not need this to work
> >>> reliably.
> >>> It has been explained to you that the bug that you reported can be
> >>> fixed by modifying the mpt3sas driver. So why to fix this by
> >>> modifying the block layer? Additionally, what prevents that a race
> >>> condition occurs between the block layer clearing
> >>> hctx->tags->rqs[rq->tag] and
> >>> scsi_host_find_tag() reading that same array element? I'm afraid
> >>> that this is an attempt to paper over a real problem instead of
> >>> fixing the root
> >> cause.
> >>
> >> I have to agree with Bart here, I just don't see how the mpt3sas use
> >> case is special. The change will paper over the issue in any case.
> >
> > Hi Jens, Bart
> >
> > One of the key requirement for iterating whole tagset using
> > scsi_host_find_tag is to block scsi host. Once we are done that, we
> > should be good. No race condition is possible if that part is taken
> > care.
> > Without this patch, if driver still receive scsi command from the
> > hctx->tags->rqs which is really not outstanding. I am finding this is
> > common issue for many scsi low level drivers.
> >
> > Just for example - fnic_is_abts_pending() function has below
> > code -
> >
> > for (tag = 0; tag < fnic->fnic_max_tag_id; tag++) {
> > sc = scsi_host_find_tag(fnic->lport->host, tag);
> > /*
> > * ignore this lun reset cmd or cmds that do not belong
> > to
> > * this lun
> > */
> > if (!sc || (lr_sc && (sc->device != lun_dev || sc ==
> > lr_sc)))
> > continue;
> >
> > Above code also has similar exposure of kernel panic like
> > driver while accessing sc->device.
> >
> > Panic is more obvious if we have add/removal of scsi device before
> > looping through scsi_host_find_tag().
> >
> > Avoiding block layer changes is also attempted in but our
> > problem is to convert that code common for non-mq and mq.
> > Temporary to unblock this issue, We have fixed using driver
> > internals scsiio_tracker() instead of piggy back in scsi_command.
>
> For mq, the requests never go out of scope, they are always valid. So the
> key
> question here is WHY they have been freed. If the queue gets killed, then
> one
> potential solution would be to clear pointers in the tag map belonging to
> that
> queue. That also takes it out of the hot path.
At driver load whenever driver does scsi_add_host_with_dma(), it follows
below code path in block layer.
scsi_mq_setup_tags
->blk_mq_alloc_tag_set
-> blk_mq_alloc_rq_maps
-> __blk_mq_alloc_rq_maps
SML create two set of request pool. One is per HBA and other is per SDEV. I
was confused why SML creates request pool per HBA.
Example - IF HBA queue depth is 1K and there are 8 device behind that HBA,
total request pool is created is 1K + 8 * scsi_device queue depth. 1K will
be always static, but other request pool is managed whenever scsi device is
added/removed.
I never observe requests allocated per HBA is used in IO path. It is always
request allocated per scsi device is what active.
Also, what I observed is whenever scsi_device is deleted, associated request
is also deleted. What is missing is - "Deleted request still available in
hctx->tags->rqs[rq->tag]."
IF there is an assurance that all the request will be valid as long as hctx
is available, this patch is not correct. I posted patch based on assumption
that request per hctx can be removed whenever scsi device is removed.
Kashyap
>
> --
> Jens Axboe
Re: [PATCH V2] blk-mq: Set request mapping to NULL in blk_mq_put_driver_tag
On 12/18/18 10:48 AM, Kashyap Desai wrote:
>>
>> On 12/18/18 10:08 AM, Kashyap Desai wrote:
>
> Other block drivers (e.g. ib_srp, skd) do not need this to work
> reliably.
> It has been explained to you that the bug that you reported can be
> fixed by modifying the mpt3sas driver. So why to fix this by
> modifying the block layer? Additionally, what prevents that a race
> condition occurs between the block layer clearing
> hctx->tags->rqs[rq->tag] and
> scsi_host_find_tag() reading that same array element? I'm afraid
> that this is an attempt to paper over a real problem instead of
> fixing the root
cause.
I have to agree with Bart here, I just don't see how the mpt3sas use
case is special. The change will paper over the issue in any case.
>>>
>>> Hi Jens, Bart
>>>
>>> One of the key requirement for iterating whole tagset using
>>> scsi_host_find_tag is to block scsi host. Once we are done that, we
>>> should be good. No race condition is possible if that part is taken
>>> care.
>>> Without this patch, if driver still receive scsi command from the
>>> hctx->tags->rqs which is really not outstanding. I am finding this is
>>> common issue for many scsi low level drivers.
>>>
>>> Just for example - fnic_is_abts_pending() function has below
>>> code -
>>>
>>> for (tag = 0; tag < fnic->fnic_max_tag_id; tag++) {
>>> sc = scsi_host_find_tag(fnic->lport->host, tag);
>>> /*
>>> * ignore this lun reset cmd or cmds that do not belong
>>> to
>>> * this lun
>>> */
>>> if (!sc || (lr_sc && (sc->device != lun_dev || sc ==
>>> lr_sc)))
>>> continue;
>>>
>>> Above code also has similar exposure of kernel panic like
>>> driver while accessing sc->device.
>>>
>>> Panic is more obvious if we have add/removal of scsi device before
>>> looping through scsi_host_find_tag().
>>>
>>> Avoiding block layer changes is also attempted in but our
>>> problem is to convert that code common for non-mq and mq.
>>> Temporary to unblock this issue, We have fixed using driver
>>> internals scsiio_tracker() instead of piggy back in scsi_command.
>>
>> For mq, the requests never go out of scope, they are always valid. So the
>> key
>> question here is WHY they have been freed. If the queue gets killed, then
>> one
>> potential solution would be to clear pointers in the tag map belonging to
>> that
>> queue. That also takes it out of the hot path.
>
> At driver load whenever driver does scsi_add_host_with_dma(), it follows
> below code path in block layer.
>
> scsi_mq_setup_tags
> ->blk_mq_alloc_tag_set
> -> blk_mq_alloc_rq_maps
> -> __blk_mq_alloc_rq_maps
>
> SML create two set of request pool. One is per HBA and other is per SDEV. I
> was confused why SML creates request pool per HBA.
>
> Example - IF HBA queue depth is 1K and there are 8 device behind that HBA,
> total request pool is created is 1K + 8 * scsi_device queue depth. 1K will
> be always static, but other request pool is managed whenever scsi device is
> added/removed.
>
> I never observe requests allocated per HBA is used in IO path. It is always
> request allocated per scsi device is what active.
> Also, what I observed is whenever scsi_device is deleted, associated request
> is also deleted. What is missing is - "Deleted request still available in
> hctx->tags->rqs[rq->tag]."
So that sounds like the issue. If the device is deleted and its requests
go away, those pointers should be cleared. That's what your patch should
do, not do it for each IO.
--
Jens Axboe
Re: [GIT PULL] SCSI fixes for 4.20-rc7
The pull request you sent on Tue, 18 Dec 2018 07:57:14 -0800: > git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/ddfbab46539f2d37a9e9d357b054486b51f7dc27 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
RE: [PATCH] hpsa: add module parameter to disable irq affinity
-Original Message-
From: Hannes Reinecke [mailto:h...@suse.de]
Sent: Tuesday, December 18, 2018 1:57 AM
To: Don Brace ; Kevin Barnett - C33748
; Scott Teel - C33730 ;
Justin Lindley - C33718 ; Scott Benesh - C33703
; bader.alisa...@microchip.com; Gerry Morong -
C33720 ; Mahesh Rajashekhara - I30583
; h...@infradead.org;
j...@linux.vnet.ibm.com; joseph.szczy...@hpe.com; posw...@suse.com;
shunyong.y...@hxt-semitech.com
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH] hpsa: add module parameter to disable irq affinity
EXTERNAL EMAIL
On 12/3/18 11:35 PM, Don Brace wrote:
> The PCI_IRQ_AFFINITY flag prevents customers from changing the
> smp_affinity and smp_affinity_list entries.
>
> - add a module parameter to allow this flag to be turned
>off.
>
> - to turn off PCI_IRQ_AFFINITY:
>flag hpsa_disable_irq_affinity=1
>
> Reviewed-by: David Carroll
> Reviewed-by: Scott Teel
> Signed-off-by: Don Brace
> ---
> drivers/scsi/hpsa.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index
> c9cccf35e9d7..0aa5aa66151f 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -87,6 +87,10 @@ static int hpsa_simple_mode;
> module_param(hpsa_simple_mode, int, S_IRUGO|S_IWUSR);
> MODULE_PARM_DESC(hpsa_simple_mode,
> "Use 'simple mode' rather than 'performant mode'");
> +static bool hpsa_disable_irq_affinity;
> +module_param(hpsa_disable_irq_affinity, bool, S_IRUGO|S_IWUSR);
> +MODULE_PARM_DESC(hpsa_disable_irq_affinity,
> + "Turn off managed irq affinity. Allows smp_affinity to be
> +changed.");
>
> /* define the PCI info for the cards we can control */
> static const struct pci_device_id hpsa_pci_device_id[] = { @@
> -7389,7 +7393,7 @@ static void hpsa_setup_reply_map(struct ctlr_info *h)
>*/
> static int hpsa_interrupt_mode(struct ctlr_info *h)
> {
> - unsigned int flags = PCI_IRQ_LEGACY;
> + unsigned int flags;
> int ret;
>
> /* Some boards advertise MSI but don't really support it */ @@
> -7400,17 +7404,20 @@ static int hpsa_interrupt_mode(struct ctlr_info *h)
> case 0x40830E11:
> break;
> default:
> + flags = PCI_IRQ_MSIX;
> + if (!hpsa_disable_irq_affinity)
> + flags |= PCI_IRQ_AFFINITY;
> ret = pci_alloc_irq_vectors(h->pdev, 1, MAX_REPLY_QUEUES,
> - PCI_IRQ_MSIX | PCI_IRQ_AFFINITY);
> + flags);
> if (ret > 0) {
> h->msix_vectors = ret;
> return 0;
> }
>
> - flags |= PCI_IRQ_MSI;
> break;
> }
>
> + flags = PCI_IRQ_LEGACY | PCI_IRQ_MSI;
> ret = pci_alloc_irq_vectors(h->pdev, 1, 1, flags);
> if (ret < 0)
> return ret;
>
That completely breaks multiqueue.
You sure about this?
What is the intention here?
Cheers,
Hannes
This patch is a result of some customers complaining that they cannot
alter the smp_affinity* entries for hpsa because the vectors are managed.
The message is: write error: Input/output error
I added the module parameter to allow them to turn off the affinity flag
so they can set the mask to whatever they want, but leaves the default
behavior in-place.
Is there a better method?
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG
Nürnberg)
[ANNOUNCE v2]: Broadcom (Emulex) FC Target driver - efct
I'd like to announce the availability of the Broadcom (Emulex) FC target driver - efct. This is the 2nd round of announcement. In the first round, after discussion with community members, it was decided that the driver would consist of the following: - A SLI4-library that can be used by both an initiator driver and a target driver - A FC Discovery library that could be used by both an initiator driver and a target driver. - A SLI3-library that would be used by an initiator driver only - Delivery of a target driver that would support SCSI and NVME - Delivery of an initiator driver that would support SCSI and NVME (NVME on SLI-4 only). As of this announcement the repository on gitlab (g...@gitlab.com:jsmart/efct-Emulex_FC_Target.git) contains: - the SLI-4 library - the FC Discovery library - the efct target driver that supports SCSI (lio) Effort is underway on adding nvme target support to the efct driver, and after that, the initiator driver will be refactored. The intent will be to move all elements into the upstream tree although it is expected the libraries and target components will be merged in a stage separate from the initiator components. While those efforts are underway, we would like receive review comments on the code that is present. Comments can be sent to: ecd-efct@broadcom.com or myself -- james On 2/27/2017 3:28 PM, James Smart wrote: I'd like to announce the availability of the Broadcom (Emulex) FC Target driver - efct. This driver has been part of the Emulex OneCore Storage SDK tool kit for Emulex SLI-4 adapters. The SLI-4 adapters support 16Gb/s and higher adapters. Although this kit has supported FCoE in the past, it is currently limited to FC support. This driver provides the following: - Target mode operation: - Functional with LIO-based interfaces - Extensive use of hw offloads such as auto-xfer_rdy, auto-rsp, cmd cpu spreading - High login mode - thousands of logins - T-10 DIF/PI support (inline and separate) - NPIV support - Concurrent Initiator support if needed - Discovery engine has F_Port and fabric services emulation. - Extended mgmt interfaces: - firmware dump api, including dump to host memory for faster dumps - Healthcheck operations and watchdogs - Extended driver behaviors such as: - polled mode operation - multi-queue: cpu, roundrobin, or priority (but not tied to scsi-mq) - long chained sgl's - extensive internal logging and statistics - Tuning parameters on modes and resource allocation to different features Broadcom is looking to upstream this driver and would like review and feedback. The driver may be found at the following git repository: g...@gitlab.com:jsmart/efct-Emulex_FC_Target.git Some of the key questions we have are with lpfc : 1) Coexistence vs integration Currently, the efct driver maps to a different set of PCI ids than lpfc. It's very clear there's an overlap with lpfc, both on SLI-4 hw as well as initiator support. Although target mode support can be simplistically added to lpfc, what we've found is that doing so means a lot of tradeoffs. Some of the target mode features, when enabled, impact the initiator support and how it would operate. 2) SLI-3 support lpfc provides SLI-3 support so that all FC adapters are supported, including the older ones. The form of the driver, based on its history, is SLI-3 with SLI-3 adapted to SLI-4 at the point it hits the hardware. efct does not support SLI-3. 3) complexity of configuration knobs caused by the kitchen-sink of features in lpfc ? we are pushing the limit on needing per-instance attributes rather than global module parameters. -- james On 6/12/2017 4:08 PM, James Smart wrote: On 5/16/2017 12:59 PM, Roland Dreier wrote: On Sun, Mar 5, 2017 at 8:35 AM, Sebastian Herbszt wrote: Just like Hannes I do favour integration. I guess it could be comparable to qla2xxx + tcm_qla2xxx, lpfc + lpfc_scst and lpfc + tcm_lpfc. That approach might even help Bart with his target driver unification if he didn't give up on that topic. Resurrecting this old topic - sorry for not seeing this go by initially. For context, I have a lot of experience debugging the qla2xxx target code - we did a lot of work to get error/exception paths correct. Basic FC target support is pretty straightforward but handling SAN log in / log out events and other strange things that initiators do took a lot of effort. Anyway, my feeling is that the integration of tcm_qla2xxx and qla2xxx was overall a net negative. Having the target driver grafted onto the side of an already-complex driver that has a bunch of code not relevant to the target (SCSI error handling, logging into and timing out remote target ports, etc) made the code harder to debug and harder to get right. Of course I'm in favor of making common code really common. So certainly we should have a common library of SLI-4 code that both the initiator and target driver share. And i
RE: [PATCH V2] blk-mq: Set request mapping to NULL in blk_mq_put_driver_tag
> On 12/18/18 10:48 AM, Kashyap Desai wrote:
> >>
> >> On 12/18/18 10:08 AM, Kashyap Desai wrote:
> >
> > Other block drivers (e.g. ib_srp, skd) do not need this to work
> > reliably.
> > It has been explained to you that the bug that you reported can be
> > fixed by modifying the mpt3sas driver. So why to fix this by
> > modifying the block layer? Additionally, what prevents that a race
> > condition occurs between the block layer clearing
> > hctx->tags->rqs[rq->tag] and
> > scsi_host_find_tag() reading that same array element? I'm afraid
> > that this is an attempt to paper over a real problem instead of
> > fixing the root
> cause.
>
> I have to agree with Bart here, I just don't see how the mpt3sas
> use case is special. The change will paper over the issue in any
> case.
> >>>
> >>> Hi Jens, Bart
> >>>
> >>> One of the key requirement for iterating whole tagset using
> >>> scsi_host_find_tag is to block scsi host. Once we are done that, we
> >>> should be good. No race condition is possible if that part is taken
> >>> care.
> >>> Without this patch, if driver still receive scsi command from the
> >>> hctx->tags->rqs which is really not outstanding. I am finding this
> >>> hctx->tags->is
> >>> common issue for many scsi low level drivers.
> >>>
> >>> Just for example - fnic_is_abts_pending() function has below
> >>> code -
> >>>
> >>> for (tag = 0; tag < fnic->fnic_max_tag_id; tag++) {
> >>> sc = scsi_host_find_tag(fnic->lport->host, tag);
> >>> /*
> >>> * ignore this lun reset cmd or cmds that do not
> >>> belong to
> >>> * this lun
> >>> */
> >>> if (!sc || (lr_sc && (sc->device != lun_dev || sc ==
> >>> lr_sc)))
> >>> continue;
> >>>
> >>> Above code also has similar exposure of kernel panic like
> >>> driver while accessing sc->device.
> >>>
> >>> Panic is more obvious if we have add/removal of scsi device before
> >>> looping through scsi_host_find_tag().
> >>>
> >>> Avoiding block layer changes is also attempted in but our
> >>> problem is to convert that code common for non-mq and mq.
> >>> Temporary to unblock this issue, We have fixed using
> >>> driver internals scsiio_tracker() instead of piggy back in
> >>> scsi_command.
> >>
> >> For mq, the requests never go out of scope, they are always valid. So
> >> the key question here is WHY they have been freed. If the queue gets
> >> killed, then one potential solution would be to clear pointers in the
> >> tag map belonging to that queue. That also takes it out of the hot
> >> path.
> >
> > At driver load whenever driver does scsi_add_host_with_dma(), it
> > follows below code path in block layer.
> >
> > scsi_mq_setup_tags
> > ->blk_mq_alloc_tag_set
> > -> blk_mq_alloc_rq_maps
> > -> __blk_mq_alloc_rq_maps
> >
> > SML create two set of request pool. One is per HBA and other is per
> > SDEV. I was confused why SML creates request pool per HBA.
> >
> > Example - IF HBA queue depth is 1K and there are 8 device behind that
> > HBA, total request pool is created is 1K + 8 * scsi_device queue
> > depth. 1K will be always static, but other request pool is managed
> > whenever scsi device is added/removed.
> >
> > I never observe requests allocated per HBA is used in IO path. It is
> > always request allocated per scsi device is what active.
> > Also, what I observed is whenever scsi_device is deleted, associated
> > request is also deleted. What is missing is - "Deleted request still
> > available in
> > hctx->tags->rqs[rq->tag]."
>
> So that sounds like the issue. If the device is deleted and its requests
> go away,
> those pointers should be cleared. That's what your patch should do, not do
> it
> for each IO.
At the time of device removal, it requires reverse traversing. Find out if
each requests associated with sdev is part of hctx->tags->rqs() and clear
that entry.
Not sure about atomic traverse if more than one device removal is happening
in parallel. May be more error prone. ?
Just wondering both the way we will be removing invalid request from array.
Are you suspecting any performance issue if we do it per IO ?
Kashyap
>
>
> --
> Jens Axboe
Re: [PATCH V2] blk-mq: Set request mapping to NULL in blk_mq_put_driver_tag
On 12/18/18 11:08 AM, Kashyap Desai wrote:
>> On 12/18/18 10:48 AM, Kashyap Desai wrote:
On 12/18/18 10:08 AM, Kashyap Desai wrote:
>>>
>>> Other block drivers (e.g. ib_srp, skd) do not need this to work
>>> reliably.
>>> It has been explained to you that the bug that you reported can be
>>> fixed by modifying the mpt3sas driver. So why to fix this by
>>> modifying the block layer? Additionally, what prevents that a race
>>> condition occurs between the block layer clearing
>>> hctx->tags->rqs[rq->tag] and
>>> scsi_host_find_tag() reading that same array element? I'm afraid
>>> that this is an attempt to paper over a real problem instead of
>>> fixing the root
>> cause.
>>
>> I have to agree with Bart here, I just don't see how the mpt3sas
>> use case is special. The change will paper over the issue in any
>> case.
>
> Hi Jens, Bart
>
> One of the key requirement for iterating whole tagset using
> scsi_host_find_tag is to block scsi host. Once we are done that, we
> should be good. No race condition is possible if that part is taken
> care.
> Without this patch, if driver still receive scsi command from the
> hctx->tags->rqs which is really not outstanding. I am finding this
> hctx->tags->is
> common issue for many scsi low level drivers.
>
> Just for example - fnic_is_abts_pending() function has below
> code -
>
> for (tag = 0; tag < fnic->fnic_max_tag_id; tag++) {
> sc = scsi_host_find_tag(fnic->lport->host, tag);
> /*
> * ignore this lun reset cmd or cmds that do not
> belong to
> * this lun
> */
> if (!sc || (lr_sc && (sc->device != lun_dev || sc ==
> lr_sc)))
> continue;
>
> Above code also has similar exposure of kernel panic like
> driver while accessing sc->device.
>
> Panic is more obvious if we have add/removal of scsi device before
> looping through scsi_host_find_tag().
>
> Avoiding block layer changes is also attempted in but our
> problem is to convert that code common for non-mq and mq.
> Temporary to unblock this issue, We have fixed using
> driver internals scsiio_tracker() instead of piggy back in
> scsi_command.
For mq, the requests never go out of scope, they are always valid. So
the key question here is WHY they have been freed. If the queue gets
killed, then one potential solution would be to clear pointers in the
tag map belonging to that queue. That also takes it out of the hot
path.
>>>
>>> At driver load whenever driver does scsi_add_host_with_dma(), it
>>> follows below code path in block layer.
>>>
>>> scsi_mq_setup_tags
>>> ->blk_mq_alloc_tag_set
>>> -> blk_mq_alloc_rq_maps
>>> -> __blk_mq_alloc_rq_maps
>>>
>>> SML create two set of request pool. One is per HBA and other is per
>>> SDEV. I was confused why SML creates request pool per HBA.
>>>
>>> Example - IF HBA queue depth is 1K and there are 8 device behind that
>>> HBA, total request pool is created is 1K + 8 * scsi_device queue
>>> depth. 1K will be always static, but other request pool is managed
>>> whenever scsi device is added/removed.
>>>
>>> I never observe requests allocated per HBA is used in IO path. It is
>>> always request allocated per scsi device is what active.
>>> Also, what I observed is whenever scsi_device is deleted, associated
>>> request is also deleted. What is missing is - "Deleted request still
>>> available in
>>> hctx->tags->rqs[rq->tag]."
>>
>> So that sounds like the issue. If the device is deleted and its requests
>> go away,
>> those pointers should be cleared. That's what your patch should do, not do
>> it
>> for each IO.
>
> At the time of device removal, it requires reverse traversing. Find out if
> each requests associated with sdev is part of hctx->tags->rqs() and clear
> that entry.
> Not sure about atomic traverse if more than one device removal is happening
> in parallel. May be more error prone. ?
>
> Just wondering both the way we will be removing invalid request from array.
> Are you suspecting any performance issue if we do it per IO ?
It's an extra store, and it's a store to an area that's then now shared
between issue and completion. Those are never a good idea. Besides, it's
the kind of issue you solve in the SLOW path, not in the fast path. Since
that's doable, it would be silly to do it for every IO.
This might not matter on mpt3sas, but on more efficient hw it definitely
will.
I'm still trying to convince myself that this issue even exists. I can see
having stale entries, but those should never be busy. Why are you finding
them with the tag iteration? It must be because the tag is reused, and
you are finding it before it's re-assigned?
--
Jens Axboe
RE: [PATCH V2] blk-mq: Set request mapping to NULL in blk_mq_put_driver_tag
> > > > At the time of device removal, it requires reverse traversing. Find > > out if each requests associated with sdev is part of hctx->tags->rqs() > > and clear that entry. > > Not sure about atomic traverse if more than one device removal is > > happening in parallel. May be more error prone. ? > > > > Just wondering both the way we will be removing invalid request from > > array. > > Are you suspecting any performance issue if we do it per IO ? > > It's an extra store, and it's a store to an area that's then now shared > between > issue and completion. Those are never a good idea. Besides, it's the kind > of > issue you solve in the SLOW path, not in the fast path. Since that's > doable, it > would be silly to do it for every IO. > > This might not matter on mpt3sas, but on more efficient hw it definitely > will. Understood your primary concern is to avoid per IO and do it if no better way. > I'm still trying to convince myself that this issue even exists. I can see > having > stale entries, but those should never be busy. Why are you finding them > with > the tag iteration? It must be because the tag is reused, and you are > finding it > before it's re-assigned? Stale entries will be forever if we remove scsi devices. It is not timing issue. If memory associated with request (freed due to device removal) reused, kernel panic occurs. We have 24 Drives behind Expander and follow expander reset which will remove all 24 drives and add it back. Add and removal of all the drives happens quickly. As part of Expander reset, driver process broadcast primitive event and that requires finding all outstanding scsi command. In some cases, we need firmware restart and that path also requires tag iteration. > > -- > Jens Axboe
Re: [PATCH V2] blk-mq: Set request mapping to NULL in blk_mq_put_driver_tag
On 12/18/18 11:22 AM, Kashyap Desai wrote: >>> >>> At the time of device removal, it requires reverse traversing. Find >>> out if each requests associated with sdev is part of hctx->tags->rqs() >>> and clear that entry. >>> Not sure about atomic traverse if more than one device removal is >>> happening in parallel. May be more error prone. ? >>> >>> Just wondering both the way we will be removing invalid request from >>> array. >>> Are you suspecting any performance issue if we do it per IO ? >> >> It's an extra store, and it's a store to an area that's then now shared >> between >> issue and completion. Those are never a good idea. Besides, it's the kind >> of >> issue you solve in the SLOW path, not in the fast path. Since that's >> doable, it >> would be silly to do it for every IO. >> >> This might not matter on mpt3sas, but on more efficient hw it definitely >> will. > > Understood your primary concern is to avoid per IO and do it if no better > way. > >> I'm still trying to convince myself that this issue even exists. I can see >> having >> stale entries, but those should never be busy. Why are you finding them >> with >> the tag iteration? It must be because the tag is reused, and you are >> finding it >> before it's re-assigned? > > > Stale entries will be forever if we remove scsi devices. It is not timing > issue. If memory associated with request (freed due to device removal) > reused, kernel panic occurs. > We have 24 Drives behind Expander and follow expander reset which will > remove all 24 drives and add it back. Add and removal of all the drives > happens quickly. > As part of Expander reset, driver process broadcast primitive > event and that requires finding all outstanding scsi command. > > In some cases, we need firmware restart and that path also requires tag > iteration. I actually took a look at scsi_host_find_tag() - what I think needs fixing here is that it should not return a tag that isn't allocated. You're just looking up random stuff, that is a recipe for disaster. But even with that, there's no guarantee that the tag isn't going away. The mpt3sas use case is crap. It's iterating every tag, just in case it needs to do something to it. My suggestion would be to scrap that bad implementation and have something available for iterating busy tags instead. That'd be more appropriate and a lot more efficient that a random loop from 0..depth. If you are flushing running commands, looking up tags that aren't even active is silly and counterproductive. -- Jens Axboe
[Bug 201935] Writing data to tape broken
https://bugzilla.kernel.org/show_bug.cgi?id=201935 --- Comment #3 from Bart Van Assche (bvanass...@acm.org) --- A fix has been queued for the 4.19 stable series and should be included in a 4.19.x stable kernel soon. See also https://lore.kernel.org/lkml/20181218163929.193192...@linuxfoundation.org/. -- You are receiving this mail because: You are the assignee for the bug.
RE: [PATCH V2] blk-mq: Set request mapping to NULL in blk_mq_put_driver_tag
> > I actually took a look at scsi_host_find_tag() - what I think needs fixing > here is > that it should not return a tag that isn't allocated. > You're just looking up random stuff, that is a recipe for disaster. > But even with that, there's no guarantee that the tag isn't going away. Got your point. Let us fix in driver. > > The mpt3sas use case is crap. It's iterating every tag, just in case it > needs to do > something to it. Many drivers in scsi layer is having similar trouble. May be they are less exposed. That was a main reason, I thought to provide common fix in block layer. > > My suggestion would be to scrap that bad implementation and have > something available for iterating busy tags instead. That'd be more > appropriate and a lot more efficient that a random loop from 0..depth. > If you are flushing running commands, looking up tags that aren't even > active > is silly and counterproductive. We will address this issue through driver changes in two steps. 1. I can use driver's internal memory and will not rely on request/scsi command. Tag 0...depth loop is not in main IO path, so what we need is contention free access of the list. Having driver's internal memory and array will provide that control. 2. As you suggested best way is to use busy tag iteration. (only for blk-mq stack) Thanks for your feedback. Kashyap > > -- > Jens Axboe
Re: [PATCH] libiscsi: add lock around task lists to fix list corruption regression
Hi, Was this issue ever resolved? We are seeing this on 4.14.35. Thanks, Alan Adamson On 5/9/18 3:27 PM, Ilkka Sovanto wrote: Hi again, Chris and others! We're hitting this again on 4.14.32. Looks like the path iscsi_queuecommand -> prepd_reject/prepd_fault results in iscsi_complete_task getting called with frwd_lock held instead of the requisite back_lock which might explain the task list corruption under heavy loads. An alternate possibly problematic situation seems to be fail_mgmt_tasks which calls iscsi_complete_task while holding frwd_lock from iscsi_start_session_recovery. See also: https://github.com/open-iscsi/open-iscsi/issues/56 Attached an ugly attempt at ensuring back_lock is held instead. - Ilkka On 5 March 2017 at 22:49, Ilkka Sovanto wrote: Hi! Was running pretty nicely for a week or so, until we got this one: [534630.679965] BUG: unable to handle kernel at 0078 [534630.684035] IP: [] iscsi_xmit_task+0x29/0xc0 [534630.685724] PGD a1fcd3067 [534630.687346] Oops: 0002 [#1] [534630.688846] Modules linked in: [534630.690348] CPU: 17 PID: 7988 Comm: kworker/u56:6 Tainted: GW [534630.693327] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 41113_115728-nilsson.home.kraxel.org 04/01/2014 [534630.696404] Workqueue: iscsi_q_8 iscsi_xmitworker [534630.697948] task: 8800bab62f00 ti: 880c0d9d8000 task.ti: 880c0d9d8000 [534630.700837] RIP: 0010:[] [] iscsi_xmit_task+0x29/0xc0 [534630.703689] RSP: 0018:880c0d9dbdb0 EFLAGS: 00010246 [534630.705209] RAX: ffc3 RBX: 880fe3dfda10 RCX: dead0200 [534630.708062] RDX: RSI: 0200 RDI: 880fe3dfda10 [534630.710773] RBP: 880c0d9dbdc8 R08: 880fe3881c80 R09: [534630.713539] R10: 880fb799f710 R11: 0260 R12: 880fe3dfda10 [534630.716344] R13: R14: 880fe3881c80 R15: 880fe3dfdae0 [534630.719143] FS: () GS:880fff42() [534630.722001] CS: 0010 DS: ES: CR0: 80050033 [534630.723492] CR2: 0078 CR3: 000aa8e44000 CR4: 000406e0 [534630.726163] Stack: [534630.727622] 880fe3dfdaa8 880fe3dfda10 880fe3dfdac0 880c0d9dbe18 [534630.730906] 8171226d 880fe3dfdad0 880fe3881c00 880fe3dfdab0 [534630.767489] 880ff7d98780 880fe3dfdae0 880ffec99400 880fe3dfdae8 [534630.770543] Call Trace: [534630.771967] [] iscsi_xmitworker+0x1dd/0x310 [534630.773523] [] process_one_work+0x149/0x3e0 [534630.775084] [] worker_thread+0x69/0x470 [534630.776585] [] ? process_one_work+0x3e0/0x3e0 [534630.778131] [] ? process_one_work+0x3e0/0x3e0 [534630.779682] [] kthread+0xea/0x100 [534630.781300] [] ? kthread_create_on_node+0x1a0/0x1a0 [534630.782872] [] ret_from_fork+0x3f/0x70 [534630.784340] [] ? kthread_create_on_node+0x1a0/0x1a0 [534630.785885] Code: [534630.792699] RIP [] iscsi_xmit_task+0x29/0xc0 [534630.794266] RSP [534630.795617] CR2: 0078 [534630.798051] ---[ end trace 9d963f95212dfbe2 ]--- [534630.799788] Kernel panic - not syncing: Fatal exception in interrupt [534630.802712] Kernel Offset: disabled [534630.804304] ---[ end Kernel panic - not syncing: Fatal exception in interrupt And here's the disassembly for iscsi_xmit_task: 8170efa0 : 8170efa0: e8 0b fd 52 00 call 81c3ecb0 <__fentry__> 8170efa5: 55 push rbp 8170efa6: b8 c3 ff ff ff moveax,0xffc3 8170efab: 48 89 e5movrbp,rsp 8170efae: 41 55 push r13 8170efb0: 41 54 push r12 8170efb2: 53 push rbx 8170efb3: 48 8b 97 f0 00 00 00movrdx,QWORD PTR [rdi+0xf0] 8170efba: 4c 8b af 90 00 00 00movr13,QWORD PTR [rdi+0x90] 8170efc1: 83 e2 02andedx,0x2 8170efc4: 75 71 jne 8170f037 8170efc6: 48 89 fbmovrbx,rdi *** 8170efc9: f0 41 ff 45 78 lock inc DWORD PTR [r13+0x78] *** 8170efce: 48 8b 47 10 movrax,QWORD PTR [rdi+0x10] 8170efd2: 48 8d b8 18 01 00 00leardi,[rax+0x118] 8170efd9: e8 b2 d3 52 00 call 81c3c390 <_raw_spin_unlock_bh> 8170efde: 48 8b 43 10 movrax,QWORD PTR [rbx+0x10] 8170efe2: 4c 89 efmovrdi,r13 8170efe5: 48 8b 80 00 01 00 00movrax,QWORD PTR [rax+0x100] 8170efec: ff 90 98 00 00 00 call QWORD PTR [rax+0x98] 8170eff2: 41 89 c4movr12d,eax 8170eff5: 48 8b 43 10 movrax,QWORD PTR [rbx+0x10] 8170eff9: 48 8d b8 18 01 00 00leardi,[rax+0x11
[PATCH 2/3] smartpqi: add ofa support
From: Mahesh Rajashekhara
- when OFA event occurs, driver will stop traffic to
RAID/HBA path. Driver waits for all the outstanding
requests to complete.
- Driver sends OFA event acknowledgment to firmware.
- Driver will wait until the new firmware is up and running.
- Driver will free up the resources.
- Driver calls SIS/PQI initialization and rescans the device list.
- Driver will resume the traffic to RAID/HBA path.
Reviewed-by: Murthy Bhat
Signed-off-by: Mahesh Rajashekhara
Signed-off-by: Don Brace
---
drivers/scsi/smartpqi/smartpqi.h | 76
drivers/scsi/smartpqi/smartpqi_init.c | 587 +++--
drivers/scsi/smartpqi/smartpqi_sis.c | 13 +
drivers/scsi/smartpqi/smartpqi_sis.h |1
4 files changed, 634 insertions(+), 43 deletions(-)
diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h
index ba499a636f43..af962368818b 100644
--- a/drivers/scsi/smartpqi/smartpqi.h
+++ b/drivers/scsi/smartpqi/smartpqi.h
@@ -100,6 +100,12 @@ struct pqi_ctrl_registers {
struct pqi_device_registers pqi_registers; /* 4000h */
};
+#if ((HZ) < 1000)
+#define PQI_HZ 1000
+#else
+#define PQI_HZ (HZ)
+#endif
+
#define PQI_DEVICE_REGISTERS_OFFSET0x4000
enum pqi_io_path {
@@ -350,6 +356,10 @@ struct pqi_event_config {
#define PQI_MAX_EVENT_DESCRIPTORS 255
+#define PQI_EVENT_OFA_MEMORY_ALLOCATION0x0
+#define PQI_EVENT_OFA_QUIESCE 0x1
+#define PQI_EVENT_OFA_CANCELLED0x2
+
struct pqi_event_response {
struct pqi_iu_header header;
u8 event_type;
@@ -357,7 +367,17 @@ struct pqi_event_response {
u8 request_acknowlege : 1;
__le16 event_id;
__le32 additional_event_id;
- u8 data[16];
+ union {
+ struct {
+ __le32 bytes_requested;
+ u8 reserved[12];
+ } ofa_memory_allocation;
+
+ struct {
+ __le16 reason; /* reason for cancellation */
+ u8 reserved[14];
+ } ofa_cancelled;
+ } data;
};
struct pqi_event_acknowledge_request {
@@ -420,6 +440,25 @@ struct pqi_vendor_general_response {
};
#define PQI_VENDOR_GENERAL_CONFIG_TABLE_UPDATE 0
+#define PQI_VENDOR_GENERAL_HOST_MEMORY_UPDATE 1
+
+#define PQI_OFA_VERSION1
+#define PQI_OFA_SIGNATURE "OFA_QRM"
+#define PQI_OFA_MAX_SG_DESCRIPTORS 64
+
+#define PQI_OFA_MEMORY_DESCRIPTOR_LENGTH \
+ (offsetof(struct pqi_ofa_memory, sg_descriptor) + \
+ (PQI_OFA_MAX_SG_DESCRIPTORS * sizeof(struct pqi_sg_descriptor)))
+
+struct pqi_ofa_memory {
+ __le64 signature; /* "OFA_QRM" */
+ __le16 version;/* version of this struct(1 = 1st version) */
+ u8 reserved[62];
+ __le32 bytes_allocated;/* total allocated memory in bytes */
+ __le16 num_memory_descriptors;
+ u8 reserved1[2];
+ struct pqi_sg_descriptor sg_descriptor[1];
+};
struct pqi_aio_error_info {
u8 status;
@@ -526,6 +565,7 @@ struct pqi_raid_error_info {
#define PQI_EVENT_TYPE_HARDWARE0x2
#define PQI_EVENT_TYPE_PHYSICAL_DEVICE 0x4
#define PQI_EVENT_TYPE_LOGICAL_DEVICE 0x5
+#define PQI_EVENT_TYPE_OFA 0xfb
#define PQI_EVENT_TYPE_AIO_STATE_CHANGE0xfd
#define PQI_EVENT_TYPE_AIO_CONFIG_CHANGE 0xfe
@@ -685,6 +725,7 @@ struct pqi_encryption_info {
#define PQI_CONFIG_TABLE_SECTION_FIRMWARE_ERRATA 2
#define PQI_CONFIG_TABLE_SECTION_DEBUG 3
#define PQI_CONFIG_TABLE_SECTION_HEARTBEAT 4
+#define PQI_CONFIG_TABLE_SECTION_SOFT_RESET5
struct pqi_config_table {
u8 signature[8]; /* "CFGTABLE" */
@@ -724,8 +765,9 @@ struct pqi_config_table_firmware_features {
/* u8 features_enabled[]; */
};
-#define PQI_FIRMWARE_FEATURE_OFA 0
-#define PQI_FIRMWARE_FEATURE_SMP 1
+#define PQI_FIRMWARE_FEATURE_OFA 0
+#define PQI_FIRMWARE_FEATURE_SMP 1
+#define PQI_FIRMWARE_FEATURE_SOFT_RESET_HANDSHAKE 11
struct pqi_config_table_debug {
struct pqi_config_table_section_header header;
@@ -737,6 +779,22 @@ struct pqi_config_table_heartbeat {
__le32 heartbeat_counter;
};
+struct pqi_config_table_soft_reset {
+ struct pqi_config_table_section_header header;
+ u8 soft_reset_status;
+};
+
+#define PQI_SOFT_RESET_INITIATE0x1
+#define PQI_SOFT_RESET_ABORT 0x2
+
+enum pqi_soft_reset_status {
+ RESET_INITIATE_FIRMWARE,
+ RESET_INITIATE_DRIVER,
+ RESET_ABORT,
+ RESET_NORESPONSE,
+ RESET_TIMEDOUT
+};
+
union pqi_reset_register {
struct {
u32 reset_type : 3;
@@ -1000,13 +1058,15 @@ struct pqi_io_request {
struct list_hea
[PATCH 1/3] smartpqi: increase fw status register read timeout
From: Mahesh Rajashekhara Problem: - during the driver initialization, driver will poll fw for KERNEL_UP in a 30 seconds timeout. - if the firmware is not ready after 30 seconds, driver will not be loaded. Fix: - change timeout from 30 seconds to 3 minutes. Reported-by: Feng Li Reviewed-by: Ajish Koshy Reviewed-by: Murthy Bhat Reviewed-by: Dave Carroll Reviewed-by: Kevin Barnett Signed-off-by: Mahesh Rajashekhara Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_sis.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/smartpqi/smartpqi_sis.c b/drivers/scsi/smartpqi/smartpqi_sis.c index ea91658c7060..9d3043df22af 100644 --- a/drivers/scsi/smartpqi/smartpqi_sis.c +++ b/drivers/scsi/smartpqi/smartpqi_sis.c @@ -59,7 +59,7 @@ #define SIS_CTRL_KERNEL_UP 0x80 #define SIS_CTRL_KERNEL_PANIC 0x100 -#define SIS_CTRL_READY_TIMEOUT_SECS30 +#define SIS_CTRL_READY_TIMEOUT_SECS180 #define SIS_CTRL_READY_RESUME_TIMEOUT_SECS 90 #define SIS_CTRL_READY_POLL_INTERVAL_MSECS 10
[ANNOUNCE] v4 sg driver: ready for testing
After an underwhelming response to my intermediate level patchsets to modernize the sg driver in October this year (see "[PATCH 0/8] sg: major cleanup, remove max_queue limit" followed by v2 and v3 between 20181019 and 20181028), I decided to move ahead and add the functionality proposed for the version 4 sg driver. That means accepting interface objects of type 'struct sg_io_v4' (as found in include/uapi/linux/bsg) plus two new ioctls: SG_IOSUBMIT and SG_IORECEIVE as proposed by Linus Torvalds to replace the unloved write(2)/read(2) asynchronous interface . There is a new feature called "sharing" explained in the web page (see below). Yes, there is a patchset available (14 part and growing) but even without explanatory comments at the top of each patch, that patchset is 4 times larger than the v4 sg driver (i.e. the finished product) and over 6 times larger than the original v3 sg driver! Part of the reason for the patchset size is the multiple backtracks and rewrites associated with a real development process. The cleanest patchset would have 3 parts: 1) split the current include/scsi/sg.h into the end product headers: include/uapi/scsi/sg.h and include/scsi/sg.h 2) delete drivers/scsi/sg.c 3) add the v4 drivers/scsi/sg.c After part 2) you could build a kernel and I can guarantee that no-one will be able to find any sg driver bugs but some users might get upset (but not the Linux security folks). So there is a working v4 sg driver discussed here, with a download: http://sg.danny.cz/sg/sg_v40.html I will keep that page up to date while the driver is in this phase. There is a sg3_utils beta of 1.45 (revision 799) package in the News section at the top of the main page: http://sg.danny.cz/sg/index.html That sg3_utils beta package will use the v4 sg interface via sg devices if the v4 driver is detected. There are also three test utilities in the 'testing' directory designed to exercise the v4 extensions. The degree of backward compatibility with the v3 driver should be high but there are limits to backward compatibility. As an example, it is possible that there are user apps that depend on hitting the 16 outstanding command limit (per fd) in the v3 driver and go "wild" when v4 removes that ceiling. If so, a "high_v3_compat" driver option could be added to put that ceiling back. The only way to find out is for folks to try and if there is a failure, contact me, or send mail to this list. Code reviews welcome as well. Doug Gilbert I felt this was a better use of my time than trying to invent a new debug/trace mechanism for the whole SCSI subsystem. That is what _SCSI_ system maintainers are for, I'll stick to the sg driver (and scsi_debug). Add user space tools and there is more than enough work there ...
[PATCH 3/3] smartpqi: update driver version
- need to bump up the driver version because of the OFA patch and the fw status register read timeout patch. Reviewed-by: Gerry Morong Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 863bd0d0345c..5e5ae47fb074 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -40,11 +40,11 @@ #define BUILD_TIMESTAMP #endif -#define DRIVER_VERSION "1.2.4-065" +#define DRIVER_VERSION "1.2.4-070" #define DRIVER_MAJOR 1 #define DRIVER_MINOR 2 #define DRIVER_RELEASE 4 -#define DRIVER_REVISION65 +#define DRIVER_REVISION70 #define DRIVER_NAME"Microsemi PQI Driver (v" \ DRIVER_VERSION BUILD_TIMESTAMP ")"
[PATCH 0/3] smartpqi additional patches
These patches are based on Linus's tree There are two more patches that need to be added that were not fully tested until now. The changes are: - smartpqi-increase-fw-status-register-read-timeout . wait longer for fw to fully initialize - smartpqi-add-ofa-support . allow on-line firmware updates. - smartpqi-update-driver-version . need to bump up driver version to match new patches. --- Don Brace (1): smartpqi: update driver version Mahesh Rajashekhara (2): smartpqi: increase fw status register read timeout smartpqi: add ofa support drivers/scsi/smartpqi/smartpqi.h | 76 drivers/scsi/smartpqi/smartpqi_init.c | 591 +++-- drivers/scsi/smartpqi/smartpqi_sis.c | 15 + drivers/scsi/smartpqi/smartpqi_sis.h |1 4 files changed, 637 insertions(+), 46 deletions(-) -- Signature
Re: [PATCH] virtio_scsi: Remove per-target data because it is no longer used
Bart,
> Commit b5b6e8c8d3b4 ("scsi: virtio_scsi: fix IO hang caused by
> automatic irq vector affinity") removed all virtio_scsi hostdata
> users. Since the SCSI host data is no longer used, also remove the
> host data itself.
Applied to 4.21/scsi-queue, thanks!
--
Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] scsi: mpt3sas: fix memory ordering on 64bit writes
> With revision 09c2f95ad404, 64bit writes in _base_writeq() were rewritten > to use __raw_writeq() instad of writeq(). > > This introduced a bug apparent on powerpc64 systems such as the Raptor > Talos II that causes the HBA to drop from the PCIe bus under heavy load and > being reinitialized after a couple of seconds. Broadcom folks: Please review! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH -next] scsi: target/core: Use kmem_cache_free() instead of kfree()
Wei, > memory allocated by kmem_cache_alloc() should be freed using > kmem_cache_free(), not kfree(). Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: remove the "clustering" flag V2
Christoph, > can we get this reviewed and merge before the end of the merge window? > That gets the clustering put of the way for the multipage-biovec work > from Ming which we want to land in the block tree early in the next > merge window. I have been somewhat hesitant to apply this just before the merge window. However, I did go through it fairly carefully and since it's a mostly mechanical change, I did apply it. There were a few things I needed to tweak by hand and patch 9 forgot to update myrs and myrb. I left patch 10 for Jens to merge later (unless he acks it for me to pull in). -- Martin K. Petersen Oracle Linux Engineering
Re: [RESEND PATCH v2] megaraid: fix out-of-bound array accesses
> diff --git a/drivers/scsi/megaraid/megaraid_sas_fp.c
> b/drivers/scsi/megaraid/megaraid_sas_fp.c
> index 59ecbb3b53b5..a33628550425 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_fp.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_fp.c
> @@ -1266,7 +1266,7 @@ void mr_update_load_balance_params(struct
> MR_DRV_RAID_MAP_ALL *drv_map,
>
> for (ldCount = 0; ldCount < MAX_LOGICAL_DRIVES_EXT; ldCount++) {
> ld = MR_TargetIdToLdGet(ldCount, drv_map);
> - if (ld >= MAX_LOGICAL_DRIVES_EXT) {
> + if (ld >= MAX_LOGICAL_DRIVES_EXT - 1) {
> lbInfo[ldCount].loadBalanceFlag = 0;
> continue;
> }
> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> index f74b5ea24f0f..49eaa87608f6 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> @@ -2832,7 +2832,7 @@ static void megasas_build_ld_nonrw_fusion(struct
> megasas_instance *instance,
> device_id < instance->fw_supported_vd_count)) {
>
> ld = MR_TargetIdToLdGet(device_id, local_map_ptr);
> - if (ld >= instance->fw_supported_vd_count)
> + if (ld >= instance->fw_supported_vd_count - 1)
> fp_possible = 0;
> else {
> raid = MR_LdRaidGet(ld, local_map_ptr);
Kashyap, Sumit, Shivasharan: Please review!
--
Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] scsi: lpfc: do not set queue->page_count to 0 if pc_sli4_params.wqpcnt is invalid
Ewan, > Certain older adapters such as the OneConnect OCe10100 may not have a > valid wqpcnt value. In this case, do not set queue->page_count to 0 > in lpfc_sli4_queue_alloc() as this will prevent the driver from > initializing. Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: remove the "clustering" flag V2
On 12/18/18 9:23 PM, Martin K. Petersen wrote: > > Christoph, > >> can we get this reviewed and merge before the end of the merge window? >> That gets the clustering put of the way for the multipage-biovec work >> from Ming which we want to land in the block tree early in the next >> merge window. > > I have been somewhat hesitant to apply this just before the merge > window. However, I did go through it fairly carefully and since it's a > mostly mechanical change, I did apply it. > > There were a few things I needed to tweak by hand and patch 9 forgot to > update myrs and myrb. > > I left patch 10 for Jens to merge later (unless he acks it for me to > pull in). You can add my reviewed-by to 10 and add that one too, that makes more sense than me adding it for a post-merge pull. But either way works for me. -- Jens Axboe
Re: [PATCH v5 1/2] scsi: hisi_sas: Add support for DIF feature for v2 hw
John, > From: Xiang Chen > > For v3 hw, we support DIF operation for SAS, but not SATA. > > In addition, DIF CRC16 is supported. > > This patchset adds the SW support for the described features. The main > components are as follows: > - Get protection mask from module param > - Fill PI fields > - Fill related to DIF in DQ and protection iu memories Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: remove the "clustering" flag V2
Jens, > You can add my reviewed-by to 10 and add that one too, that makes more > sense than me adding it for a post-merge pull. But either way works > for me. OK, done. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 00/41] scsi: Mark expected switch fall-throughs
On 12/18/18 9:45 PM, Martin K. Petersen wrote: If you haven't received feedback on a patch you should poke the relevant driver maintainer. Got it. Will do so. Thanks -- Gustavo
Re: [PATCH] scsi: mpt3sas: fix memory ordering on 64bit writes
Please consider this patch as Acked-by: Sreekanth Reddy On Wed, Dec 19, 2018 at 9:22 AM Martin K. Petersen wrote: > > > > With revision 09c2f95ad404, 64bit writes in _base_writeq() were rewritten > > to use __raw_writeq() instad of writeq(). > > > > This introduced a bug apparent on powerpc64 systems such as the Raptor > > Talos II that causes the HBA to drop from the PCIe bus under heavy load and > > being reinitialized after a couple of seconds. > > Broadcom folks: Please review! > > -- > Martin K. Petersen Oracle Linux Engineering
Re: remove the "clustering" flag V2
On Tue, Dec 18, 2018 at 11:23:32PM -0500, Martin K. Petersen wrote: > > Christoph, > > > can we get this reviewed and merge before the end of the merge window? > > That gets the clustering put of the way for the multipage-biovec work > > from Ming which we want to land in the block tree early in the next > > merge window. > > I have been somewhat hesitant to apply this just before the merge > window. However, I did go through it fairly carefully and since it's a > mostly mechanical change, I did apply it. > > There were a few things I needed to tweak by hand and patch 9 forgot to > update myrs and myrb. Hmm, there should have been two patches to drop these flags entirely for those, given that the DAC960 driver didn't have any such limitations. Not sure why those got skipped. But I'll send follow ons for that.
remove dma_boundary limits for ex-DAC960 drivers
These drivers didn't bother to enable clustering, which allows merges across pages. But the original block driver had not such limitation, so there should be no need to set these limits either.
[PATCH 1/2] myrb: remove the dma_boundary limit
The old DAC960 drivers was fine with merging over segment bondaries,
so this new driver should be to.
Signed-off-by: Christoph Hellwig
---
drivers/scsi/myrb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
index 1d6dbd77bd0e..aeb282f617c5 100644
--- a/drivers/scsi/myrb.c
+++ b/drivers/scsi/myrb.c
@@ -2236,7 +2236,6 @@ struct scsi_host_template myrb_template = {
.shost_attrs= myrb_shost_attrs,
.sdev_attrs = myrb_sdev_attrs,
.this_id= -1,
- .dma_boundary = PAGE_SIZE - 1,
};
/**
--
2.19.2
[PATCH 2/2] myrs: remove the dma_boundary_limit
The old DAC960 drivers was fine with merging over segment bondaries,
so this new driver should be to.
Signed-off-by: Christoph Hellwig
---
drivers/scsi/myrs.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/scsi/myrs.c b/drivers/scsi/myrs.c
index 76a04cedfc83..0264a2e2bc19 100644
--- a/drivers/scsi/myrs.c
+++ b/drivers/scsi/myrs.c
@@ -1929,7 +1929,6 @@ struct scsi_host_template myrs_template = {
.shost_attrs= myrs_shost_attrs,
.sdev_attrs = myrs_sdev_attrs,
.this_id= -1,
- .dma_boundary = PAGE_SIZE - 1,
};
static struct myrs_hba *myrs_alloc_host(struct pci_dev *pdev,
--
2.19.2
