Re: [PATCH v2] qla2xxx: Fix race condition between iocb timeout and initialisation
On Wed, 2018-03-21 at 17:55 +, Madhani, Himanshu wrote: > Hi Ben, > > > On Mar 21, 2018, at 10:45 AM, Ben Hutchings > > wrote: > > > > On Wed, 2018-03-21 at 17:17 +, Madhani, Himanshu wrote: > > > Hi Ben, > > > > > > > On Mar 20, 2018, at 2:36 PM, Ben Hutchings > > > > wrote: > > > > > > > > qla2x00_init_timer() calls add_timer() on the iocb timeout timer, > > > > which means the timeout function pointer and any data that the > > > > function depends on must be initialised beforehand. > > > > > > > > Move this initialisation before each call to qla2x00_init_timer(). In > > > > some cases qla2x00_init_timer() initialises a completion structure > > > > needed by the timeout function, so move the call to add_timer() after > > > > that. > > > > [...] > > > What motivated this patch? are you debugging any crash which was > > > helped by moving code around? > > > > I saw the recent fix that added a call to del_timer(), noticed the lack > > of synchronisation and then kept looking further. > > > > > What I see from this patch is that its moving iocb.cmd.timeout field > > > before qla2x00_init_timer(), > > > which are completely different from each other. > > > > How are they "completely different"? qla2x00_init_timer() starts a > > timer that will call qla2x00_sp_timeout(), which in turn uses the > > iocb_cmd.timeout function pointer. We should not assume that any > > initialisation code after the call to add_timer() will run before the > > timer expires, since the scheduler might run other tasks for the whole > > time. It's unlikely but not impossible. > > > > I see. I used “completely different” due to the lack of better wording. > > I wanted to get context and understand if there was any issue that would > have helped with these changes. > > your explanation does help and I agree that there is window where timer() > will > pop before timeout is initialized. > > Let me put this patch in our test cycle for couple days and will respond on > this one Thanks. Ben. -- Ben Hutchings Software Developer, Codethink Ltd.
Re: [PATCH v2] qla2xxx: Fix race condition between iocb timeout and initialisation
On Wed, 2018-03-21 at 17:17 +, Madhani, Himanshu wrote: > Hi Ben, > > > On Mar 20, 2018, at 2:36 PM, Ben Hutchings > > wrote: > > > > qla2x00_init_timer() calls add_timer() on the iocb timeout timer, > > which means the timeout function pointer and any data that the > > function depends on must be initialised beforehand. > > > > Move this initialisation before each call to qla2x00_init_timer(). In > > some cases qla2x00_init_timer() initialises a completion structure > > needed by the timeout function, so move the call to add_timer() after > > that. [...] > What motivated this patch? are you debugging any crash which was > helped by moving code around? I saw the recent fix that added a call to del_timer(), noticed the lack of synchronisation and then kept looking further. > What I see from this patch is that its moving iocb.cmd.timeout field > before qla2x00_init_timer(), > which are completely different from each other. How are they "completely different"? qla2x00_init_timer() starts a timer that will call qla2x00_sp_timeout(), which in turn uses the iocb_cmd.timeout function pointer. We should not assume that any initialisation code after the call to add_timer() will run before the timer expires, since the scheduler might run other tasks for the whole time. It's unlikely but not impossible. Ben. -- Ben Hutchings Software Developer, Codethink Ltd.
[RFC][PATCH] qla2xxx: Fix race between iocb timeout and free
qla2x00_sp_free() cancels the iocb timeout timer if it is still
pending, but doesn't (and can't) wait for the timer function to
complete if it is already running. Add a reference count to async
iocb commands to ensure that they aren't freed too early:
- One reference is held by the timer, and dropped either at the end
of the timer function or after the timer is cancelled
- One reference is held by the completion path, and dropped by
qla2x00_sp_free()
Signed-off-by: Ben Hutchings
---
This probably isn't quite right, since it's based on only a brief code
review and is untested. And maybe there's some reason that this race
condition is somehow avoided.
This depends on the previous two fixes I sent for qla2xxx.
Ben.
drivers/scsi/qla2xxx/qla_def.h| 1 +
drivers/scsi/qla2xxx/qla_gs.c | 4 ++--
drivers/scsi/qla2xxx/qla_init.c | 10 +++---
drivers/scsi/qla2xxx/qla_inline.h | 12
drivers/scsi/qla2xxx/qla_iocb.c | 6 ++
5 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index c9689f97c307..0337bacd0dc7 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -483,6 +483,7 @@ struct srb_iocb {
struct timer_list timer;
void (*timeout)(void *);
+ refcount_t ref;
};
/* Values for srb_ctx type */
diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index fcde5ea203c0..e98ba70b7cbe 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -545,7 +545,7 @@ static void qla2x00_async_sns_sp_done(void *s, int rc)
if (!e)
goto err2;
- del_timer(&sp->u.iocb_cmd.timer);
+ qla2x00_del_timer(sp);
e->u.iosb.sp = sp;
qla2x00_post_work(vha, e);
return;
@@ -4175,7 +4175,7 @@ void qla24xx_async_gpnft_done(scsi_qla_host_t *vha, srb_t
*sp)
{
ql_dbg(ql_dbg_disc, vha, 0x,
"%s enter\n", __func__);
- del_timer(&sp->u.iocb_cmd.timer);
+ qla2x00_del_timer(sp);
qla24xx_async_gnnft(vha, sp, sp->gen2);
}
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 45319119606a..ecdb78924ca8 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -60,6 +60,9 @@ qla2x00_sp_timeout(struct timer_list *t)
iocb = &sp->u.iocb_cmd;
iocb->timeout(sp);
spin_unlock_irqrestore(&vha->hw->hardware_lock, flags);
+
+ if (refcount_dec_and_test(&iocb->ref))
+ qla2x00_rel_sp(sp);
}
void
@@ -68,8 +71,9 @@ qla2x00_sp_free(void *ptr)
srb_t *sp = ptr;
struct srb_iocb *iocb = &sp->u.iocb_cmd;
- del_timer(&iocb->timer);
- qla2x00_rel_sp(sp);
+ qla2x00_del_timer(sp);
+ if (refcount_dec_and_test(&iocb->ref))
+ qla2x00_rel_sp(sp);
}
/* Asynchronous Login/Logout Routines -- */
@@ -1553,7 +1557,7 @@ qla24xx_abort_sp_done(void *ptr, int res)
srb_t *sp = ptr;
struct srb_iocb *abt = &sp->u.iocb_cmd;
- if (del_timer(&sp->u.iocb_cmd.timer))
+ if (qla2x00_del_timer(sp))
complete(&abt->u.abt.comp);
}
diff --git a/drivers/scsi/qla2xxx/qla_inline.h
b/drivers/scsi/qla2xxx/qla_inline.h
index 06c4a843e2ad..75af2c4bb92f 100644
--- a/drivers/scsi/qla2xxx/qla_inline.h
+++ b/drivers/scsi/qla2xxx/qla_inline.h
@@ -277,9 +277,21 @@ qla2x00_init_timer(srb_t *sp, unsigned long tmo)
init_completion(&sp->u.iocb_cmd.u.fxiocb.fxiocb_comp);
if (sp->type == SRB_ELS_DCMD)
init_completion(&sp->u.iocb_cmd.u.els_logo.comp);
+ refcount_set(&sp->u.iocb_cmd.ref, 2);
add_timer(&sp->u.iocb_cmd.timer);
}
+static inline int
+qla2x00_del_timer(srb_t *sp)
+{
+ int rval;
+
+ rval = del_timer(&sp->u.iocb_cmd.timer);
+ if (rval)
+ refcount_dec(&sp->u.iocb_cmd.ref);
+ return rval;
+}
+
static inline int
qla2x00_gid_list_size(struct qla_hw_data *ha)
{
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 6a719c1f8af5..bfde6ebc30d3 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -2384,8 +2384,7 @@ qla2x00_els_dcmd_sp_free(void *data)
elsio->u.els_logo.els_logo_pyld,
elsio->u.els_logo.els_logo_pyld_dma);
- del_timer(&elsio->timer);
- qla2x00_rel_sp(sp);
+ qla2x00_sp_free(sp);
}
static void
@@ -2581,8 +2580,7 @@ qla2x00_els_dcmd2_sp_free(void *data)
elsio->u.els_plogi.els_resp_pyld,
elsio->u.els_plogi.els_resp_pyld_dma);
- del_timer(&elsio->timer);
- qla2x00_rel_sp(sp);
+ qla2x00_sp_free(sp);
}
static void
--
2.15.0.rc0
[PATCH v2] qla2xxx: Fix race condition between iocb timeout and initialisation
qla2x00_init_timer() calls add_timer() on the iocb timeout timer, which means the timeout function pointer and any data that the function depends on must be initialised beforehand. Move this initialisation before each call to qla2x00_init_timer(). In some cases qla2x00_init_timer() initialises a completion structure needed by the timeout function, so move the call to add_timer() after that. Signed-off-by: Ben Hutchings --- Changes from v1: Rebased to remove textual dependency on a patch I haven't yet sent (oops). Ben. drivers/scsi/qla2xxx/qla_gs.c | 12 +++- drivers/scsi/qla2xxx/qla_init.c | 37 +++-- drivers/scsi/qla2xxx/qla_inline.h | 2 +- drivers/scsi/qla2xxx/qla_iocb.c | 8 +--- drivers/scsi/qla2xxx/qla_mbx.c| 8 drivers/scsi/qla2xxx/qla_mid.c| 2 +- drivers/scsi/qla2xxx/qla_mr.c | 5 +++-- drivers/scsi/qla2xxx/qla_target.c | 2 +- 8 files changed, 45 insertions(+), 31 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c index 403fa096f8c8..fcde5ea203c0 100644 --- a/drivers/scsi/qla2xxx/qla_gs.c +++ b/drivers/scsi/qla2xxx/qla_gs.c @@ -3796,6 +3796,7 @@ int qla24xx_async_gffid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->gen1 = fcport->rscn_gen; sp->gen2 = fcport->login_gen; + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); /* CT_IU preamble */ @@ -3814,7 +3815,6 @@ int qla24xx_async_gffid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->u.iocb_cmd.u.ctarg.rsp_size = GFF_ID_RSP_SIZE; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla24xx_async_gffid_sp_done; rval = qla2x00_start_sp(sp); @@ -4121,6 +4121,8 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha, struct srb *sp, sp->name = "gnnft"; sp->gen1 = vha->hw->base_qpair->chip_reset; sp->gen2 = fc4_type; + + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); memset(sp->u.iocb_cmd.u.ctarg.rsp, 0, sp->u.iocb_cmd.u.ctarg.rsp_size); @@ -4137,7 +4139,6 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha, struct srb *sp, sp->u.iocb_cmd.u.ctarg.req_size = GNN_FT_REQ_SIZE; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_gpnft_gnnft_sp_done; rval = qla2x00_start_sp(sp); @@ -4210,6 +4211,8 @@ int qla24xx_async_gpnft(scsi_qla_host_t *vha, u8 fc4_type) sp->name = "gpnft"; sp->gen1 = vha->hw->base_qpair->chip_reset; sp->gen2 = fc4_type; + + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); sp->u.iocb_cmd.u.ctarg.req = dma_zalloc_coherent(&vha->hw->pdev->dev, @@ -4248,7 +4251,6 @@ int qla24xx_async_gpnft(scsi_qla_host_t *vha, u8 fc4_type) sp->u.iocb_cmd.u.ctarg.rsp_size = rspsz; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_gpnft_gnnft_sp_done; rval = qla2x00_start_sp(sp); @@ -4356,6 +4358,7 @@ int qla24xx_async_gnnid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->gen1 = fcport->rscn_gen; sp->gen2 = fcport->login_gen; + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); /* CT_IU preamble */ @@ -4377,7 +4380,6 @@ int qla24xx_async_gnnid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->u.iocb_cmd.u.ctarg.rsp_size = GNN_ID_RSP_SIZE; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_gnnid_sp_done; rval = qla2x00_start_sp(sp); @@ -4493,6 +4495,7 @@ int qla24xx_async_gfpnid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->gen1 = fcport->rscn_gen; sp->gen2 = fcport->login_gen; + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); /* CT_IU preamble */ @@ -4514,7 +4517,6 @@ int qla24xx_async_gfpnid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->u.iocb_cmd.u.ctarg.rsp_size = GFPN_ID_RSP_SIZE; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_gfpnid_sp_done; rval = qla2x00_start_sp(sp); diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/
[PATCH] qla2xxx: Fix race condition between iocb timeout and initialisation
qla2x00_init_timer() calls add_timer() on the iocb timeout timer, which means the timeout function pointer and any data that the function depends on must be initialised beforehand. Move this initialisation before each call to qla2x00_init_timer(). In some cases qla2x00_init_timer() initialises a completion structure needed by the timeout function, so move the call to add_timer() after that. Signed-off-by: Ben Hutchings Cc: sta...@vger.kernel.org --- drivers/scsi/qla2xxx/qla_gs.c | 12 +++- drivers/scsi/qla2xxx/qla_init.c | 37 +++-- drivers/scsi/qla2xxx/qla_inline.h | 2 +- drivers/scsi/qla2xxx/qla_iocb.c | 8 +--- drivers/scsi/qla2xxx/qla_mbx.c| 8 drivers/scsi/qla2xxx/qla_mid.c| 2 +- drivers/scsi/qla2xxx/qla_mr.c | 5 +++-- drivers/scsi/qla2xxx/qla_target.c | 2 +- 8 files changed, 45 insertions(+), 31 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c index d4bf787a9ea2..e98ba70b7cbe 100644 --- a/drivers/scsi/qla2xxx/qla_gs.c +++ b/drivers/scsi/qla2xxx/qla_gs.c @@ -3796,6 +3796,7 @@ int qla24xx_async_gffid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->gen1 = fcport->rscn_gen; sp->gen2 = fcport->login_gen; + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); /* CT_IU preamble */ @@ -3814,7 +3815,6 @@ int qla24xx_async_gffid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->u.iocb_cmd.u.ctarg.rsp_size = GFF_ID_RSP_SIZE; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla24xx_async_gffid_sp_done; rval = qla2x00_start_sp(sp); @@ -4121,6 +4121,8 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha, struct srb *sp, sp->name = "gnnft"; sp->gen1 = vha->hw->base_qpair->chip_reset; sp->gen2 = fc4_type; + + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); memset(sp->u.iocb_cmd.u.ctarg.rsp, 0, sp->u.iocb_cmd.u.ctarg.rsp_size); @@ -4137,7 +4139,6 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha, struct srb *sp, sp->u.iocb_cmd.u.ctarg.req_size = GNN_FT_REQ_SIZE; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_gpnft_gnnft_sp_done; rval = qla2x00_start_sp(sp); @@ -4210,6 +4211,8 @@ int qla24xx_async_gpnft(scsi_qla_host_t *vha, u8 fc4_type) sp->name = "gpnft"; sp->gen1 = vha->hw->base_qpair->chip_reset; sp->gen2 = fc4_type; + + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); sp->u.iocb_cmd.u.ctarg.req = dma_zalloc_coherent(&vha->hw->pdev->dev, @@ -4248,7 +4251,6 @@ int qla24xx_async_gpnft(scsi_qla_host_t *vha, u8 fc4_type) sp->u.iocb_cmd.u.ctarg.rsp_size = rspsz; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_gpnft_gnnft_sp_done; rval = qla2x00_start_sp(sp); @@ -4356,6 +4358,7 @@ int qla24xx_async_gnnid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->gen1 = fcport->rscn_gen; sp->gen2 = fcport->login_gen; + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); /* CT_IU preamble */ @@ -4377,7 +4380,6 @@ int qla24xx_async_gnnid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->u.iocb_cmd.u.ctarg.rsp_size = GNN_ID_RSP_SIZE; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_gnnid_sp_done; rval = qla2x00_start_sp(sp); @@ -4493,6 +4495,7 @@ int qla24xx_async_gfpnid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->gen1 = fcport->rscn_gen; sp->gen2 = fcport->login_gen; + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); /* CT_IU preamble */ @@ -4514,7 +4517,6 @@ int qla24xx_async_gfpnid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->u.iocb_cmd.u.ctarg.rsp_size = GFPN_ID_RSP_SIZE; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_gfpnid_sp_done; rval = qla2x00_start_sp(sp); diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 608a49a419aa..ecdb78924ca8 100644 --- a/drivers/scsi/qla2xxx/ql
[PATCH] qla2xxx: Avoid double completion of abort command
qla2x00_tmf_sp_done() now deletes the timer that will run
qla2x00_tmf_iocb_timeout(), but doesn't check whether the timer
already expired. Check the return value from del_timer() to avoid
calling complete() a second time.
Fixes: 4440e46d5db7 ("[SCSI] qla2xxx: Add IOCB Abort command asynchronous ...")
Fixes: 1514839b3664 ("scsi: qla2xxx: Fix NULL pointer crash due to active ...")
Signed-off-by: Ben Hutchings
---
drivers/scsi/qla2xxx/qla_init.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 00329dda6179..b0aa8cc96f0f 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -1546,8 +1546,8 @@ qla24xx_abort_sp_done(void *ptr, int res)
srb_t *sp = ptr;
struct srb_iocb *abt = &sp->u.iocb_cmd;
- del_timer(&sp->u.iocb_cmd.timer);
- complete(&abt->u.abt.comp);
+ if (del_timer(&sp->u.iocb_cmd.timer))
+ complete(&abt->u.abt.comp);
}
int
--
2.15.0.rc0
Re: scsi: sg: assorted memory corruptions
On Thu, 2018-02-01 at 08:04 +0100, Dmitry Vyukov wrote: > On Thu, Feb 1, 2018 at 7:03 AM, Douglas Gilbert wrote: > > On 2018-01-30 07:22 AM, Dmitry Vyukov wrote: [...] > > > [1:0:0:0]cd/dvd QEMU QEMU DVD-ROM 2.0. /dev/sr0 /dev/sg1 > > > > > > # readlink /sys/class/scsi_generic/sg0 > > > > > > ../../devices/pci:00/:00:01.1/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0 > > > > > > # cat /sys/class/scsi_generic/sg0/device/vendor > > > ATA > > > > > > ^ > > That subsystem is the culprit IMO, most likely libata. > > > > Until you can show this test failing on something other than an > > ATA disk, then I will treat this issue as closed. > > Hi Doug, > > Why is bug in ATA not a bug? Is it long unused by everybody? I've got > it by running qemu with default flags... If the bug is in libata then it's not on Doug to fix it since he's only maintaining sg. Ben. -- Ben Hutchings Software Developer, Codethink Ltd.
Re: [PATCH 1/1] qed: Add firmware 8.33.1.0
On Wed, 2017-10-11 at 00:57 -0700, Rahul Verma wrote: > The new qed firmware contains fixes to firmware and added > support for new features, > -Add UFP support and drop action support. > -DCQCN support for unlimited number of QP > -Add IP type to GFT filter profile. > -Added new TCP function counters. > -Support flow ID in aRFS flow. > > Signed-off-by: Rahul Verma > --- > WHENCE | 1 + > qed/qed_init_values_zipped-8.33.1.0.bin | Bin 0 -> 838612 bytes > 2 files changed, 1 insertion(+) > create mode 100755 qed/qed_init_values_zipped-8.33.1.0.bin [...] Applied; sorry for the delay. Ben. -- Ben Hutchings When in doubt, use brute force. - Ken Thompson signature.asc Description: This is a digitally signed message part
Re: [Y2038] [PATCH 3/7] scsi: bfa: improve bfa_ioc_send_enable/disable data
On Fri, 2017-11-10 at 16:37 +0100, Arnd Bergmann wrote:
> In bfa_ioc_send_enable, we use the deprecated do_gettimeofday() function
> to read the current time. This is not a problem, since the firmware
> interface is already limited to 32-bit timestamps, but it's better
> to use ktime_get_seconds() and document what the limitation is.
>
> I noticed that I did the same change in commit a5af83925363 ("bna:
> avoid writing uninitialized data into hw registers") for the ethernet
> driver. That commit also changed the "disable" funtion to initialize
> the data we pass to the firmware properly, so I'm doing the same
> thing here.
>
> Signed-off-by: Arnd Bergmann
> ---
> drivers/scsi/bfa/bfa_ioc.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
> index 256f4afaccf9..117332537763 100644
> --- a/drivers/scsi/bfa/bfa_ioc.c
> +++ b/drivers/scsi/bfa/bfa_ioc.c
> @@ -1809,13 +1809,12 @@ static void
> bfa_ioc_send_enable(struct bfa_ioc_s *ioc)
> {
> struct bfi_ioc_ctrl_req_s enable_req;
> - struct timeval tv;
>
> bfi_h2i_set(enable_req.mh, BFI_MC_IOC, BFI_IOC_H2I_ENABLE_REQ,
> bfa_ioc_portid(ioc));
> enable_req.clscode = cpu_to_be16(ioc->clscode);
> - do_gettimeofday(&tv);
> - enable_req.tv_sec = be32_to_cpu(tv.tv_sec);
> + /* unsigned 32-bit time_t overflow in y2106 */
> + enable_req.tv_sec = be32_to_cpu(ktime_get_real_seconds());
The byte order conversion should also logically be cpu_to_be32(), not
be32_to_cpu().
Ben.
> bfa_ioc_mbox_send(ioc, &enable_req, sizeof(struct bfi_ioc_ctrl_req_s));
> }
>
> @@ -1826,6 +1825,9 @@ bfa_ioc_send_disable(struct bfa_ioc_s *ioc)
>
> bfi_h2i_set(disable_req.mh, BFI_MC_IOC, BFI_IOC_H2I_DISABLE_REQ,
> bfa_ioc_portid(ioc));
> + disable_req.clscode = cpu_to_be16(ioc->clscode);
> + /* unsigned 32-bit time_t overflow in y2106 */
> + disable_req.tv_sec = be32_to_cpu(ktime_get_real_seconds());
> bfa_ioc_mbox_send(ioc, &disable_req, sizeof(struct bfi_ioc_ctrl_req_s));
> }
>
--
Ben Hutchings
Software Developer, Codethink Ltd.
[PATCH] scsi: sg: Re-fix off by one in sg_fill_request_table()
Commit 109bade9c625 ("scsi: sg: use standard lists for sg_requests")
introduced an off-by-one error in sg_ioctl(), which was fixed by
commit bd46fc406b30 ("scsi: sg: off by one in sg_ioctl()").
Unfortunately commit 4759df905a47 ("scsi: sg: factor out
sg_fill_request_table()") moved that code, and reintroduced the
bug (perhaps due to a botched rebase). Fix it again.
Fixes: 4759df905a47 ("scsi: sg: factor out sg_fill_request_table()")
Cc: sta...@vger.kernel.org
Signed-off-by: Ben Hutchings
---
drivers/scsi/sg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 0419c2298eab..aa28874e8fb9 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -837,7 +837,7 @@ sg_fill_request_table(Sg_fd *sfp, sg_req_info_t *rinfo)
val = 0;
list_for_each_entry(srp, &sfp->rq_list, entry) {
- if (val > SG_MAX_QUEUE)
+ if (val >= SG_MAX_QUEUE)
break;
rinfo[val].req_state = srp->done + 1;
rinfo[val].problem =
--
2.15.0.rc0
Re: [PATCH linux-firmware 1/1] qed: Add firmware 8.30.16.0
On Wed, 2017-09-13 at 03:46 -0700, Rahul Verma wrote: > The new qed firmware contains fixes to firmware and added > support for new features, > -Add UFP support. > -DCQCN support for unlimited number of QP > -Add IP type to GFT filter profile. > -Added new TCP function counters. > -Support flow ID in aRFS flow. > > Signed-off-by: Rahul Verma > --- > qed/qed_init_values_zipped-8.30.16.0.bin | Bin 0 -> 837008 bytes > 1 file changed, 0 insertions(+), 0 deletions(-) > create mode 100755 qed/qed_init_values_zipped-8.30.16.0.bin [...] The new file needs an entry in WHENCE. Ben. -- Ben Hutchings Humour is the best antidote to reality. signature.asc Description: This is a digitally signed message part
[PATCH 3.16 051/192] scsi: virtio_scsi: let host do exception handling
3.16.49-rc1 review patch. If anyone has any objections, please let me know.
--
From: Paolo Bonzini
commit e72c9a2a67a6400c8ef3d01d4c461dbbbfa0e1f0 upstream.
virtio_scsi tries to do exception handling after the default 30 seconds
timeout expires. However, it's better to let the host control the
timeout, otherwise with a heavy I/O load it is likely that an abort will
also timeout. This leads to fatal errors like filesystems going
offline.
Disable the 'sd' timeout and allow the host to do exception handling,
following the precedent of the storvsc driver.
Hannes has a proposal to introduce timeouts in virtio, but this provides
an immediate solution for stable kernels too.
[mkp: fixed typo]
Reported-by: Douglas Miller
Cc: "James E.J. Bottomley"
Cc: "Martin K. Petersen"
Cc: Hannes Reinecke
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Paolo Bonzini
Signed-off-by: Martin K. Petersen
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings
---
drivers/scsi/virtio_scsi.c | 12
1 file changed, 12 insertions(+)
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -686,6 +686,16 @@ static void virtscsi_target_destroy(stru
kfree(tgt);
}
+/*
+ * The host guarantees to respond to each command, although I/O
+ * latencies might be higher than on bare metal. Reset the timer
+ * unconditionally to give the host a chance to perform EH.
+ */
+static enum blk_eh_timer_return virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
+{
+ return BLK_EH_RESET_TIMER;
+}
+
static struct scsi_host_template virtscsi_host_template_single = {
.module = THIS_MODULE,
.name = "Virtio SCSI HBA",
@@ -695,6 +705,7 @@ static struct scsi_host_template virtscs
.queuecommand = virtscsi_queuecommand_single,
.eh_abort_handler = virtscsi_abort,
.eh_device_reset_handler = virtscsi_device_reset,
+ .eh_timed_out = virtscsi_eh_timed_out,
.can_queue = 1024,
.dma_boundary = UINT_MAX,
@@ -712,6 +723,7 @@ static struct scsi_host_template virtscs
.queuecommand = virtscsi_queuecommand_multi,
.eh_abort_handler = virtscsi_abort,
.eh_device_reset_handler = virtscsi_device_reset,
+ .eh_timed_out = virtscsi_eh_timed_out,
.can_queue = 1024,
.dma_boundary = UINT_MAX,
[PATCH 3.16 226/306] scsi: mpt3sas: Fix secure erase premature termination
3.16.40-rc1 review patch. If anyone has any objections, please let me know.
--
From: Andrey Grodzovsky
commit 18f6084a989ba1b38702f9af37a2e4049a924be6 upstream.
This is a work around for a bug with LSI Fusion MPT SAS2 when perfoming
secure erase. Due to the very long time the operation takes, commands
issued during the erase will time out and will trigger execution of the
abort hook. Even though the abort hook is called for the specific
command which timed out, this leads to entire device halt
(scsi_state terminated) and premature termination of the secure erase.
Set device state to busy while ATA passthrough commands are in progress.
[mkp: hand applied to 4.9/scsi-fixes, tweaked patch description]
Signed-off-by: Andrey Grodzovsky
Acked-by: Sreekanth Reddy
Cc:
Cc: Sathya Prakash
Cc: Chaitra P B
Cc: Suganath Prabu Subramani
Cc: Sreekanth Reddy
Cc: Hannes Reinecke
Signed-off-by: Martin K. Petersen
Signed-off-by: Ben Hutchings
---
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -1625,7 +1625,10 @@ _scsih_get_volume_capabilities(struct MP
return 0;
}
-
+static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
+{
+ return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);
+}
/**
* _scsih_enable_tlr - setting TLR flags
@@ -3541,6 +3544,13 @@ _scsih_qcmd(struct Scsi_Host *shost, str
scsi_print_command(scmd);
#endif
+ /*
+* Lock the device for any subsequent command until command is
+* done.
+*/
+ if (ata_12_16_cmd(scmd))
+ scsi_internal_device_block(scmd->device);
+
sas_device_priv_data = scmd->device->hostdata;
if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
scmd->result = DID_NO_CONNECT << 16;
@@ -4041,6 +4051,9 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *i
if (scmd == NULL)
return 1;
+ if (ata_12_16_cmd(scmd))
+ scsi_internal_device_unblock(scmd->device, SDEV_RUNNING);
+
mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
if (mpi_reply == NULL) {
[stable] SCSI discard fixes
This commit in 3.19: commit 7985090aa0201fa7760583f9f8e6ba41a8d4c392 Author: Martin K. Petersen Date: Fri Nov 7 00:08:13 2014 -0500 sd: disable discard_zeroes_data for UNMAP appears to fix a data corruption bug. Should it be backported to older stable branches? Similarly for this one in 4.4: commit 397737223c59e89dca7305feb6528caef8fbef84 Author: Martin K. Petersen Date: Fri Nov 13 16:46:47 2015 -0500 sd: Make discard granularity match logical block size when LBPRZ=1 (along with: commit f4327a95dd080ed6aecb185478a88ce1ee4fa3c4 Author: Martin K. Petersen Date: Sat Mar 5 17:52:02 2016 -0500 sd: Fix discard granularity when LBPRZ=1 ) Ben. -- Ben Hutchings When in doubt, use brute force. - Ken Thompson signature.asc Description: This is a digitally signed message part
Re: NULL pointer dereference: IP: [] sr_runtime_suspend+0xc/0x20 [sr_mod]
On Tue, 2016-02-09 at 20:56 +0100, Alexandre Rossi wrote:
> Hi,
>
> netconsole does not seem to work so early in the boot process this time.
>
> > As this is Linux 4.3 and not 4.4, I guess this is a different problem
> > though. Alexandre, where you able to capture the stack trace? I’d submit
> > a new bug report with this.
>
> Here is a photo. Please ping me if you need to test some debugging patches.
I'm pretty sure this crash is fixed by commit 4fd41a8552af ("SCSI: Fix NULL
pointer dereference in runtime PM"), which I've now queued up for 4.3
(though it's already in 4.4 which I'll probably upload to unstable soon).
Ben.
--
Ben Hutchings
Design a system any fool can use, and only a fool will want to use it.
signature.asc
Description: This is a digitally signed message part
Re: NULL pointer dereference: IP: [] sr_runtime_suspend+0xc/0x20 [sr_mod]
On Fri, 2015-10-16 at 09:54 +0200, Paul Menzel wrote: [...] > > BUG: unable to handle kernel NULL pointer dereference at 0014 > > IP: [] sr_runtime_suspend+0xc/0x20 [sr_mod] > > *pdpt = 3696e001 *pde = 00 > > Oops: [#1] SMB > > Modules linked in: sd_mod(+) sr_mod(+) cdrom ata_generic ohci_pci ahci > > libahci pata_amd firwire_ohci firewire_core crc_iti_t forcedeth libata > > scsi_mod ohci_hcd ehci_pci ehci_hcd usbcore usb_common fan thermal > > thermal_sys floppy(+) > > CPU: 1 PID: 73 Comm: systemd-udevd Not tainted 4.2.0-1-686-pae #1 Debian > > 4.2.3-1 > > Hardware name: Packard Bell imedia S3210/WMCP78M, BIOs P01-B2 11/06/2009 > > task: f68dd040 ti: f6988000 task.ti: f6988000 > > EIP: 0060:[] EFLAGS: 00010246 CPU: 1 > > EIP is at sr_runtime_suspend+0xc/0x20 [sr_mod] > > EAX: EBX: f6a30cd8 ECX: f6c03d2c EDX: > > ESI: EDI: f828e100 EBP: f6989ba8 ESP: f6989b88 > > DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 > > CR0: 8005003b CR2: 0014 CR3: 3696d780 CR4: 06f0 > > Stack: > > af83346c3 0001 fff5 f6a7d150 f6a30cd8 f6a30d3c > > f6989bbc c1390cb7 f6a30cd8 f8334660 f6989bd0 c1390d0f f6a30cd8 > > f8334660 f6989c0c c13916cb f694a614 f68dd040 0000 0008 > > Call Trace: > > […] ? scsi_runtime_suspend+0x63/0xa0 [scsi_mod] > > […] ? __rpm_callback+0x27/0x60 > > […] [...] > Ben Hutchings asked me to test the patch below to get more debug > information. [...] Well, that didn't help much. Paul hit another oops, this time in sd_mod but again apparently related to runtime PM. My patch only touched sr_mod. This time he sent photos of the complete oops; see <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=801925;filename=20151020_005.jpg;att=4;msg=15> and <https://bugs.debian.org/cgi-bin/bugreport.cgi?filename=20151020_006.jpg;bug=801925;att=3;msg=15> Ben. -- Ben Hutchings The first rule of tautology club is the first rule of tautology club. signature.asc Description: This is a digitally signed message part
Re: [PATCH] scsi: storvsc: Fix a bug in copy_from_bounce_buffer()
On Sun, 2015-08-09 at 02:01 +, KY Srinivasan wrote:
>
> > -Original Message-
> > From: KY Srinivasan
> > Sent: Saturday, August 8, 2015 7:55 AM
> > To: 'Ben Hutchings'
> > Cc: Long Li ; linux-scsi
> > Subject: RE: [PATCH] scsi: storvsc: Fix a bug in copy_from_bounce_buffer()
> >
> > > -Original Message-
> > > From: Ben Hutchings [mailto:b...@decadent.org.uk]
> > > Sent: Friday, August 7, 2015 12:05 PM
> > > To: KY Srinivasan
> > > Cc: Long Li ; linux-scsi
> > >
> > > Subject: Re: [PATCH] scsi: storvsc: Fix a bug in copy_from_bounce_buffer()
> > >
> > > Commit 8de580742fee ("scsi: storvsc: Fix a bug in
> > > copy_from_bounce_buffer()"), actually modified the function
> > > copy_to_bounce_buffer(). Is the commit message wrong, or was the
> > patch
> > > applied to the wrong function?
> > >
> > Ben,
> >
> > Most likely the message is "misleading". I currently don't have access to
> > the
> > source; I will confirm sometime tonight.
>
> The commit message is a typo (and is misleading).
Thanks for confirming.
Ben.
--
Ben Hutchings
Beware of programmers who carry screwdrivers. - Leonard Brandwein
signature.asc
Description: This is a digitally signed message part
Re: [PATCH] scsi: storvsc: Fix a bug in copy_from_bounce_buffer()
Commit 8de580742fee ("scsi: storvsc: Fix a bug in
copy_from_bounce_buffer()"), actually modified the function
copy_to_bounce_buffer(). Is the commit message wrong, or was the patch
applied to the wrong function?
Ben.
--
Ben Hutchings
Computers are not intelligent. They only think they are.
signature.asc
Description: This is a digitally signed message part
Re: [PATCH] Initialize off value in asd_process_ctrl_a_user()
On Tue, 2014-12-02 at 11:34 -0500, Eric B Munson wrote:
> If the asd_find_flash_de() function returns ENOENT the off value will
> be used uninitialized in the call to asd_read_flash_seg().
This is just papering over the problem. This was my attempt at a proper
fix: http://article.gmane.org/gmane.linux.scsi/91320
Ben.
> Signed-off-by: Eric B Munson
> Cc: sta...@vger.kernel.org
> ---
> drivers/scsi/aic94xx/aic94xx_sds.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c
> b/drivers/scsi/aic94xx/aic94xx_sds.c
> index edb43fd..6f6a5b8 100644
> --- a/drivers/scsi/aic94xx/aic94xx_sds.c
> +++ b/drivers/scsi/aic94xx/aic94xx_sds.c
> @@ -982,7 +982,7 @@ static int asd_process_ctrl_a_user(struct asd_ha_struct
> *asd_ha,
> struct asd_flash_dir *flash_dir)
> {
> int err, i;
> - u32 offs, size;
> + u32 offs = 0, size;
> struct asd_ll_el *el;
> struct asd_ctrla_phy_settings *ps;
> struct asd_ctrla_phy_settings dflt_ps;
--
Ben Hutchings
Life would be so much easier if we could look at the source code.
signature.asc
Description: This is a digitally signed message part
[PATCH 3.2 075/164] scsi: Fix error handling in SCSI_IOCTL_SEND_COMMAND
3.2.65-rc1 review patch. If anyone has any objections, please let me know.
--
From: Jan Kara
commit 84ce0f0e94ac97217398b3b69c21c7a62ebeed05 upstream.
When sg_scsi_ioctl() fails to prepare request to submit in
blk_rq_map_kern() we jump to a label where we just end up copying
(luckily zeroed-out) kernel buffer to userspace instead of reporting
error. Fix the problem by jumping to the right label.
CC: Jens Axboe
CC: linux-scsi@vger.kernel.org
Coverity-id: 1226871
Signed-off-by: Jan Kara
Fixed up the, now unused, out label.
Signed-off-by: Jens Axboe
Signed-off-by: Ben Hutchings
---
block/scsi_ioctl.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -505,7 +505,7 @@ int sg_scsi_ioctl(struct request_queue *
if (bytes && blk_rq_map_kern(q, rq, buffer, bytes, __GFP_WAIT)) {
err = DRIVER_ERROR << 24;
- goto out;
+ goto error;
}
memset(sense, 0, sizeof(sense));
@@ -515,7 +515,6 @@ int sg_scsi_ioctl(struct request_queue *
blk_execute_rq(q, disk, rq, 0);
-out:
err = rq->errors & 0xff;/* only 8 bit SCSI status */
if (err) {
if (rq->sense_len && rq->sense) {
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Backport of a fix for a race condition in HPSA driver (3rd time is the charm)
On Fri, 2014-11-14 at 11:26 -0800, Masoud Sharbiani wrote: > Dear stable maintainers, > Can you please backport commitid > 2cc5bfaf854463d9d1aa52091f60110fbf102a9 ([SCSI] hpsa: fix a race in > cmd_free/scsi_done) to 3.10 stable (and earlier, if applicable)? It > seems to fix an issue that we have run into a couple of times. > > Many thanks, > Masoud Sharbiani > > PS: Apologies for duplicate message that may have been captured in > your spam folder. I've queued this up for 3.2, thanks. Ben. -- Ben Hutchings Kids! Bringing about Armageddon can be dangerous. Do not attempt it in your own home. - Terry Pratchett and Neil Gaiman, `Good Omens' signature.asc Description: This is a digitally signed message part
Re: [PATCH] [SCSI] hpsa: Fix driver support flag initialisation on !x86
On Sun, 2014-07-20 at 17:42 -0700, Christoph Hellwig wrote: > On Mon, Jul 21, 2014 at 01:09:17AM +0100, Ben Hutchings wrote: > > On !x86 we currently don't read the existing support flags: > > Arnd already sent this and I've included it in the for-3.17 queue. Thanks. Ben. -- Ben Hutchings Make three consecutive correct guesses and you will be considered an expert. signature.asc Description: This is a digitally signed message part
[PATCH] [SCSI] hpsa: Fix driver support flag initialisation on !x86
On !x86 we currently don't read the existing support flags:
/home/benh/linux-3.14.13/drivers/scsi/hpsa.c:4375:17: warning: 'driver_support'
is used uninitialized in this function [-Wuninitialized]
driver_support |= ENABLE_UNIT_ATTN;
^
Signed-off-by: Ben Hutchings
Fixes: 28e134464734 ("[SCSI] hpsa: enable unit attention reporting")
Cc: # 3.14+
---
Compile-tested only.
Ben.
drivers/scsi/hpsa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 9a6e4a2..7c3ff51 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6350,9 +6350,9 @@ static inline void hpsa_set_driver_support_bits(struct
ctlr_info *h)
{
u32 driver_support;
+ driver_support = readl(&(h->cfgtable->driver_support));
#ifdef CONFIG_X86
/* Need to enable prefetch in the SCSI core for 6400 in x86 */
- driver_support = readl(&(h->cfgtable->driver_support));
driver_support |= ENABLE_SCSI_PREFETCH;
#endif
driver_support |= ENABLE_UNIT_ATTN;
--
Ben Hutchings
Make three consecutive correct guesses and you will be considered an expert.
signature.asc
Description: This is a digitally signed message part
Re: [PATCH RESEND] [SCSI] aic94xx: Remove broken fallback for missing 'Ctrl-A' user settings
On Wed, 2014-06-25 at 22:44 -0400, Martin K. Petersen wrote: > >>>>> "Christoph" == Christoph Hellwig writes: > > Christoph> On Sun, Jun 08, 2014 at 11:37:44PM +0100, Ben Hutchings wrote: > >> asd_process_ctrl_a_user() attempts to find user settings in flash, > >> and if they are missing it prepares a substitute structure containing > >> default values for PHY settings. But having done so, it will still > >> try to read user settings - from some random address in flash, as the > >> local variable 'offs' has not been initialised. > >> > >> Since asd_common_setup() already sets default PHY settings, there > >> seems to be no need to repeat them here, and we can just return 0. > >> > >> Compile-tested only. > > If I read this correctly we'll get the link rates initialized but the > sas_addr will no longer be populated for each phy_desc. > > The old code copied the board global sas_addr into dflt_ps before > calling asd_process_ctrla_phy_settings(). Whereas asd_common_setup() > does not populate the sas_addr. The old code also reassigned ps before calling asd_process_ctrla_phy_settings(), so dflt_ps was not actually used. Ben. -- Ben Hutchings Quantity is no substitute for quality, but it's the only one we've got. signature.asc Description: This is a digitally signed message part
[PATCH RESEND] [SCSI] bfa: Fix undefined bit shift on big-endian architectures with 32-bit DMA address
bfa_swap_words() shifts its argument (assumed to be 64-bit) by 32 bits
each way. In two places the argument type is dma_addr_t, which may be
32-bit, in which case the effect of the bit shift is undefined:
drivers/scsi/bfa/bfa_fcpim.c: In function 'bfa_ioim_send_ioreq':
drivers/scsi/bfa/bfa_fcpim.c:2497:4: warning: left shift count >= width of type
[enabled by default]
addr = bfa_sgaddr_le(sg_dma_address(sg));
^
drivers/scsi/bfa/bfa_fcpim.c:2497:4: warning: right shift count >= width of
type [enabled by default]
drivers/scsi/bfa/bfa_fcpim.c:2509:4: warning: left shift count >= width of type
[enabled by default]
addr = bfa_sgaddr_le(sg_dma_address(sg));
^
drivers/scsi/bfa/bfa_fcpim.c:2509:4: warning: right shift count >= width of
type [enabled by default]
Avoid this by adding casts to u64 in bfa_swap_words().
Compile-tested only.
Signed-off-by: Ben Hutchings
Cc: sta...@vger.kernel.org
Fixes: f16a17507b09 ('[SCSI] bfa: remove all OS wrappers')
---
Resending yet again, this time with current maintainer addresses.
Ben.
drivers/scsi/bfa/bfa_ioc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/bfa/bfa_ioc.h b/drivers/scsi/bfa/bfa_ioc.h
index 2e28392..a38aafa0 100644
--- a/drivers/scsi/bfa/bfa_ioc.h
+++ b/drivers/scsi/bfa/bfa_ioc.h
@@ -72,7 +72,7 @@ struct bfa_sge_s {
} while (0)
#define bfa_swap_words(_x) ( \
- ((_x) << 32) | ((_x) >> 32))
+ ((u64)(_x) << 32) | ((u64)(_x) >> 32))
#ifdef __BIG_ENDIAN
#define bfa_sge_to_be(_x)
--
Ben Hutchings
One of the nice things about standards is that there are so many of them.
signature.asc
Description: This is a digitally signed message part
[PATCH RESEND] [SCSI] aic94xx: Remove broken fallback for missing 'Ctrl-A' user settings
asd_process_ctrl_a_user() attempts to find user settings in flash, and
if they are missing it prepares a substitute structure containing
default values for PHY settings. But having done so, it will still
try to read user settings - from some random address in flash, as the
local variable 'offs' has not been initialised.
Since asd_common_setup() already sets default PHY settings, there
seems to be no need to repeat them here, and we can just return 0.
Compile-tested only.
Signed-off-by: Ben Hutchings
---
drivers/scsi/aic94xx/aic94xx_sds.c | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c
b/drivers/scsi/aic94xx/aic94xx_sds.c
index edb43fd..f5d51d2 100644
--- a/drivers/scsi/aic94xx/aic94xx_sds.c
+++ b/drivers/scsi/aic94xx/aic94xx_sds.c
@@ -981,29 +981,15 @@ static int asd_process_ctrla_phy_settings(struct
asd_ha_struct *asd_ha,
static int asd_process_ctrl_a_user(struct asd_ha_struct *asd_ha,
struct asd_flash_dir *flash_dir)
{
- int err, i;
+ int err;
u32 offs, size;
struct asd_ll_el *el;
struct asd_ctrla_phy_settings *ps;
- struct asd_ctrla_phy_settings dflt_ps;
err = asd_find_flash_de(flash_dir, FLASH_DE_CTRL_A_USER, &offs, &size);
if (err) {
ASD_DPRINTK("couldn't find CTRL-A user settings section\n");
- ASD_DPRINTK("Creating default CTRL-A user settings section\n");
-
- dflt_ps.id0 = 'h';
- dflt_ps.num_phys = 8;
- for (i =0; i < ASD_MAX_PHYS; i++) {
- memcpy(dflt_ps.phy_ent[i].sas_addr,
- asd_ha->hw_prof.sas_addr, SAS_ADDR_SIZE);
- dflt_ps.phy_ent[i].sas_link_rates = 0x98;
- dflt_ps.phy_ent[i].flags = 0x0;
- dflt_ps.phy_ent[i].sata_link_rates = 0x0;
- }
-
- size = sizeof(struct asd_ctrla_phy_settings);
- ps = &dflt_ps;
+ return 0;
}
if (size == 0)
--
Ben Hutchings
Never attribute to conspiracy what can adequately be explained by stupidity.
signature.asc
Description: This is a digitally signed message part
[PATCH RESEND] [SCSI] bfa: Fix undefined bit shift on big-endian architectures with 32-bit DMA address
bfa_swap_words() shifts its argument (assumed to be 64-bit) by 32 bits
each way. In two places the argument type is dma_addr_t, which may be
32-bit, in which case the effect of the bit shift is undefined:
drivers/scsi/bfa/bfa_fcpim.c: In function 'bfa_ioim_send_ioreq':
drivers/scsi/bfa/bfa_fcpim.c:2497:4: warning: left shift count >= width of type
[enabled by default]
addr = bfa_sgaddr_le(sg_dma_address(sg));
^
drivers/scsi/bfa/bfa_fcpim.c:2497:4: warning: right shift count >= width of
type [enabled by default]
drivers/scsi/bfa/bfa_fcpim.c:2509:4: warning: left shift count >= width of type
[enabled by default]
addr = bfa_sgaddr_le(sg_dma_address(sg));
^
drivers/scsi/bfa/bfa_fcpim.c:2509:4: warning: right shift count >= width of
type [enabled by default]
Avoid this by adding casts to u64 in bfa_swap_words().
Compile-tested only.
Signed-off-by: Ben Hutchings
Cc: sta...@vger.kernel.org
Fixes: f16a17507b09 ('[SCSI] bfa: remove all OS wrappers')
---
drivers/scsi/bfa/bfa_ioc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/bfa/bfa_ioc.h b/drivers/scsi/bfa/bfa_ioc.h
index 2e28392..a38aafa0 100644
--- a/drivers/scsi/bfa/bfa_ioc.h
+++ b/drivers/scsi/bfa/bfa_ioc.h
@@ -72,7 +72,7 @@ struct bfa_sge_s {
} while (0)
#define bfa_swap_words(_x) ( \
- ((_x) << 32) | ((_x) >> 32))
+ ((u64)(_x) << 32) | ((u64)(_x) >> 32))
#ifdef __BIG_ENDIAN
#define bfa_sge_to_be(_x)
--
Ben Hutchings
Never attribute to conspiracy what can adequately be explained by stupidity.
signature.asc
Description: This is a digitally signed message part
Re: 3.2.57 regression: isci driver broken: Unable to reset I T nexus?
I'm adding this revert to 3.2.58, taking your 'drop commit 584ec1226519'
as an ack.
Ben.
---
From: Ben Hutchings
Date: Wed, 30 Apr 2014 13:22:22 +0100
Subject: Revert "isci: fix reset timeout handling"
This reverts commit 584ec12265192bf49dfa270d517380f6723a6956, which
was commit ddfadd7736b677de2d4ca2cd5b4b655368c85a7a upstream. It
causes boot failure on 3.2 although no such problem occurs upstream.
Reported-by: Ondrej Zary
Signed-off-by: Ben Hutchings
Acked-by: Dan Williams
---
--- a/drivers/scsi/isci/port_config.c
+++ b/drivers/scsi/isci/port_config.c
@@ -610,6 +610,13 @@ static void sci_apc_agent_link_up(struct
sci_apc_agent_configure_ports(ihost, port_agent, iphy, true);
} else {
/* the phy is already the part of the port */
+ u32 port_state = iport->sm.current_state_id;
+
+ /* if the PORT'S state is resetting then the link up is from
+* port hard reset in this case, we need to tell the port
+* that link up is recieved
+*/
+ BUG_ON(port_state != SCI_PORT_RESETTING);
port_agent->phy_ready_mask |= 1 << phy_index;
sci_port_link_up(iport, iphy);
}
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -1390,7 +1390,7 @@ int isci_task_I_T_nexus_reset(struct dom
spin_unlock_irqrestore(&ihost->scic_lock, flags);
if (!idev || !test_bit(IDEV_EH, &idev->flags)) {
- ret = -ENODEV;
+ ret = TMF_RESP_FUNC_COMPLETE;
goto out;
}
--
Ben Hutchings
Life would be so much easier if we could look at the source code.
signature.asc
Description: This is a digitally signed message part
Bug#741686: linux-image-3.13-1-amd64: systemd-udevd kills long running mptsas module initailization, resulting in kernel oops
ocal_pci_probe+0x3a/0xa0 > [ 34.535584] [] ? pci_device_probe+0xca/0x120 > [ 34.549436] [] ? driver_probe_device+0x68/0x220 > [ 34.562960] [] ? __driver_attach+0x7b/0x80 > [ 34.576082] [] ? __device_attach+0x40/0x40 > [ 34.588726] [] ? bus_for_each_dev+0x53/0x90 > [ 34.600982] [] ? bus_add_driver+0x170/0x220 > [ 34.612910] [] ? 0xa014cfff > [ 34.624691] [] ? driver_register+0x56/0xd0 > [ 34.636367] [] ? 0xa014cfff > [ 34.647907] [] ? mptsas_init+0x11a/0x1000 [mptsas] > [ 34.659558] [] ? do_one_initcall+0x112/0x170 > [ 34.671205] [] ? load_module+0x1b07/0x23f0 > [ 34.682908] [] ? m_show+0x1c0/0x1c0 > [ 34.694597] [] ? SyS_finit_module+0x6d/0x70 > [ 34.706285] [] ? system_call_fastpath+0x16/0x1b > [ 34.718018] Code: 00 00 48 89 e6 4c 89 f7 e8 35 56 bf ff e9 40 ff ff ff b8 > 01 00 00 00 e9 87 fe ff ff 66 0f 1f 44 00 00 53 48 89 fb e8 57 e4 ff ff > ff 0b 79 08 48 89 df e8 3a fe ff ff 65 48 8b 04 25 40 c8 00 > [ 34.742873] RIP [] mutex_lock+0x9/0x25 > [ 34.754928] RSP > [ 34.766735] CR2: 0060 > [ 34.778493] ---[ end trace 7cf83da47bb3f354 ]--- -- Ben Hutchings When you say `I wrote a program that crashed Windows', people just stare ... and say `Hey, I got those with the system, *for free*'. - Linus Torvalds signature.asc Description: This is a digitally signed message part
[PATCH] [SCSI] bfa: Replace large udelay() with mdelay()
udelay() does not work on some architectures for values above
2000, in particular on ARM:
ERROR: "__bad_udelay" [drivers/scsi/bfa/bfa.ko] undefined!
Reported-by: Vagrant Cascadian
Signed-off-by: Ben Hutchings
---
drivers/scsi/bfa/bfa_ioc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
index 65180e1..50c75e1 100644
--- a/drivers/scsi/bfa/bfa_ioc.c
+++ b/drivers/scsi/bfa/bfa_ioc.c
@@ -7006,7 +7006,7 @@ bfa_flash_sem_get(void __iomem *bar)
while (!bfa_raw_sem_get(bar)) {
if (--n <= 0)
return BFA_STATUS_BADFLASH;
- udelay(1);
+ mdelay(10);
}
return BFA_STATUS_OK;
}
--
Ben Hutchings
I say we take off; nuke the site from orbit. It's the only way to be sure.
signature.asc
Description: This is a digitally signed message part
[PATCH RESEND 2] mvsas: Recognise device/subsystem 9485/9485 as 88SE9485
Matt Taggart reported that mvsas didn't bind to the Marvell
SAS controller on a Supermicro AOC-SAS2LP-MV8 board.
lspci reports it as:
01:00.0 RAID bus controller [0104]: Marvell Technology Group Ltd. Device
[1b4b:9485] (rev 03)
Subsystem: Marvell Technology Group Ltd. Device [1b4b:9485]
[...]
Add it to the device table as chip_9485.
Reported-by: Matt Taggart
Tested-by: Matt Taggart
Signed-off-by: Ben Hutchings
---
drivers/scsi/mvsas/mv_init.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index 7b7381d..83fa5f8 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -729,6 +729,15 @@ static struct pci_device_id mvs_pci_table[] = {
.class_mask = 0,
.driver_data= chip_9485,
},
+ {
+ .vendor = PCI_VENDOR_ID_MARVELL_EXT,
+ .device = 0x9485,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = 0x9485,
+ .class = 0,
+ .class_mask = 0,
+ .driver_data= chip_9485,
+ },
{ PCI_VDEVICE(OCZ, 0x1021), chip_9485}, /* OCZ RevoDrive3 */
{ PCI_VDEVICE(OCZ, 0x1022), chip_9485}, /* OCZ RevoDrive3/zDriveR4
(exact model unknown) */
{ PCI_VDEVICE(OCZ, 0x1040), chip_9485}, /* OCZ RevoDrive3/zDriveR4
(exact model unknown) */
--
Ben Hutchings
One of the nice things about standards is that there are so many of them.
--
Ben Hutchings
Absolutum obsoletum. (If it works, it's out of date.) - Stafford Beer
signature.asc
Description: This is a digitally signed message part
Re: Bug#733565: SIX messages per on boot console should be TWO
Control: reassign -1 src:linux 3.12.6-1 On Mon, 2013-12-30 at 07:13 +0800, jida...@jidanni.org wrote: > Package: linux-image-3.12-1-686-pae > > Currently any such devices as below that are attached during boot still > produce the below SIX messages per device when they should only produce > TWO: > > # dmesg |egrep Caching\|Assuming > [3.960663] sd 3:0:0:0: [sdb] No Caching mode page found > [3.960701] sd 3:0:0:0: [sdb] Assuming drive cache: write through > [3.978659] sd 3:0:0:0: [sdb] No Caching mode page found > [3.978694] sd 3:0:0:0: [sdb] Assuming drive cache: write through > [3.998652] sd 3:0:0:0: [sdb] No Caching mode page found > [3.998689] sd 3:0:0:0: [sdb] Assuming drive cache: write through > [4.174630] sd 2:0:0:2: [sde] No Caching mode page found > [4.174669] sd 2:0:0:2: [sde] Assuming drive cache: write through > [4.196628] sd 2:0:0:2: [sde] No Caching mode page found > [4.196664] sd 2:0:0:2: [sde] Assuming drive cache: write through > [4.220624] sd 2:0:0:2: [sde] No Caching mode page found > [4.220661] sd 2:0:0:2: [sde] Assuming drive cache: write through > > Please confirm that you also see the six messages right there on the > console stderr. I'm not seeing it at all here as my SSD apparently does implement the caching mode page. I can see that it is emitted by sd_read_cache_type(), which is called by sd_revalidate_disk(), and that is apparently now called 3 times during probe. Which is quite ridiculous. And I wonder whether this situation (no caching mode page) is really serious enough to deserve logging at ERR severity? Ben. -- Ben Hutchings Logic doesn't apply to the real world. - Marvin Minsky signature.asc Description: This is a digitally signed message part
Re: [net-next 1/1] linux-firmware: 3.2.3.0 Firmware for Brocade Adapters
On Tue, 2013-12-17 at 17:12 -0800, Rasesh Mody wrote: > This is the 3.2.3.0 firmware patch for Brocade 3.2.23.0 BFA and BNA drivers. > > Signed-off-by: Rasesh Mody > --- > WHENCE| 3 +++ > cbfw-3.2.3.0.bin | Bin 0 -> 414016 bytes > ct2fw-3.2.3.0.bin | Bin 0 -> 583688 bytes > ctfw-3.2.3.0.bin | Bin 0 -> 538712 bytes > 4 files changed, 3 insertions(+) > create mode 100644 cbfw-3.2.3.0.bin > create mode 100644 ct2fw-3.2.3.0.bin > create mode 100644 ctfw-3.2.3.0.bin [...] Applied, thanks. Ben. -- Ben Hutchings It is easier to write an incorrect program than to understand a correct one. signature.asc Description: This is a digitally signed message part
Re: [net-next 1/1] linux-firmware: 3.2.23.0 Firmware for Brocade Adapters
On Sat, 2013-12-07 at 22:00 -0800, Rasesh Mody wrote: > This is the firmware patch for Brocade 3.2.23.0 BFA and BNA drivers. > > Signed-off-by: Rasesh Mody > --- > WHENCE| 3 +++ > cbfw-3.2.3.0.bin | Bin 0 -> 414016 bytes > ct2fw-3.2.3.0.bin | Bin 0 -> 583688 bytes > ctfw-3.2.3.0.bin | Bin 0 -> 538712 bytes > 4 files changed, 3 insertions(+) > create mode 100644 cbfw-3.2.3.0.bin > create mode 100644 ct2fw-3.2.3.0.bin > create mode 100644 ctfw-3.2.3.0.bin [...] This patch seems to have been truncated in transit: $ git am -s ~/tmp/\[net-next_1_1\]_linux-firmware\:_3.2.23.0_Firmware_for_Brocade_Adapters.mbox Applying: linux-firmware: 3.2.23.0 Firmware for Brocade Adapters error: corrupt binary patch at line 13542: znHIKNgBV5>FJqIQgo|#@us1Wq_D(J Please re-send (but not to the mailing lists; it's pointless to send binaries there). Ben. -- Ben Hutchings Lowery's Law: If it jams, force it. If it breaks, it needed replacing anyway. signature.asc Description: This is a digitally signed message part
Re: [PATCH 1/1 linux-scs-ml] qla2xxx: Update ql2{4,5}00_fw.bin to version 7.01.00
On Sat, 2013-11-23 at 14:00 +0100, Xose Vazquez Perez wrote: > resent to linux-scsi-ml, without the binary blob(316K). > > fw and licence were taken from http://ldriver.qlogic.com/firmware/ > > Cc: Chad Dupuis > Cc: Andrew Vasquez > Cc: linux-dri...@qlogic.com > Cc: David Woodhouse > Cc: Ben Hutchings > Signed-off-by: Xose Vazquez Perez > --- > LICENCE.qla2xxx | 60 > > WHENCE | 4 ++-- > ql2400_fw.bin | Bin 257108 -> 259332 bytes > ql2500_fw.bin | Bin 260064 -> 265420 bytes > 4 files changed, 28 insertions(+), 36 deletions(-) [...] Applied, thanks. Ben. -- Ben Hutchings Q. Which is the greater problem in the world today, ignorance or apathy? A. I don't know and I couldn't care less. signature.asc Description: This is a digitally signed message part
[PATCH RESEND] mvsas: Recognise device/subsystem 9485/9485 as 88SE9485
Matt Taggart reported that mvsas didn't bind to the Marvell
SAS controller on a Supermicro AOC-SAS2LP-MV8 board.
lspci reports it as:
01:00.0 RAID bus controller [0104]: Marvell Technology Group Ltd. Device
[1b4b:9485] (rev 03)
Subsystem: Marvell Technology Group Ltd. Device [1b4b:9485]
[...]
Add it to the device table as chip_9485.
Reported-by: Matt Taggart
Tested-by: Matt Taggart
Signed-off-by: Ben Hutchings
---
drivers/scsi/mvsas/mv_init.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index 7b7381d..83fa5f8 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -729,6 +729,15 @@ static struct pci_device_id mvs_pci_table[] = {
.class_mask = 0,
.driver_data= chip_9485,
},
+ {
+ .vendor = PCI_VENDOR_ID_MARVELL_EXT,
+ .device = 0x9485,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = 0x9485,
+ .class = 0,
+ .class_mask = 0,
+ .driver_data= chip_9485,
+ },
{ PCI_VDEVICE(OCZ, 0x1021), chip_9485}, /* OCZ RevoDrive3 */
{ PCI_VDEVICE(OCZ, 0x1022), chip_9485}, /* OCZ RevoDrive3/zDriveR4
(exact model unknown) */
{ PCI_VDEVICE(OCZ, 0x1040), chip_9485}, /* OCZ RevoDrive3/zDriveR4
(exact model unknown) */
--
Ben Hutchings
One of the nice things about standards is that there are so many of them.
signature.asc
Description: This is a digitally signed message part
[PATCH 8/8] [SCSI] pmcraid: Pass pointers to access_ok(), not integers
Most architectures define access_ok() as a macro that casts its
argument such that an argument of type unsigned long will be accepted
without complaint. However, the proper type is void *, and passing
unsigned long results in a warning on sparc64.
Compile-tested only.
Signed-off-by: Ben Hutchings
---
drivers/scsi/pmcraid.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index 1eb7b028..4e0a2f3 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -3794,7 +3794,8 @@ static long pmcraid_ioctl_passthrough(
}
if (request_size > 0) {
- rc = access_ok(access, arg, request_offset + request_size);
+ rc = access_ok(access, (void *)arg,
+ request_offset + request_size);
if (!rc) {
rc = -EFAULT;
--
Ben Hutchings
If at first you don't succeed, you're doing about average.
signature.asc
Description: This is a digitally signed message part
[PATCH 5/8] [SCSI] tgt: Pass pointers to virt_to_page(), not integers
Most architectures define virt_to_page() as a macro that casts its
argument such that an argument of type unsigned long will be accepted
without complaint. However, the proper type is void *, and passing
unsigned long results in a warning on MIPS.
Compile-tested only.
Signed-off-by: Ben Hutchings
---
drivers/scsi/scsi_tgt_if.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_tgt_if.c b/drivers/scsi/scsi_tgt_if.c
index 6209110..7199753 100644
--- a/drivers/scsi/scsi_tgt_if.c
+++ b/drivers/scsi/scsi_tgt_if.c
@@ -286,7 +286,7 @@ static int uspace_ring_map(struct vm_area_struct *vma,
unsigned long addr,
int i, err;
for (i = 0; i < TGT_RING_PAGES; i++) {
- struct page *page = virt_to_page(ring->tr_pages[i]);
+ struct page *page = virt_to_page((void *)ring->tr_pages[i]);
err = vm_insert_page(vma, addr, page);
if (err)
return err;
--
Ben Hutchings
If at first you don't succeed, you're doing about average.
signature.asc
Description: This is a digitally signed message part
[PATCH 0/8] Fix minor address type errors
Various bits of code are mixing making assumptions about the size of dma_addr_t or resource_size_t, or mixing up pointer and integer types. All these fixes are based on compiler warnings and so far as I can see the bugs are practically harmless. Ben. Ben Hutchings (8): IB/cxgb4: Fix formatting of physical address farsync: Fix confusion about DMA address and buffer offset types drm: Do not include page offset in argument to virt_to_page() drm: Pass pointers to virt_to_page() [SCSI] tgt: Pass pointers to virt_to_page(), not integers uio: Pass pointers to virt_to_page(), not integers rds: Pass pointers to virt_to_page(), not integers [SCSI] pmcraid: Pass pointers to access_ok(), not integers drivers/gpu/drm/drm_pci.c| 4 ++-- drivers/gpu/drm/drm_vm.c | 2 +- drivers/infiniband/hw/cxgb4/device.c | 4 ++-- drivers/net/wan/farsync.c| 19 +++ drivers/scsi/pmcraid.c | 3 ++- drivers/scsi/scsi_tgt_if.c | 2 +- drivers/uio/uio.c| 6 -- net/rds/message.c| 2 +- 8 files changed, 20 insertions(+), 22 deletions(-) -- Ben Hutchings If at first you don't succeed, you're doing about average. signature.asc Description: This is a digitally signed message part
[PATCH] [SCSI] aic94xx: Remove broken fallback for missing 'Ctrl-A' user settings
asd_process_ctrl_a_user() attempts to find user settings in flash, and
if they are missing it prepares a substitute structure containing
default values for PHY settings. But having done so, it will still
try to read user settings - from some random address in flash, as the
local variable 'offs' has not been initialised.
Since asd_common_setup() already sets default PHY settings, there
seems to be no need to repeat them here, and we can just return 0.
Compile-tested only.
Signed-off-by: Ben Hutchings
---
drivers/scsi/aic94xx/aic94xx_sds.c | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c
b/drivers/scsi/aic94xx/aic94xx_sds.c
index edb43fd..f5d51d2 100644
--- a/drivers/scsi/aic94xx/aic94xx_sds.c
+++ b/drivers/scsi/aic94xx/aic94xx_sds.c
@@ -981,29 +981,15 @@ static int asd_process_ctrla_phy_settings(struct
asd_ha_struct *asd_ha,
static int asd_process_ctrl_a_user(struct asd_ha_struct *asd_ha,
struct asd_flash_dir *flash_dir)
{
- int err, i;
+ int err;
u32 offs, size;
struct asd_ll_el *el;
struct asd_ctrla_phy_settings *ps;
- struct asd_ctrla_phy_settings dflt_ps;
err = asd_find_flash_de(flash_dir, FLASH_DE_CTRL_A_USER, &offs, &size);
if (err) {
ASD_DPRINTK("couldn't find CTRL-A user settings section\n");
- ASD_DPRINTK("Creating default CTRL-A user settings section\n");
-
- dflt_ps.id0 = 'h';
- dflt_ps.num_phys = 8;
- for (i =0; i < ASD_MAX_PHYS; i++) {
- memcpy(dflt_ps.phy_ent[i].sas_addr,
- asd_ha->hw_prof.sas_addr, SAS_ADDR_SIZE);
- dflt_ps.phy_ent[i].sas_link_rates = 0x98;
- dflt_ps.phy_ent[i].flags = 0x0;
- dflt_ps.phy_ent[i].sata_link_rates = 0x0;
- }
-
- size = sizeof(struct asd_ctrla_phy_settings);
- ps = &dflt_ps;
+ return 0;
}
if (size == 0)
--
Ben Hutchings
If at first you don't succeed, you're doing about average.
signature.asc
Description: This is a digitally signed message part
[PATCH] [SCSI] bfa: Fix undefined bit shift on big-endian architectures with 32-bit DMA address
bfa_swap_words() shifts its argument (assumed to be 64-bit) by 32 bits
each way. In two places the argument type is dma_addr_t, which may be
32-bit, in which case the effect of the bit shift is undefined:
drivers/scsi/bfa/bfa_fcpim.c: In function 'bfa_ioim_send_ioreq':
drivers/scsi/bfa/bfa_fcpim.c:2497:4: warning: left shift count >= width of type
[enabled by default]
addr = bfa_sgaddr_le(sg_dma_address(sg));
^
drivers/scsi/bfa/bfa_fcpim.c:2497:4: warning: right shift count >= width of
type [enabled by default]
drivers/scsi/bfa/bfa_fcpim.c:2509:4: warning: left shift count >= width of type
[enabled by default]
addr = bfa_sgaddr_le(sg_dma_address(sg));
^
drivers/scsi/bfa/bfa_fcpim.c:2509:4: warning: right shift count >= width of
type [enabled by default]
Avoid this by adding casts to u64 in bfa_swap_words().
Compile-tested only.
Signed-off-by: Ben Hutchings
Cc: sta...@vger.kernel.org
Fixes: f16a17507b09 ('[SCSI] bfa: remove all OS wrappers')
---
drivers/scsi/bfa/bfa_ioc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/bfa/bfa_ioc.h b/drivers/scsi/bfa/bfa_ioc.h
index 90814fe..d5b3f66 100644
--- a/drivers/scsi/bfa/bfa_ioc.h
+++ b/drivers/scsi/bfa/bfa_ioc.h
@@ -72,7 +72,7 @@ struct bfa_sge_s {
} while (0)
#define bfa_swap_words(_x) ( \
- ((_x) << 32) | ((_x) >> 32))
+ ((u64)(_x) << 32) | ((u64)(_x) >> 32))
#ifdef __BIG_ENDIAN
#define bfa_sge_to_be(_x)
--
Ben Hutchings
If at first you don't succeed, you're doing about average.
signature.asc
Description: This is a digitally signed message part
Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
On Sun, 2013-10-06 at 09:10 +0200, Alexander Gordeev wrote: > On Sun, Oct 06, 2013 at 05:19:46PM +1100, Benjamin Herrenschmidt wrote: > > On Sun, 2013-10-06 at 08:02 +0200, Alexander Gordeev wrote: > > > In fact, in the current design to address the quota race decently the > > > drivers would have to protect the *loop* to prevent the quota change > > > between a pci_enable_msix() returned a positive number and the the next > > > call to pci_enable_msix() with that number. Is it doable? > > > > I am not advocating for the current design, simply saying that your > > proposal doesn't address this issue while Ben's does. > > There is one major flaw in min-max approach - the generic MSI layer > will have to take decisions on exact number of MSIs to request, not > device drivers. [... No, the min-max functions should be implemented using the same loop that drivers are expected to use now. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
On Tue, 2013-10-08 at 07:10 +1100, Benjamin Herrenschmidt wrote: > On Mon, 2013-10-07 at 14:01 -0400, Tejun Heo wrote: > > I don't think the same race condition would happen with the loop. The > > problem case is where multiple msi(x) allocation fails completely > > because the global limit went down before inquiry and allocation. In > > the loop based interface, it'd retry with the lower number. > > > > As long as the number of drivers which need this sort of adaptive > > allocation isn't too high and the common cases can be made simple, I > > don't think the "complex" part of interface is all that important. > > Maybe we can have reserve / cancel type interface or just keep the > > loop with more explicit function names (ie. try_enable or something > > like that). > > I'm thinking a better API overall might just have been to request > individual MSI-X one by one :-) > > We want to be able to request an MSI-X at runtime anyway ... if I want > to dynamically add a queue to my network interface, I want it to be able > to pop a new arbitrary MSI-X. Yes, this would be very useful. > And we don't want to lock drivers into contiguous MSI-X sets either. I don't think there's any such limitation now. The entries array passed to pci_enable_msix() specifies which MSI-X vectors the driver wants to enable. It's usually filled with 0..nvec-1 in order, but not always. And the IRQ numbers returned aren't usually contiguous either, on x86. Ben. > And for the cleanup ... well that's what the "pcim" functions are for, > we can just make MSI-X variants. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
On Fri, 2013-10-04 at 10:29 +0200, Alexander Gordeev wrote: > On Thu, Oct 03, 2013 at 11:49:45PM +0100, Ben Hutchings wrote: > > On Wed, 2013-10-02 at 12:48 +0200, Alexander Gordeev wrote: > > > This update converts pci_enable_msix() and pci_enable_msi_block() > > > interfaces to canonical kernel functions and makes them return a > > > error code in case of failure or 0 in case of success. > > [...] > > > > I think this is fundamentally flawed: pci_msix_table_size() and > > pci_get_msi_cap() can only report the limits of the *device* (which the > > driver usually already knows), whereas MSI allocation can also be > > constrained due to *global* limits on the number of distinct IRQs. > > Even the current implementation by no means addresses it. Although it > might seem a case for architectures to report the number of IRQs available > for a driver to retry, in fact they all just fail. The same applies to > *any* other type of resource involved: irq_desc's, CPU interrupt vector > space, msi_desc's etc. No platform cares about it and just bails out once > a constrain met (please correct me if I am wrong here). Given that Linux > has been doing well even on embedded I think we should not change it. > > The only exception to the above is pSeries platform which takes advantage > of the current design (to implement MSI quota). There are indications we > can satisfy pSeries requirements, but the design proposed in this RFC > is not going to change drastically anyway. The start of the discusstion > is here: https://lkml.org/lkml/2013/9/5/293 All I can see there is that Tejun didn't think that the global limits and positive return values were implemented by any architecture. But you have a counter-example, so I'm not sure what your point is. It has been quite a while since I saw this happen on x86. But I just checked on a test system running RHEL 5 i386 (Linux 2.6.18). If I ask for 16 MSI-X vectors on a device that supports 1024, the return value is 8, and indeed I can then successfully allocate 8. Now that's going quite a way back, and it may be that global limits aren't a significant problem any more. With the x86_64 build of RHEL 5 on an identical system, I can allocate 16 or even 32, so this is apparently not a hardware limit in this case. > > Currently pci_enable_msix() will report a positive value if it fails due > > to the global limit. Your patch 7 removes that. pci_enable_msi_block() > > unfortunately doesn't appear to do this. > > pci_enable_msi_block() can do more than one MSI only on x86 (with IOMMU), > but it does not bother to return positive numbers, indeed. > > > It seems to me that a more useful interface would take a minimum and > > maximum number of vectors from the driver. This wouldn't allow the > > driver to specify that it could only accept, say, any even number within > > a certain range, but you could still leave the current functions > > available for any driver that needs that. > > Mmmm.. I am not sure I am getting it. Could you please rephrase? Most drivers seem to either: (a) require exactly a certain number of MSI vectors, or (b) require a minimum number of MSI vectors, usually want to allocate more, and work with any number in between We can support drivers in both classes by adding new allocation functions that allow specifying a minimum (required) and maximum (wanted) number of MSI vectors. Those in class (a) would just specify the same value for both. These new functions can take account of any global limit or allocation policy without any further changes to the drivers that use them. The few drivers with more specific requirements would still need to implement the currently recommended loop, using the old allocation functions. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
On Wed, 2013-10-02 at 12:48 +0200, Alexander Gordeev wrote:
> This series is against "next" branch in Bjorn's repo:
> git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git
>
> Currently pci_enable_msi_block() and pci_enable_msix() interfaces
> return a error code in case of failure, 0 in case of success and a
> positive value which indicates the number of MSI-X/MSI interrupts
> that could have been allocated. The latter value should be passed
> to a repeated call to the interfaces until a failure or success:
>
>
> for (i = 0; i < FOO_DRIVER_MAXIMUM_NVEC; i++)
> adapter->msix_entries[i].entry = i;
>
> while (nvec >= FOO_DRIVER_MINIMUM_NVEC) {
> rc = pci_enable_msix(adapter->pdev,
>adapter->msix_entries, nvec);
> if (rc > 0)
> nvec = rc;
> else
> return rc;
> }
>
> return -ENOSPC;
>
>
> This technique proved to be confusing and error-prone. Vast share
> of device drivers simply fail to follow the described guidelines.
>
> This update converts pci_enable_msix() and pci_enable_msi_block()
> interfaces to canonical kernel functions and makes them return a
> error code in case of failure or 0 in case of success.
[...]
I think this is fundamentally flawed: pci_msix_table_size() and
pci_get_msi_cap() can only report the limits of the *device* (which the
driver usually already knows), whereas MSI allocation can also be
constrained due to *global* limits on the number of distinct IRQs.
Currently pci_enable_msix() will report a positive value if it fails due
to the global limit. Your patch 7 removes that. pci_enable_msi_block()
unfortunately doesn't appear to do this.
It seems to me that a more useful interface would take a minimum and
maximum number of vectors from the driver. This wouldn't allow the
driver to specify that it could only accept, say, any even number within
a certain range, but you could still leave the current functions
available for any driver that needs that.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 06/77] PCI/MSI: Factor out pci_get_msi_cap() interface
On Wed, 2013-10-02 at 12:48 +0200, Alexander Gordeev wrote:
[...]
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -812,6 +812,21 @@ static int pci_msi_check_device(struct pci_dev *dev, int
> nvec, int type)
> return 0;
> }
>
> +int pci_get_msi_cap(struct pci_dev *dev)
> +{
> + int ret;
> + u16 msgctl;
> +
> + if (!dev->msi_cap)
> + return -EINVAL;
[...]
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1144,6 +1144,11 @@ struct msix_entry {
>
>
> #ifndef CONFIG_PCI_MSI
> +static inline int pci_get_msi_cap(struct pci_dev *dev)
> +{
> + return -1;
[...]
Shouldn't this also return -EINVAL?
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 01/77] PCI/MSI: Fix return value when populate_msi_sysfs() failed
On Wed, 2013-10-02 at 17:39 -0700, Jon Mason wrote: > On Wed, Oct 02, 2013 at 12:48:17PM +0200, Alexander Gordeev wrote: > > Signed-off-by: Alexander Gordeev > > Since you are changing the behavior of the msix_capability_init > function on populate_msi_sysfs error, a comment describing why in this > commit would be nice. [...] This function was already treating that error as fatal, and freeing the MSIs. The change in behaviour is that it now returns the error code in this case, rather than 0. This is obviously correct and properly described by the one-line summary. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] linux-firmware: Add Brocade FC/FCOE Adapter firmware files
On Tue, 2013-09-10 at 16:10 -0700, Rasesh Mody wrote: > This patch adds firmware files for Brocade HBA and CNA drivers(BFA and BNA). > > Signed-off-by: Rasesh Mody > --- > WHENCE| 27 +++ > cbfw-3.2.1.1.bin | Bin 0 -> 412528 bytes > ct2fw-3.2.1.1.bin | Bin 0 -> 582440 bytes > ctfw-3.2.1.1.bin | Bin 0 -> 537160 bytes > 4 files changed, 27 insertions(+) > create mode 100644 cbfw-3.2.1.1.bin > create mode 100644 ct2fw-3.2.1.1.bin > create mode 100644 ctfw-3.2.1.1.bin [...] Applied, thanks. Sorry for the delay. Ben. -- Ben Hutchings Life is like a sewer: what you get out of it depends on what you put into it. signature.asc Description: This is a digitally signed message part
Re: [PATCH 01/51] DMA-API: provide a helper to set both DMA and coherent DMA masks
On Thu, 2013-09-19 at 22:25 +0100, Russell King wrote: > Provide a helper to set both the DMA and coherent DMA masks to the > same value - this avoids duplicated code in a number of drivers, > sometimes with buggy error handling, and also allows us identify > which drivers do things differently. > > Signed-off-by: Russell King > --- > Documentation/DMA-API-HOWTO.txt | 37 ++--- > Documentation/DMA-API.txt |8 > include/linux/dma-mapping.h | 14 ++ > 3 files changed, 44 insertions(+), 15 deletions(-) > > diff --git a/Documentation/DMA-API-HOWTO.txt b/Documentation/DMA-API-HOWTO.txt > index 14129f1..5e98303 100644 > --- a/Documentation/DMA-API-HOWTO.txt > +++ b/Documentation/DMA-API-HOWTO.txt [...] > -dma_set_coherent_mask() will always be able to set the same or a > -smaller mask as dma_set_mask(). However for the rare case that a > +The coherent coherent mask will always be able to set the same or a > +smaller mask as the streaming mask. However for the rare case that a [...] The new wording doesn't make sense; a mask doesn't set itself. I would suggest: "The coherent mask can always be set to the same or a smaller mask than the streaming mask." Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mvsas: Recognise device/subsystem 9485/9485 as 88SE9485
Matt Taggart reported that mvsas didn't bind to the Marvell
SAS controller on a Supermicro AOC-SAS2LP-MV8 board.
lspci reports it as:
01:00.0 RAID bus controller [0104]: Marvell Technology Group Ltd. Device
[1b4b:9485] (rev 03)
Subsystem: Marvell Technology Group Ltd. Device [1b4b:9485]
[...]
Add it to the device table as chip_9485.
Reported-by: Matt Taggart
Tested-by: Matt Taggart
Signed-off-by: Ben Hutchings
---
drivers/scsi/mvsas/mv_init.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index 7b7381d..83fa5f8 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -729,6 +729,15 @@ static struct pci_device_id mvs_pci_table[] = {
.class_mask = 0,
.driver_data= chip_9485,
},
+ {
+ .vendor = PCI_VENDOR_ID_MARVELL_EXT,
+ .device = 0x9485,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = 0x9485,
+ .class = 0,
+ .class_mask = 0,
+ .driver_data= chip_9485,
+ },
{ PCI_VDEVICE(OCZ, 0x1021), chip_9485}, /* OCZ RevoDrive3 */
{ PCI_VDEVICE(OCZ, 0x1022), chip_9485}, /* OCZ RevoDrive3/zDriveR4
(exact model unknown) */
{ PCI_VDEVICE(OCZ, 0x1040), chip_9485}, /* OCZ RevoDrive3/zDriveR4
(exact model unknown) */
signature.asc
Description: This is a digitally signed message part
Re: [stable] [SCSI] Fix incorrect memset in bnx2fc_parse_fcp_rsp
On Wed, 2013-07-24 at 16:20 +0200, Andi Kleen wrote: > On Wed, Jul 24, 2013 at 04:42:42AM +0100, Ben Hutchings wrote: > > This looks like a candidate for stable: > > Don't know if it's a security issue, but should be fine for stable. OK, I've queued it up for 3.2. Ben. > > commit 16da05b1158d1bcb31656e636a8736a663b1cf1f > > Author: Andi Kleen > > Date: Mon Sep 3 20:50:30 2012 +0200 > > > > [SCSI] Fix incorrect memset in bnx2fc_parse_fcp_rsp > > > > Typically this sort of bug can result in leaking kernel memory to > > userland, which is a security issue, but I don't know whether that's > > true in this case. > > > > Ben. > > > > -- > > Ben Hutchings > > Once a job is fouled up, anything done to improve it makes it worse. > > > -- Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse. signature.asc Description: This is a digitally signed message part
[stable] [SCSI] Fix incorrect memset in bnx2fc_parse_fcp_rsp
This looks like a candidate for stable: commit 16da05b1158d1bcb31656e636a8736a663b1cf1f Author: Andi Kleen Date: Mon Sep 3 20:50:30 2012 +0200 [SCSI] Fix incorrect memset in bnx2fc_parse_fcp_rsp Typically this sort of bug can result in leaking kernel memory to userland, which is a security issue, but I don't know whether that's true in this case. Ben. -- Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse. signature.asc Description: This is a digitally signed message part
[SCSI] ufs: Add missing dependency on CONFIG_HAS_IOMEM
The ufs driver doesn't build on s390 with CONFIG_PCI disabled as it requires MMIO functions. Marking for 3.9-stable only as CONFIG_SCSI_UFSHCD was previously dependent on CONFIG_PCI. Signed-off-by: Ben Hutchings Cc: sta...@vger.kernel.org # 3.9 --- --- a/drivers/scsi/ufs/Kconfig +++ b/drivers/scsi/ufs/Kconfig @@ -34,7 +34,7 @@ config SCSI_UFSHCD tristate "Universal Flash Storage Controller Driver Core" - depends on SCSI + depends on SCSI && HAS_IOMEM ---help--- This selects the support for UFS devices in Linux, say Y and make sure that you know the name of your UFS host adapter (the card signature.asc Description: This is a digitally signed message part
[PATCH] [SCSI] sd: Fix parsing of 'temporary ' cache mode prefix
Commit 39c60a0948cc '[SCSI] sd: fix array cache flushing bug causing performance problems' added temp as a pointer to "temporary " and used sizeof(temp) - 1 as its length. But sizeof(temp) is the size of the pointer, not the size of the string constant. Change temp to a static array so that sizeof() does what was intended. Compile-tested only. Signed-off-by: Ben Hutchings Cc: sta...@vger.kernel.org --- drivers/scsi/sd.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index c1c5552..6f6a1b4 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -142,7 +142,7 @@ sd_store_cache_type(struct device *dev, struct device_attribute *attr, char *buffer_data; struct scsi_mode_data data; struct scsi_sense_hdr sshdr; - const char *temp = "temporary "; + static const char temp[] = "temporary "; int len; if (sdp->type != TYPE_DISK) signature.asc Description: This is a digitally signed message part
[PATCH] [SCSI] aacraid: Fix invalid bit shifts of DMA address
Commit b5f1758f221e ('[SCSI] aacraid: Fix endian issues in core and
SRC portions of driver') changed the type of the address variable in
aac_src_deliver_message() from u64 to dma_addr_t. In configurations
with 32-bit dma_addr_t, this results in:
drivers/scsi/aacraid/src.c: In function 'aac_src_deliver_message':
drivers/scsi/aacraid/src.c:410:3: warning: right shift count >= width of type
[enabled by default]
drivers/scsi/aacraid/src.c:434:2: warning: right shift count >= width of type
[enabled by default]
Signed-off-by: Ben Hutchings
Cc: stable # v3.6+
---
This is compile-tested only.
Ben.
drivers/scsi/aacraid/src.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index 3b021ec..129b984 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -407,7 +407,7 @@ static int aac_src_deliver_message(struct fib *fib)
fib->hw_fib_va->header.StructType = FIB_MAGIC2;
fib->hw_fib_va->header.SenderFibAddress = (u32)address;
fib->hw_fib_va->header.u.TimeStamp = 0;
- BUG_ON((u32)(address >> 32) != 0L);
+ BUG_ON((u64)address >> 32);
address |= fibsize;
} else {
/* Calculate the amount to the fibsize bits */
@@ -431,7 +431,7 @@ static int aac_src_deliver_message(struct fib *fib)
address |= fibsize;
}
- src_writel(dev, MUnit.IQ_H, (address >> 32) & 0x);
+ src_writel(dev, MUnit.IQ_H, (u64)address >> 32);
src_writel(dev, MUnit.IQ_L, address & 0x);
return 0;
signature.asc
Description: This is a digitally signed message part
Re: [PATCH] iscsi-target: Fix bug in handling of ExpStatSN ACK during u32 wrap-around
This patch has just made its way into my queue for 3.2.y:
On Mon, 2012-11-05 at 18:02 -0800, Steve Hodgson wrote:
> This patch fixes a bug in the hanlding of initiator provided ExpStatSN and
> individual iscsi_cmd->stat_sn comparision during iscsi_conn->stat_sn
> wrap-around within iscsit_ack_from_expstatsn() code.
>
> This bug would manifest itself as iscsi_cmd descriptors not being Acked
> by a lower ExpStatSn, causing them to be leaked until an iSCSI connection
> or session reinstatement event occurs to release all commands.
>
> Also fix up two other uses of incorrect CmdSN SNA comparison to use wrapper
> usage from include/scsi/iscsi_proto.h.
[...]
> --- a/drivers/target/iscsi/iscsi_target_erl2.c
> +++ b/drivers/target/iscsi/iscsi_target_erl2.c
> @@ -372,7 +372,7 @@ int iscsit_prepare_cmds_for_realligance(struct iscsi_conn
> *conn)
>* made generic here.
>*/
> if (!(cmd->cmd_flags & ICF_OOO_CMDSN) && !cmd->immediate_cmd &&
> - (cmd->cmd_sn >= conn->sess->exp_cmd_sn)) {
> + iscsi_sna_gte(cmd->stat_sn, conn->sess->exp_cmd_sn)) {
> list_del(&cmd->i_conn_node);
> spin_unlock_bh(&conn->cmd_lock);
> iscsit_free_cmd(cmd);
[...]
This changes cmd->cmd_sn to cmd->stat_sn, but the commit message only
describes fixes to wrap-around. Is that another fix or a bug?
Ben.
--
Ben Hutchings
If God had intended Man to program,
we'd have been born with serial I/O ports.
signature.asc
Description: This is a digitally signed message part
Re: Bug#666108: megaraid_sas: which patches are needed for 2.6.32.y?
On Mon, 2012-12-03 at 22:27 -0800, Jonathan Nieder wrote: > tags 666108 - moreinfo > quit > > Torbjørn Thorsen wrote: > > > The kernel boots with no problems, the storage controller seems to be > > working nicely. > > > > root@xen14:~# dmesg | grep -i mega > > [3.340638] megasas: 00.00.05.38-rc1 Wed. May. 11 17:00:00 PDT 2011 > > [3.340742] megasas: 0x1000:0x005b:0x1028:0x1f38: bus 3:slot 0:func 0 > > Yay, thanks much for checking. > > Ben and Dann, what do you think? Should we apply these patches[1]? > > Regards, > Jonathan > > [1] > http://alioth.debian.org/~jrnieder-guest/temp/driver-test/megaraid-driver-backport.mbox I've committed these, but we should really get some more users of both old and new hardware to test the result. Ben. -- Ben Hutchings Always try to do things in chronological order; it's less confusing that way. signature.asc Description: This is a digitally signed message part
[PATCH] [SCSI] mv_sas: Fix confusion between enum sas_device_type and enum sas_dev_type
The enumeration of standard types is called enum sas_device_type and is defined in along with the Linux internal structures, whereas the enumeration with Linux extensions is called enum sas_dev_type and is defined in along with the standard-defined structures. All clear? Apparently not; I can't think why. Signed-off-by: Ben Hutchings --- drivers/scsi/mvsas/mv_sas.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c index a3776d6..9f91359 100644 --- a/drivers/scsi/mvsas/mv_sas.c +++ b/drivers/scsi/mvsas/mv_sas.c @@ -1153,10 +1153,10 @@ void mvs_update_phyinfo(struct mvs_info *mvi, int i, int get_st) phy->identify.device_type = phy->att_dev_info & PORT_DEV_TYPE_MASK; - if (phy->identify.device_type == SAS_END_DEV) + if (phy->identify.device_type == SAS_END_DEVICE) phy->identify.target_port_protocols = SAS_PROTOCOL_SSP; - else if (phy->identify.device_type != NO_DEVICE) + else if (phy->identify.device_type != SAS_PHY_UNUSED) phy->identify.target_port_protocols = SAS_PROTOCOL_SMP; if (oob_done) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [SCSI] aic94xx: Remove broken fallback for missing 'Ctrl-A' user settings
asd_process_ctrl_a_user() attempts to find user settings in flash, and
if they are missing it prepares a substitute structure containing
default values for PHY settings. But having done so, it will still
try to read user settings - from some random address in flash, as the
local variable 'offs' has not been initialised.
Since asd_common_setup() already sets default PHY settings, there
seems to be no need to repeat them here, and we can just return 0.
This matches what is done if any empty user settings area is found.
Signed-off-by: Ben Hutchings
---
Compile-tested only.
Ben.
drivers/scsi/aic94xx/aic94xx_sds.c | 19 +++
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c
b/drivers/scsi/aic94xx/aic94xx_sds.c
index edb43fd..7b13ccc 100644
--- a/drivers/scsi/aic94xx/aic94xx_sds.c
+++ b/drivers/scsi/aic94xx/aic94xx_sds.c
@@ -981,29 +981,16 @@ static int asd_process_ctrla_phy_settings(struct
asd_ha_struct *asd_ha,
static int asd_process_ctrl_a_user(struct asd_ha_struct *asd_ha,
struct asd_flash_dir *flash_dir)
{
- int err, i;
+ int err;
u32 offs, size;
struct asd_ll_el *el;
struct asd_ctrla_phy_settings *ps;
- struct asd_ctrla_phy_settings dflt_ps;
err = asd_find_flash_de(flash_dir, FLASH_DE_CTRL_A_USER, &offs, &size);
if (err) {
ASD_DPRINTK("couldn't find CTRL-A user settings section\n");
- ASD_DPRINTK("Creating default CTRL-A user settings section\n");
-
- dflt_ps.id0 = 'h';
- dflt_ps.num_phys = 8;
- for (i =0; i < ASD_MAX_PHYS; i++) {
- memcpy(dflt_ps.phy_ent[i].sas_addr,
- asd_ha->hw_prof.sas_addr, SAS_ADDR_SIZE);
- dflt_ps.phy_ent[i].sas_link_rates = 0x98;
- dflt_ps.phy_ent[i].flags = 0x0;
- dflt_ps.phy_ent[i].sata_link_rates = 0x0;
- }
-
- size = sizeof(struct asd_ctrla_phy_settings);
- ps = &dflt_ps;
+ ASD_DPRINTK("Using default settings\n");
+ return 0;
}
if (size == 0)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [SCSI] hpsa: dial down lockup detection during firmware flash
On Wed, 2012-10-10 at 09:18 -0500, scame...@beardog.cce.hp.com wrote: > Yes, I think so. Thanks. I've queued this up for 3.2, thanks. Ben. > -- steve > > On Wed, Oct 10, 2012 at 04:47:38AM +0100, Ben Hutchings wrote: > > Should this fix for hpsa be included in stable updates? It looks like > > it would be needed in 3.2.y and 3.4.y, as lockup detection was > > introduced in 3.2 and the fix went into 3.5. > > > > commit e85c59746957fd6e3595d02cf614370056b5816e > > Author: Stephen M. Cameron > > Date: Tue May 1 11:43:42 2012 -0500 > > > > [SCSI] hpsa: dial down lockup detection during firmware flash > > > > Ben. > > > > -- > > Ben Hutchings > > Who are all these weirdos? - David Bowie, about L-Space IRC channel #afp > > > -- Ben Hutchings Always try to do things in chronological order; it's less confusing that way. signature.asc Description: This is a digitally signed message part
[SCSI] hpsa: dial down lockup detection during firmware flash
Should this fix for hpsa be included in stable updates? It looks like it would be needed in 3.2.y and 3.4.y, as lockup detection was introduced in 3.2 and the fix went into 3.5. commit e85c59746957fd6e3595d02cf614370056b5816e Author: Stephen M. Cameron Date: Tue May 1 11:43:42 2012 -0500 [SCSI] hpsa: dial down lockup detection during firmware flash Ben. -- Ben Hutchings Who are all these weirdos? - David Bowie, about L-Space IRC channel #afp signature.asc Description: This is a digitally signed message part
Re: [V2 PATCH 6/9] csiostor: Chelsio FCoE offload driver submission (headers part 1).
On Wed, 2012-09-05 at 22:56 +0530, Naresh Kumar Inna wrote: > On 9/5/2012 10:01 PM, Stephen Hemminger wrote: > > On Wed, 5 Sep 2012 18:03:59 +0530 > > Naresh Kumar Inna wrote: > > > >> +#define CSIO_ROUNDUP(__v, __r)(((__v) + (__r) - 1) / (__r)) > > > > This is similar to existing round_up() in kernel.h could you use that? > > > I will replace it with round_up() if it serves the same purpose. Thanks. Stephen is probably thinking of DIV_ROUND_UP(). round_up() does something different. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list
On Wed, 2012-07-25 at 20:13 +0800, Wang Sen wrote: > When using the commands below to write some data to a virtio-scsi LUN of the > QEMU guest(32-bit) with 1G physical memory(qemu -m 1024), the qemu will crash. > > # sudo mkfs.ext4 /dev/sdb (/dev/sdb is the virtio-scsi LUN.) > # sudo mount /dev/sdb /mnt > # dd if=/dev/zero of=/mnt/file bs=1M count=1024 > > In current implementation, sg_set_buf is called to add buffers to sg list > which > is put into the virtqueue eventually. But if there are some HighMem pages in > table->sgl you can not get virtual address by sg_virt. So, sg_virt(sg_elem) > may > return NULL value. This will cause QEMU exit when virtqueue_map_sg is called > in QEMU because an invalid GPA is passed by virtqueue. > > I take Paolo's solution mentioned in last thread to avoid failure on handling > flag bits. > > I have tested the patch on my workstation. QEMU would not crash any more. > > Signed-off-by: Wang Sen [...] This is not the correct way to submit a change for stable. See Documentation/stable_kernel_rules.txt. Ben. -- Ben Hutchings If more than one person is responsible for a bug, no one is at fault. signature.asc Description: This is a digitally signed message part
