Andrew Vasquez wrote:
On Mon, 28 Jul 2008, Vladislav Bolkhovitin wrote:
This patch fixes race on dpc_thread field of struct scsi_qla_host,
which can lead to crash on the module unload.
This patch is against 2.6.26
Signed-off-by: Vladislav Bolkhovitin <vst@xxxxxxxx>
qla_def.h | 1 +
qla_mbx.c | 2 +-
qla_os.c | 36 ++++++++++++++++++++++++++----------
3 files changed, 28 insertions(+), 11 deletions(-)
diff -upr linux-2.6.26/drivers/scsi/qla2xxx/qla_def.h linux-2.6.26/drivers/scsi/qla2xxx/qla_def.h
--- linux-2.6.26/drivers/scsi/qla2xxx/qla_def.h 2008-07-14 01:51:29.000000000 +0400
+++ linux-2.6.26/drivers/scsi/qla2xxx/qla_def.h 2008-07-24 10:07:39.000000000 +0400
@@ -2425,6 +2436,7 @@ typedef struct scsi_qla_host {
void *sfp_data;
dma_addr_t sfp_data_dma;
+ spinlock_t dpc_lock;
As James mentioned in another email, there's something else going on
with qla2xxx shutdown process which should be addressed...
struct task_struct *dpc_thread;
uint8_t dpc_active; /* DPC routine is active */
diff -upr linux-2.6.26/drivers/scsi/qla2xxx/qla_mbx.c linux-2.6.26/drivers/scsi/qla2xxx/qla_mbx.c
--- linux-2.6.26/drivers/scsi/qla2xxx/qla_mbx.c 2008-07-14 01:51:29.000000000 +0400
+++ linux-2.6.26/drivers/scsi/qla2xxx/qla_mbx.c 2008-07-23 19:27:20.000000000 +0400
@@ -2683,7 +2687,7 @@ qla24xx_report_id_acquisition(scsi_qla_h
set_bit(VP_IDX_ACQUIRED, &vha->vp_flags);
set_bit(VP_DPC_NEEDED, &ha->dpc_flags);
- wake_up_process(ha->dpc_thread);
+ qla2xxx_wake_dpc(ha);
Ok, your change looks correct here... This is defintely the correct
way the qla24xx_report_id_acquisition() routine should be triggering
the DPC thread.
I'm guessing the backtrace in "Gal Rosen's" report pointed to
qla24xx_report_id_acquisition()? If, so, I'm not entirely sure we
need anything more to 'protect' tear-down...
Nope, taking only one that hunk from this patch isn't sufficient. Around
dpc_thread there is pretty simple and classical race. You can't do
if (x != NULL)
y = *x;
without any protection, if x can be set to NULL by another thread. It
can happen exactly between "if" and "*x" and hence lead to a crash, correct?
What this patch does is adding such protection.
Vlad
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
