From: Amit Shah <amit@kernel.org>
To: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>,
linux-scsi <linux-scsi@vger.kernel.org>,
Sathya Prakash <sathya.prakash@broadcom.com>,
Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>,
stable@vger.kernel.org
Subject: Re: [PATCH] mpt3sas: Fix kernel panic observed on soft HBA unplug
Date: Wed, 11 Mar 2020 15:48:59 +0100 [thread overview]
Message-ID: <8bfcee3ee98003da4d6cdb81d9e80f860d85d7d5.camel@kernel.org> (raw)
In-Reply-To: <CAK=zhgp-oFoMkG_X8e5sm13=14TA5WZAHXYSeuZAV2fmUKbPow@mail.gmail.com>
On Wed, 2020-03-11 at 17:19 +0530, Sreekanth Reddy wrote:
> On Wed, Mar 11, 2020 at 4:55 PM Sreekanth Reddy
> <sreekanth.reddy@broadcom.com> wrote:
> >
> > On Wed, Mar 11, 2020 at 4:35 PM Amit Shah <amit@kernel.org> wrote:
> > >
> > > On Wed, 2020-03-11 at 06:36 -0400, Sreekanth Reddy wrote:
> > > > Generic protection fault type kernel panic is observed when
> > > > user
> > > > performs soft(ordered) HBA unplug operation while IOs are
> > > > running
> > > > on drives connected to HBA.
> > > >
> > > > When user performs ordered HBA removal operation then kernel
> > > > calls
> > > > PCI device's .remove() call back function where driver is
> > > > flushing
> > > > out
> > > > all the outstanding SCSI IO commands with DID_NO_CONNECT host
> > > > byte
> > > > and
> > > > also un-maps sg buffers allocated for these IO commands.
> > > > But in the ordered HBA removal case (unlike of real HBA hot
> > > > unplug)
> > > > HBA device is still alive and hence HBA hardware is performing
> > > > the
> > > > DMA operations to those buffers on the system memory which are
> > > > already
> > > > unmapped while flushing out the outstanding SCSI IO commands
> > > > and this leads to Kernel panic.
> > > >
> > > > Fix:
> > > > Don't flush out the outstanding IOs from .remove() path in case
> > > > of
> > > > ordered HBA removal since HBA will be still alive in this case
> > > > and
> > > > it can complete the outstanding IOs. Flush out the outstanding
> > > > IOs
> > > > only in case physical HBA hot unplug where their won't be any
> > > > communication with the HBA.
> > >
> > > Can you please point to the commit that introduces the bug?
> >
> > Sure I will add the commit ID which introduced this bug in the next
> > patch.
Thanks.
> >
> > >
> > > >
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
> > > > ---
> > > > drivers/scsi/mpt3sas/mpt3sas_scsih.c | 8 ++++----
> > > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > > > b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > > > index 778d5e6..04a40af 100644
> > > > --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > > > +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > > > @@ -9908,8 +9908,8 @@ static void scsih_remove(struct pci_dev
> > > > *pdev)
> > > >
> > > > ioc->remove_host = 1;
> > > >
> > > > - mpt3sas_wait_for_commands_to_complete(ioc);
> > > > - _scsih_flush_running_cmds(ioc);
> > > > + if (!pci_device_is_present(pdev))
> > > > + _scsih_flush_running_cmds(ioc);
> > > >
> > > > _scsih_fw_event_cleanup_queue(ioc);
> > > >
> > > > @@ -9992,8 +9992,8 @@ static void scsih_remove(struct pci_dev
> > > > *pdev)
> > >
> > > Just a note: this function is scsih_shutdown(). Doesn't block
> > > application of the patch, though. Just wondering how the patch
> > > was
> > > created.
>
> I got your query now, yes this hunk change is in scsih_shutdown()
> function. I am not sure why scsih_remove name is getting displayed
> here in this hunk. I have used 'git format-patch' to generate the
> patch.
Thanks. Does the commit description need an update as well? It only
talks about remove callback.
>
> >
> > Sorry I didn't get you. Can you please elaborate your query?
> >
> > >
> > > >
> > > > ioc->remove_host = 1;
> > > >
> > > > - mpt3sas_wait_for_commands_to_complete(ioc);
> > > > - _scsih_flush_running_cmds(ioc);
> > > > + if (!pci_device_is_present(pdev))
> > > > + _scsih_flush_running_cmds(ioc);
> > > >
> > > > _scsih_fw_event_cleanup_queue(ioc);
> > > >
next prev parent reply other threads:[~2020-03-11 14:49 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-11 10:36 [PATCH] mpt3sas: Fix kernel panic observed on soft HBA unplug Sreekanth Reddy
2020-03-11 11:04 ` Amit Shah
2020-03-11 11:25 ` Sreekanth Reddy
2020-03-11 11:49 ` Sreekanth Reddy
2020-03-11 14:48 ` Amit Shah [this message]
2020-03-14 2:25 ` Elliott, Robert (Servers)
2020-03-14 2:25 ` Elliott, Robert (Servers)
2020-03-16 6:15 ` Sreekanth Reddy
2020-03-27 1:50 ` Martin K. Petersen
2020-03-27 10:35 ` Sreekanth Reddy
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=8bfcee3ee98003da4d6cdb81d9e80f860d85d7d5.camel@kernel.org \
--to=amit@kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=sathya.prakash@broadcom.com \
--cc=sreekanth.reddy@broadcom.com \
--cc=stable@vger.kernel.org \
--cc=suganath-prabu.subramani@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).