megaraid_sas : MFI MPT linked list corruption fix
Resending the patch. Addressed the review comments from Tomas Henzl.
Added comment for to-do work.
Problem statement:
MFI link list in megaraid_sas driver is used from mfi-mpt pass-through commands.
This list can be corrupted due to many possible race conditions in driver and
eventually we may see kernel panic.
One example -
MFI frame is freed from calling process as driver send command via polling method and interrupt
for that command comes after driver free mfi frame (actually even after some other context reuse
the mfi frame). When driver receive MPT frame in ISR, driver will be using the index of MFI and
access that MFI frame and finally in-used MFI frame’s list will be corrupted.
High level description of new solution -
Free MFI and MPT command from same context.
Free both the command either from process (from where mfi-mpt pass-through was called) or from
ISR context. Do not split freeing of MFI and MPT, because it creates the race condition which
will do MFI/MPT list corruption.
Renamed the cmd_pool_lock which is used in instance as well as fusion with below name.
mfi_pool_lock and mpt_pool_lock to add more code readability.
Signed-off-by: Sumit Saxena <sumit.saxena@avagotech.com>
Signed-off-by: Kashyap Desai <kashyap.desai@avagotech.com>
Reviewed-by: Tomas Henzl <thenzl@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 4c4c266..d8cc922 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -210,21 +210,54 @@
unsigned long flags;
struct megasas_cmd *cmd = NULL;
- spin_lock_irqsave(&instance->cmd_pool_lock, flags);
+ spin_lock_irqsave(&instance->mfi_pool_lock, flags);
if (!list_empty(&instance->cmd_pool)) {
cmd = list_entry((&instance->cmd_pool)->next,
struct megasas_cmd, list);
list_del_init(&cmd->list);
+ atomic_set(&cmd->mfi_mpt_pthr, MFI_MPT_DETACHED);
} else {
printk(KERN_ERR "megasas: Command pool empty!\n");
}
- spin_unlock_irqrestore(&instance->cmd_pool_lock, flags);
+ spin_unlock_irqrestore(&instance->mfi_pool_lock, flags);
return cmd;
}
/**
+ * __megasas_return_cmd - Return a cmd to free command pool
+ * @instance: Adapter soft state
+ * @cmd: Command packet to be returned to free command pool
+ */
+inline void
+__megasas_return_cmd(struct megasas_instance *instance, struct megasas_cmd *cmd)
+{
+ /*
+ * Don't go ahead and free the MFI frame, if corresponding
+ * MPT frame is not freed(valid for only fusion adapters).
+ * In case of MFI adapters, anyways for any allocated MFI
+ * frame will have cmd->mfi_mpt_mpthr set to MFI_MPT_DETACHED
+ */
+ if (atomic_read(&cmd->mfi_mpt_pthr) != MFI_MPT_DETACHED)
+ return;
+
+ cmd->scmd = NULL;
+ cmd->frame_count = 0;
+ cmd->is_wait_event = 0;
+ cmd->mpt_pthr_cmd_blocked = NULL;
+
+ if ((instance->pdev->device != PCI_DEVICE_ID_LSI_FUSION) &&
+ (instance->pdev->device != PCI_DEVICE_ID_LSI_INVADER) &&
+ (instance->pdev->device != PCI_DEVICE_ID_LSI_FURY) &&
+ (reset_devices))
+ cmd->frame->hdr.cmd = MFI_CMD_INVALID;
+
+ atomic_set(&cmd->mfi_mpt_pthr, MFI_LIST_ADDED);
+ list_add(&cmd->list, (&instance->cmd_pool)->next);
+}
+
+/**
* megasas_return_cmd - Return a cmd to free command pool
* @instance: Adapter soft state
* @cmd: Command packet to be returned to free command pool
@@ -234,19 +267,9 @@
{
unsigned long flags;
- spin_lock_irqsave(&instance->cmd_pool_lock, flags);
-
- cmd->scmd = NULL;
- cmd->frame_count = 0;
- if ((instance->pdev->device != PCI_DEVICE_ID_LSI_FUSION) &&
- (instance->pdev->device != PCI_DEVICE_ID_LSI_PLASMA) &&
- (instance->pdev->device != PCI_DEVICE_ID_LSI_INVADER) &&
- (instance->pdev->device != PCI_DEVICE_ID_LSI_FURY) &&
- (reset_devices))
- cmd->frame->hdr.cmd = MFI_CMD_INVALID;
- list_add_tail(&cmd->list, &instance->cmd_pool);
-
- spin_unlock_irqrestore(&instance->cmd_pool_lock, flags);
+ spin_lock_irqsave(&instance->mfi_pool_lock, flags);
+ __megasas_return_cmd(instance, cmd);
+ spin_unlock_irqrestore(&instance->mfi_pool_lock, flags);
}
@@ -925,13 +948,14 @@
* Max wait time is MEGASAS_INTERNAL_CMD_WAIT_TIME secs
* Used to issue ioctl commands.
*/
-static int
+int
megasas_issue_blocked_cmd(struct megasas_instance *instance,
struct megasas_cmd *cmd, int timeout)
{
int ret = 0;
cmd->cmd_status = ENODATA;
+ cmd->is_wait_event = 1;
instance->instancet->issue_dcmd(instance, cmd);
if (timeout) {
ret = wait_event_timeout(instance->int_cmd_wait_q,
@@ -1903,7 +1927,12 @@
new_affiliation_111,
new_affiliation_111_h);
}
- megasas_return_cmd(instance, cmd);
+
+ if (instance->ctrl_context && cmd->mpt_pthr_cmd_blocked)
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
+ else
+ megasas_return_cmd(instance, cmd);
return retval;
}
@@ -2070,7 +2099,11 @@
(MAX_LOGICAL_DRIVES + 1) *
sizeof(struct MR_LD_VF_AFFILIATION),
new_affiliation, new_affiliation_h);
- megasas_return_cmd(instance, cmd);
+ if (instance->ctrl_context && cmd->mpt_pthr_cmd_blocked)
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
+ else
+ megasas_return_cmd(instance, cmd);
return retval;
}
@@ -2530,7 +2563,12 @@
cmd->abort_aen = 0;
instance->aen_cmd = NULL;
- megasas_return_cmd(instance, cmd);
+
+ if (instance->ctrl_context && cmd->mpt_pthr_cmd_blocked)
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
+ else
+ megasas_return_cmd(instance, cmd);
if ((instance->unload == 0) &&
((instance->issuepend_done == 1))) {
@@ -2906,7 +2944,8 @@
"failed, status = 0x%x.\n",
cmd->frame->hdr.cmd_status);
else {
- megasas_return_cmd(instance, cmd);
+ megasas_return_mfi_mpt_pthr(instance,
+ cmd, cmd->mpt_pthr_cmd_blocked);
spin_unlock_irqrestore(
instance->host->host_lock,
flags);
@@ -2914,7 +2953,8 @@
}
} else
instance->map_id++;
- megasas_return_cmd(instance, cmd);
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
/*
* Set fast path IO to ZERO.
@@ -3070,7 +3110,7 @@
unsigned long flags;
defer_index = 0;
- spin_lock_irqsave(&instance->cmd_pool_lock, flags);
+ spin_lock_irqsave(&instance->mfi_pool_lock, flags);
for (i = 0; i < max_cmd; i++) {
cmd = instance->cmd_list[i];
if (cmd->sync_cmd == 1 || cmd->scmd) {
@@ -3091,7 +3131,7 @@
&instance->internal_reset_pending_q);
}
}
- spin_unlock_irqrestore(&instance->cmd_pool_lock, flags);
+ spin_unlock_irqrestore(&instance->mfi_pool_lock, flags);
}
@@ -3656,7 +3696,9 @@
int j;
u32 max_cmd;
struct megasas_cmd *cmd;
+ struct fusion_context *fusion;
+ fusion = instance->ctrl_context;
max_cmd = instance->max_mfi_cmds;
/*
@@ -3689,13 +3731,11 @@
}
}
- /*
- * Add all the commands to command pool (instance->cmd_pool)
- */
for (i = 0; i < max_cmd; i++) {
cmd = instance->cmd_list[i];
memset(cmd, 0, sizeof(struct megasas_cmd));
cmd->index = i;
+ atomic_set(&cmd->mfi_mpt_pthr, MFI_LIST_ADDED);
cmd->scmd = NULL;
cmd->instance = instance;
@@ -3766,11 +3806,11 @@
dcmd->sgl.sge32[0].phys_addr = cpu_to_le32(ci_h);
dcmd->sgl.sge32[0].length = cpu_to_le32(MEGASAS_MAX_PD * sizeof(struct MR_PD_LIST));
- if (!megasas_issue_polled(instance, cmd)) {
- ret = 0;
- } else {
- ret = -1;
- }
+ if (instance->ctrl_context && !instance->mask_interrupts)
+ ret = megasas_issue_blocked_cmd(instance, cmd,
+ MEGASAS_BLOCKED_CMD_TIMEOUT);
+ else
+ ret = megasas_issue_polled(instance, cmd);
/*
* the following function will get the instance PD LIST.
@@ -3802,7 +3842,12 @@
pci_free_consistent(instance->pdev,
MEGASAS_MAX_PD * sizeof(struct MR_PD_LIST),
ci, ci_h);
- megasas_return_cmd(instance, cmd);
+
+ if (instance->ctrl_context && cmd->mpt_pthr_cmd_blocked)
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
+ else
+ megasas_return_cmd(instance, cmd);
return ret;
}
@@ -3861,11 +3906,12 @@
dcmd->sgl.sge32[0].length = cpu_to_le32(sizeof(struct MR_LD_LIST));
dcmd->pad_0 = 0;
- if (!megasas_issue_polled(instance, cmd)) {
- ret = 0;
- } else {
- ret = -1;
- }
+ if (instance->ctrl_context && !instance->mask_interrupts)
+ ret = megasas_issue_blocked_cmd(instance, cmd,
+ MEGASAS_BLOCKED_CMD_TIMEOUT);
+ else
+ ret = megasas_issue_polled(instance, cmd);
+
ld_count = le32_to_cpu(ci->ldCount);
@@ -3888,7 +3934,11 @@
ci,
ci_h);
- megasas_return_cmd(instance, cmd);
+ if (instance->ctrl_context && cmd->mpt_pthr_cmd_blocked)
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
+ else
+ megasas_return_cmd(instance, cmd);
return ret;
}
@@ -3949,12 +3999,11 @@
dcmd->sgl.sge32[0].length = cpu_to_le32(sizeof(struct MR_LD_TARGETID_LIST));
dcmd->pad_0 = 0;
- if (!megasas_issue_polled(instance, cmd) && !dcmd->cmd_status) {
- ret = 0;
- } else {
- /* On failure, call older LD list DCMD */
- ret = 1;
- }
+ if (instance->ctrl_context && !instance->mask_interrupts)
+ ret = megasas_issue_blocked_cmd(instance, cmd,
+ MEGASAS_BLOCKED_CMD_TIMEOUT);
+ else
+ ret = megasas_issue_polled(instance, cmd);
tgtid_count = le32_to_cpu(ci->count);
@@ -3970,7 +4019,11 @@
pci_free_consistent(instance->pdev, sizeof(struct MR_LD_TARGETID_LIST),
ci, ci_h);
- megasas_return_cmd(instance, cmd);
+ if (instance->ctrl_context && cmd->mpt_pthr_cmd_blocked)
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
+ else
+ megasas_return_cmd(instance, cmd);
return ret;
}
@@ -4027,17 +4080,23 @@
dcmd->sgl.sge32[0].length = cpu_to_le32(sizeof(struct megasas_ctrl_info));
dcmd->mbox.b[0] = 1;
- if (!megasas_issue_polled(instance, cmd)) {
- ret = 0;
+ if (instance->ctrl_context && !instance->mask_interrupts)
+ ret = megasas_issue_blocked_cmd(instance, cmd,
+ MEGASAS_BLOCKED_CMD_TIMEOUT);
+ else
+ ret = megasas_issue_polled(instance, cmd);
+
+ if (!ret)
memcpy(ctrl_info, ci, sizeof(struct megasas_ctrl_info));
- } else {
- ret = -1;
- }
pci_free_consistent(instance->pdev, sizeof(struct megasas_ctrl_info),
ci, ci_h);
- megasas_return_cmd(instance, cmd);
+ if (instance->ctrl_context && cmd->mpt_pthr_cmd_blocked)
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
+ else
+ megasas_return_cmd(instance, cmd);
return ret;
}
@@ -4086,11 +4145,17 @@
dcmd->sgl.sge32[0].phys_addr = cpu_to_le32(instance->crash_dump_h);
dcmd->sgl.sge32[0].length = cpu_to_le32(CRASH_DMA_BUF_SIZE);
- if (!megasas_issue_polled(instance, cmd))
- ret = 0;
+ if (instance->ctrl_context && !instance->mask_interrupts)
+ ret = megasas_issue_blocked_cmd(instance, cmd,
+ MEGASAS_BLOCKED_CMD_TIMEOUT);
else
- ret = -1;
- megasas_return_cmd(instance, cmd);
+ ret = megasas_issue_polled(instance, cmd);
+
+ if (instance->ctrl_context && cmd->mpt_pthr_cmd_blocked)
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
+ else
+ megasas_return_cmd(instance, cmd);
return ret;
}
@@ -4660,7 +4725,11 @@
pci_free_consistent(instance->pdev, sizeof(struct megasas_evt_log_info),
el_info, el_info_h);
- megasas_return_cmd(instance, cmd);
+ if (instance->ctrl_context && cmd->mpt_pthr_cmd_blocked)
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
+ else
+ megasas_return_cmd(instance, cmd);
return 0;
}
@@ -5015,7 +5084,7 @@
}
fusion = instance->ctrl_context;
INIT_LIST_HEAD(&fusion->cmd_pool);
- spin_lock_init(&fusion->cmd_pool_lock);
+ spin_lock_init(&fusion->mpt_pool_lock);
memset(fusion->load_balance_info, 0,
sizeof(struct LD_LOAD_BALANCE_INFO) * MAX_LOGICAL_DRIVES_EXT);
}
@@ -5086,7 +5155,7 @@
init_waitqueue_head(&instance->int_cmd_wait_q);
init_waitqueue_head(&instance->abort_cmd_wait_q);
- spin_lock_init(&instance->cmd_pool_lock);
+ spin_lock_init(&instance->mfi_pool_lock);
spin_lock_init(&instance->hba_lock);
spin_lock_init(&instance->completion_lock);
@@ -5106,7 +5175,7 @@
instance->flag_ieee = 1;
sema_init(&instance->ioctl_sem, MEGASAS_SKINNY_INT_CMDS);
} else
- sema_init(&instance->ioctl_sem, MEGASAS_INT_CMDS);
+ sema_init(&instance->ioctl_sem, (MEGASAS_INT_CMDS - 5));
megasas_dbg_lvl = 0;
instance->flag = 0;
@@ -5318,7 +5387,11 @@
dev_err(&instance->pdev->dev, "Command timedout"
" from %s\n", __func__);
- megasas_return_cmd(instance, cmd);
+ if (instance->ctrl_context && cmd->mpt_pthr_cmd_blocked)
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
+ else
+ megasas_return_cmd(instance, cmd);
return;
}
@@ -5365,7 +5438,11 @@
dev_err(&instance->pdev->dev, "Command timedout"
"from %s\n", __func__);
- megasas_return_cmd(instance, cmd);
+ if (instance->ctrl_context && cmd->mpt_pthr_cmd_blocked)
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
+ else
+ megasas_return_cmd(instance, cmd);
return;
}
@@ -6026,9 +6103,14 @@
le32_to_cpu(kern_sge32[i].length),
kbuff_arr[i],
le32_to_cpu(kern_sge32[i].phys_addr));
+ kbuff_arr[i] = NULL;
}
- megasas_return_cmd(instance, cmd);
+ if (instance->ctrl_context && cmd->mpt_pthr_cmd_blocked)
+ megasas_return_mfi_mpt_pthr(instance, cmd,
+ cmd->mpt_pthr_cmd_blocked);
+ else
+ megasas_return_cmd(instance, cmd);
return error;
}