[v2,10/11] Revert "scsi: qla2xxx: Fix crash on qla2x00_mailbox_command"
diff mbox series

Message ID 20200806111014.28434-11-njavali@marvell.com
State Accepted
Headers show
Series
  • qla2xxx driver bug fixes
Related show

Commit Message

Nilesh Javali Aug. 6, 2020, 11:10 a.m. UTC
FCoE adapter initialization failed for ISP8021.

This reverts commit 3cb182b3fa8b7a61f05c671525494697cba39c6a.

Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_mbx.c | 8 --------
 1 file changed, 8 deletions(-)

Comments

Daniel Wagner Aug. 7, 2020, 7:54 a.m. UTC | #1
On Thu, Aug 06, 2020 at 04:10:13AM -0700, Nilesh Javali wrote:
> FCoE adapter initialization failed for ISP8021.
> 
> This reverts commit 3cb182b3fa8b7a61f05c671525494697cba39c6a.

But wouldn't this revert not also bring back the crash from 3cb182b3fa8b
("scsi: qla2xxx: Fix crash on qla2x00_mailbox_command"):

    This patch fixes a crash on qla2x00_mailbox_command caused when the driver
    is on UNLOADING state and tries to call qla2x00_poll, which triggers a
    NULL pointer dereference.
Saurav Kashyap Aug. 10, 2020, 9:55 a.m. UTC | #2
Hi Daniel,

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org <linux-scsi-owner@vger.kernel.org>
> On Behalf Of Daniel Wagner
> Sent: Friday, August 7, 2020 1:24 PM
> To: Nilesh Javali <njavali@marvell.com>
> Cc: martin.petersen@oracle.com; linux-scsi@vger.kernel.org; GR-QLogic-
> Storage-Upstream <GR-QLogic-Storage-Upstream@marvell.com>
> Subject: Re: [PATCH v2 10/11] Revert "scsi: qla2xxx: Fix crash on
> qla2x00_mailbox_command"
> 
> On Thu, Aug 06, 2020 at 04:10:13AM -0700, Nilesh Javali wrote:
> > FCoE adapter initialization failed for ISP8021.
> >
> > This reverts commit 3cb182b3fa8b7a61f05c671525494697cba39c6a.
> 
> But wouldn't this revert not also bring back the crash from 3cb182b3fa8b
> ("scsi: qla2xxx: Fix crash on qla2x00_mailbox_command"):

This patch was never there in OOT driver, and we never hit an original problem. I have tested this patch myself
and this have gone through test cycles as well. If an original issue is hit again, we will do an analysis and provide
the fix. This revert fixes a load issues with ISP82XX.

Thanks,
~Saurav
> 
>     This patch fixes a crash on qla2x00_mailbox_command caused when the
> driver
>     is on UNLOADING state and tries to call qla2x00_poll, which triggers a
>     NULL pointer dereference.
Himanshu Madhani Aug. 12, 2020, 7:31 p.m. UTC | #3
> On Aug 10, 2020, at 4:55 AM, Saurav Kashyap <skashyap@marvell.com> wrote:
> 
> Hi Daniel,
> 
>> -----Original Message-----
>> From: linux-scsi-owner@vger.kernel.org <linux-scsi-owner@vger.kernel.org>
>> On Behalf Of Daniel Wagner
>> Sent: Friday, August 7, 2020 1:24 PM
>> To: Nilesh Javali <njavali@marvell.com>
>> Cc: martin.petersen@oracle.com; linux-scsi@vger.kernel.org; GR-QLogic-
>> Storage-Upstream <GR-QLogic-Storage-Upstream@marvell.com>
>> Subject: Re: [PATCH v2 10/11] Revert "scsi: qla2xxx: Fix crash on
>> qla2x00_mailbox_command"
>> 
>> On Thu, Aug 06, 2020 at 04:10:13AM -0700, Nilesh Javali wrote:
>>> FCoE adapter initialization failed for ISP8021.
>>> 
>>> This reverts commit 3cb182b3fa8b7a61f05c671525494697cba39c6a.
>> 
>> But wouldn't this revert not also bring back the crash from 3cb182b3fa8b
>> ("scsi: qla2xxx: Fix crash on qla2x00_mailbox_command"):
> 
> This patch was never there in OOT driver, and we never hit an original problem. I have tested this patch myself
> and this have gone through test cycles as well. If an original issue is hit again, we will do an analysis and provide
> the fix. This revert fixes a load issues with ISP82XX.
> 
> Thanks,
> ~Saurav
>> 
>>    This patch fixes a crash on qla2x00_mailbox_command caused when the
>> driver
>>    is on UNLOADING state and tries to call qla2x00_poll, which triggers a
>>    NULL pointer dereference.
> 

If my memory serves me right. This crash reported was never reproduced during my testing when this patch was submitted and nor did it cause any issue. So I am fine with this change being reverted. 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	 Oracle Linux Engineering
Daniel Wagner Aug. 24, 2020, 8:04 a.m. UTC | #4
Hi Saurav,

Sorry for the late response, was on holiday.

> On Aug 10, 2020, at 4:55 AM, Saurav Kashyap <skashyap@marvell.com> wrote:
> This patch was never there in OOT driver, and we never hit an original problem. I have tested this patch myself
> and this have gone through test cycles as well. If an original issue is hit again, we will do an analysis and provide
> the fix. This revert fixes a load issues with ISP82XX.

Ok. Maybe this info could be added to the commit message (next time).

Thanks,
Daniel
Saurav Kashyap Aug. 24, 2020, 10:49 a.m. UTC | #5
> -----Original Message-----
> From: Daniel Wagner <dwagner@suse.de>
> Sent: Monday, August 24, 2020 1:34 PM
> To: Himanshu Madhani <himanshu.madhani@oracle.com>
> Cc: Saurav Kashyap <skashyap@marvell.com>; Nilesh Javali
> <njavali@marvell.com>; martin.petersen@oracle.com; linux-
> scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-
> Upstream@marvell.com>
> Subject: [EXT] Re: [PATCH v2 10/11] Revert "scsi: qla2xxx: Fix crash on
> qla2x00_mailbox_command"
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Saurav,
> 
> Sorry for the late response, was on holiday.
> 
> > On Aug 10, 2020, at 4:55 AM, Saurav Kashyap <skashyap@marvell.com>
> wrote:
> > This patch was never there in OOT driver, and we never hit an original
> problem. I have tested this patch myself
> > and this have gone through test cycles as well. If an original issue is hit again,
> we will do an analysis and provide
> > the fix. This revert fixes a load issues with ISP82XX.
> 
> Ok. Maybe this info could be added to the commit message (next time).

Make sense.

Thanks,
~Saurav
> 
> Thanks,
> Daniel
> 
>

Patch
diff mbox series

diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index 73883435ab58..93aafef7f21c 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -334,14 +334,6 @@  qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t *mcp)
 			if (time_after(jiffies, wait_time))
 				break;
 
-			/*
-			 * Check if it's UNLOADING, cause we cannot poll in
-			 * this case, or else a NULL pointer dereference
-			 * is triggered.
-			 */
-			if (unlikely(test_bit(UNLOADING, &base_vha->dpc_flags)))
-				return QLA_FUNCTION_TIMEOUT;
-
 			/* Check for pending interrupts. */
 			qla2x00_poll(ha->rsp_q_map[0]);