From: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
To: linux-scsi@vger.kernel.org
Cc: martin.petersen@oracle.com, mpi3mr-linuxdrv.pdl@broadcom.com,
Sreekanth Reddy <sreekanth.reddy@broadcom.com>
Subject: [PATCH 1/9] mpi3mr: Fix deadlock while canceling the fw event
Date: Thu, 10 Feb 2022 15:28:09 +0530 [thread overview]
Message-ID: <20220210095817.22828-2-sreekanth.reddy@broadcom.com> (raw)
In-Reply-To: <20220210095817.22828-1-sreekanth.reddy@broadcom.com>
[-- Attachment #1: Type: text/plain, Size: 7871 bytes --]
During controller reset, the driver tries to flush all
the pending firmware event works from worker queue that are queued
prior to the reset. However, if any work is waiting for
device addition/removal operation to be completed at the
SML then a deadlock is observed. This is due to the reason that
the controller reset is waiting for the device addition/removal
to be completed and the device/addition removal is waiting for
the controller reset to be completed.
So, to limit this deadlock, continue with the controller reset
handling without canceling the work which is waiting for
device addition/removal operation to complete at SML.
Signed-off-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
---
drivers/scsi/mpi3mr/mpi3mr.h | 4 ++
drivers/scsi/mpi3mr/mpi3mr_fw.c | 2 +-
drivers/scsi/mpi3mr/mpi3mr_os.c | 106 ++++++++++++++++++++++++++------
3 files changed, 91 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index fc4eaf6..d892ade 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -866,6 +866,8 @@ struct mpi3mr_ioc {
* @send_ack: Event acknowledgment required or not
* @process_evt: Bottomhalf processing required or not
* @evt_ctx: Event context to send in Ack
+ * @pending_at_sml: waiting for device add/remove API to complete
+ * @discard: discard this event
* @ref_count: kref count
* @event_data: Actual MPI3 event data
*/
@@ -877,6 +879,8 @@ struct mpi3mr_fwevt {
bool send_ack;
bool process_evt;
u32 evt_ctx;
+ bool pending_at_sml;
+ bool discard;
struct kref ref_count;
char event_data[0] __aligned(4);
};
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 15bdc21..7193b98 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -4353,8 +4353,8 @@ int mpi3mr_soft_reset_handler(struct mpi3mr_ioc *mrioc,
memset(mrioc->devrem_bitmap, 0, mrioc->devrem_bitmap_sz);
memset(mrioc->removepend_bitmap, 0, mrioc->dev_handle_bitmap_sz);
memset(mrioc->evtack_cmds_bitmap, 0, mrioc->evtack_cmds_bitmap_sz);
- mpi3mr_cleanup_fwevt_list(mrioc);
mpi3mr_flush_host_io(mrioc);
+ mpi3mr_cleanup_fwevt_list(mrioc);
mpi3mr_invalidate_devhandles(mrioc);
if (mrioc->prepare_for_reset) {
mrioc->prepare_for_reset = 0;
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 284117d..90911ea 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -285,6 +285,35 @@ static struct mpi3mr_fwevt *mpi3mr_dequeue_fwevt(
return fwevt;
}
+/**
+ * mpi3mr_cancel_work - cancel firmware event
+ * @fwevt: fwevt object which needs to be canceled
+ *
+ * Return: Nothing.
+ */
+static void mpi3mr_cancel_work(struct mpi3mr_fwevt *fwevt)
+{
+ /*
+ * Wait on the fwevt to complete. If this returns 1, then
+ * the event was never executed.
+ *
+ * If it did execute, we wait for it to finish, and the put will
+ * happen from mpi3mr_process_fwevt()
+ */
+ if (cancel_work_sync(&fwevt->work)) {
+ /*
+ * Put fwevt reference count after
+ * dequeuing it from worker queue
+ */
+ mpi3mr_fwevt_put(fwevt);
+ /*
+ * Put fwevt reference count to neutralize
+ * kref_init increment
+ */
+ mpi3mr_fwevt_put(fwevt);
+ }
+}
+
/**
* mpi3mr_cleanup_fwevt_list - Cleanup firmware event list
* @mrioc: Adapter instance reference
@@ -302,28 +331,25 @@ void mpi3mr_cleanup_fwevt_list(struct mpi3mr_ioc *mrioc)
!mrioc->fwevt_worker_thread)
return;
- while ((fwevt = mpi3mr_dequeue_fwevt(mrioc)) ||
- (fwevt = mrioc->current_event)) {
+ while ((fwevt = mpi3mr_dequeue_fwevt(mrioc)))
+ mpi3mr_cancel_work(fwevt);
+
+ if (mrioc->current_event) {
+ fwevt = mrioc->current_event;
/*
- * Wait on the fwevt to complete. If this returns 1, then
- * the event was never executed, and we need a put for the
- * reference the work had on the fwevt.
- *
- * If it did execute, we wait for it to finish, and the put will
- * happen from mpi3mr_process_fwevt()
+ * Don't call cancel_work_sync() API for the
+ * fwevt work if the controller reset is
+ * get called as part of processing the
+ * same fwevt work (or) when worker thread is
+ * waiting for device add/remove APIs to complete.
+ * Otherwise we will see deadlock.
*/
- if (cancel_work_sync(&fwevt->work)) {
- /*
- * Put fwevt reference count after
- * dequeuing it from worker queue
- */
- mpi3mr_fwevt_put(fwevt);
- /*
- * Put fwevt reference count to neutralize
- * kref_init increment
- */
- mpi3mr_fwevt_put(fwevt);
+ if (current_work() == &fwevt->work || fwevt->pending_at_sml) {
+ fwevt->discard = 1;
+ return;
}
+
+ mpi3mr_cancel_work(fwevt);
}
}
@@ -690,6 +716,24 @@ static struct mpi3mr_tgt_dev *__mpi3mr_get_tgtdev_from_tgtpriv(
return tgtdev;
}
+/**
+ * mpi3mr_print_device_event_notice - print notice related to post processing of
+ * device event after controller reset.
+ *
+ * @mrioc: Adapter instance reference
+ * @device_add: true for device add event and false for device removal event
+ *
+ * Return: None.
+ */
+static void mpi3mr_print_device_event_notice(struct mpi3mr_ioc *mrioc,
+ bool device_add)
+{
+ ioc_notice(mrioc, "Device %s was under process before the reset and\n",
+ (device_add ? "addition" : "removal"));
+ ioc_notice(mrioc, "completed after reset, verify whether the exposed devices\n");
+ ioc_notice(mrioc, "are matched with attached devices for correctness\n");
+}
+
/**
* mpi3mr_remove_tgtdev_from_host - Remove dev from upper layers
* @mrioc: Adapter instance reference
@@ -714,8 +758,17 @@ static void mpi3mr_remove_tgtdev_from_host(struct mpi3mr_ioc *mrioc,
}
if (tgtdev->starget) {
+ if (mrioc->current_event)
+ mrioc->current_event->pending_at_sml = 1;
scsi_remove_target(&tgtdev->starget->dev);
tgtdev->host_exposed = 0;
+ if (mrioc->current_event) {
+ mrioc->current_event->pending_at_sml = 0;
+ if (mrioc->current_event->discard) {
+ mpi3mr_print_device_event_notice(mrioc, false);
+ return;
+ }
+ }
}
ioc_info(mrioc, "%s :Removed handle(0x%04x), wwid(0x%016llx)\n",
__func__, tgtdev->dev_handle, (unsigned long long)tgtdev->wwid);
@@ -749,11 +802,20 @@ static int mpi3mr_report_tgtdev_to_host(struct mpi3mr_ioc *mrioc,
}
if (!tgtdev->host_exposed && !mrioc->reset_in_progress) {
tgtdev->host_exposed = 1;
+ if (mrioc->current_event)
+ mrioc->current_event->pending_at_sml = 1;
scsi_scan_target(&mrioc->shost->shost_gendev, 0,
tgtdev->perst_id,
SCAN_WILD_CARD, SCSI_SCAN_INITIAL);
if (!tgtdev->starget)
tgtdev->host_exposed = 0;
+ if (mrioc->current_event) {
+ mrioc->current_event->pending_at_sml = 0;
+ if (mrioc->current_event->discard) {
+ mpi3mr_print_device_event_notice(mrioc, true);
+ goto out;
+ }
+ }
}
out:
if (tgtdev)
@@ -1193,6 +1255,8 @@ static void mpi3mr_sastopochg_evt_bh(struct mpi3mr_ioc *mrioc,
mpi3mr_sastopochg_evt_debug(mrioc, event_data);
for (i = 0; i < event_data->num_entries; i++) {
+ if (fwevt->discard)
+ return;
handle = le16_to_cpu(event_data->phy_entry[i].attached_dev_handle);
if (!handle)
continue;
@@ -1324,6 +1388,8 @@ static void mpi3mr_pcietopochg_evt_bh(struct mpi3mr_ioc *mrioc,
mpi3mr_pcietopochg_evt_debug(mrioc, event_data);
for (i = 0; i < event_data->num_entries; i++) {
+ if (fwevt->discard)
+ return;
handle =
le16_to_cpu(event_data->port_entry[i].attached_dev_handle);
if (!handle)
@@ -1362,8 +1428,8 @@ static void mpi3mr_pcietopochg_evt_bh(struct mpi3mr_ioc *mrioc,
static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc,
struct mpi3mr_fwevt *fwevt)
{
- mrioc->current_event = fwevt;
mpi3mr_fwevt_del_from_list(mrioc, fwevt);
+ mrioc->current_event = fwevt;
if (mrioc->stop_drv_processing)
goto out;
--
2.27.0
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]
next prev parent reply other threads:[~2022-02-10 9:48 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-10 9:58 [PATCH 0/9] mpi3mr: Bug fixes Sreekanth Reddy
2022-02-10 9:58 ` Sreekanth Reddy [this message]
2022-02-10 9:58 ` [PATCH 2/9] mpi3mr: Fix print proper pending IO count Sreekanth Reddy
2022-02-10 9:58 ` [PATCH 3/9] mpi3mr: update MPI3 headers Sreekanth Reddy
2022-02-10 9:58 ` [PATCH 4/9] mpi3mr: Fix hibernation issue Sreekanth Reddy
2022-02-10 9:58 ` [PATCH 5/9] mpi3mr: Fix cmnd getting marked as inuse forever Sreekanth Reddy
2022-02-10 9:58 ` [PATCH 6/9] mpi3mr: Fix report actual data transfer sz Sreekanth Reddy
2022-02-10 9:58 ` [PATCH 7/9] mpi3mr: Update the copyright year Sreekanth Reddy
2022-02-10 9:58 ` [PATCH 8/9] mpi3mr: Fix memory leaks Sreekanth Reddy
2022-02-10 9:58 ` [PATCH 9/9] mpi3mr: Bump driver version to 8.0.0.68.0 Sreekanth Reddy
2022-02-11 22:45 ` [PATCH 0/9] mpi3mr: Bug fixes Martin K. Petersen
2022-02-15 3:19 ` Martin K. Petersen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220210095817.22828-2-sreekanth.reddy@broadcom.com \
--to=sreekanth.reddy@broadcom.com \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=mpi3mr-linuxdrv.pdl@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.