[PATCH 0/3] qedf misc bug fixes

[PATCH 0/3] qedf misc bug fixes

[Nilesh Javali]

Martin,

Please apply the qedf driver misc bug fixes to the scsi tree
at your earliest convenience.

Thanks,
Nilesh

Saurav Kashyap (3):
  qedf: Add stag_work to all the vports
  qedf: Fix refcount issue when LOGO is received during TMF
  qedf: Change context reset messages to ratelimited

 drivers/scsi/qedf/qedf_io.c   | 1 +
 drivers/scsi/qedf/qedf_main.c | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)


base-commit: 315d049ad1951cef02d9337a2469cac51cca6932
-- 
2.23.1

Re: [PATCH 0/3] qedf misc bug fixes

[Martin K. Petersen]

Nilesh,

> Please apply the qedf driver misc bug fixes to the scsi tree at your
> earliest convenience.

Applied to 5.17/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/3] qedf misc bug fixes

[Martin K. Petersen]

On Mon, 17 Jan 2022 05:53:08 -0800, Nilesh Javali wrote:

> Martin,
> 
> Please apply the qedf driver misc bug fixes to the scsi tree
> at your earliest convenience.
> 
> Thanks,
> Nilesh
> 
> [...]

Applied to 5.17/scsi-fixes, thanks!

[1/3] qedf: Add stag_work to all the vports
      https://git.kernel.org/mkp/scsi/c/b70a99fd1328
[2/3] qedf: Fix refcount issue when LOGO is received during TMF
      https://git.kernel.org/mkp/scsi/c/5239ab63f17c
[3/3] qedf: Change context reset messages to ratelimited
      https://git.kernel.org/mkp/scsi/c/64fd4af6274e

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 0/3] qedf misc bug fixes

[Saurav Kashyap]

Hi Martin,

Please apply the qedf driver fixes to the
scsi tree at your earliest convenience.

Thanks,
Saurav

Saurav Kashyap (3):
  qedf: Don't process stag work during unload and recovery.
  qedf: Wait for stag work during unload.
  qedf: Memset qed_slowpath_params to zero before use.

 drivers/scsi/qedf/qedf.h      |  1 +
 drivers/scsi/qedf/qedf_main.c | 43 ++++++++++++++++++++++++++++++++++-
 2 files changed, 43 insertions(+), 1 deletion(-)

-- 
2.23.1

[PATCH 1/3] qedf: Don't process stag work during unload and recovery.

[Saurav Kashyap]

Stag work can cause issues during unload and recovery,
hence don't process it.

Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qedf/qedf_main.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index a58353b7b4e8..e882aec86765 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -3997,6 +3997,22 @@ void qedf_stag_change_work(struct work_struct *work)
 	struct qedf_ctx *qedf =
 	    container_of(work, struct qedf_ctx, stag_work.work);
 
+	if (!qedf) {
+		QEDF_ERR(&qedf->dbg_ctx, "qedf is NULL");
+		return;
+	}
+
+	if (test_bit(QEDF_IN_RECOVERY, &qedf->flags)) {
+		QEDF_ERR(&qedf->dbg_ctx,
+			 "Already is in recovery, hence not calling software context reset.\n");
+		return;
+	}
+
+	if (test_bit(QEDF_UNLOADING, &qedf->flags)) {
+		QEDF_ERR(&qedf->dbg_ctx, "Driver unloading\n");
+		return;
+	}
+
 	printk_ratelimited("[%s]:[%s:%d]:%d: Performing software context reset.",
 			dev_name(&qedf->pdev->dev), __func__, __LINE__,
 			qedf->dbg_ctx.host_no);
-- 
2.23.1

[PATCH 2/3] qedf: Wait for stag work during unload.

[Saurav Kashyap]

If stag work is already schedule and unload is called, it can
lead to issues as unload cleanups the work element, wait for
stag work to get completed before cleanup during unload.

Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qedf/qedf.h      |  1 +
 drivers/scsi/qedf/qedf_main.c | 26 +++++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qedf/qedf.h b/drivers/scsi/qedf/qedf.h
index 5058e01b65a2..98afdfe63600 100644
--- a/drivers/scsi/qedf/qedf.h
+++ b/drivers/scsi/qedf/qedf.h
@@ -363,6 +363,7 @@ struct qedf_ctx {
 #define QEDF_IN_RECOVERY		5
 #define QEDF_DBG_STOP_IO		6
 #define QEDF_PROBING			8
+#define QEDF_STAG_IN_PROGRESS		9
 	unsigned long flags; /* Miscellaneous state flags */
 	int fipvlan_retries;
 	u8 num_queues;
diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index e882aec86765..d441054dadb7 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -318,11 +318,18 @@ static struct fc_seq *qedf_elsct_send(struct fc_lport *lport, u32 did,
 	 */
 	if (resp == fc_lport_flogi_resp) {
 		qedf->flogi_cnt++;
+		qedf->flogi_pending++;
+
+		if (test_bit(QEDF_UNLOADING, &qedf->flags)) {
+			QEDF_ERR(&qedf->dbg_ctx, "Driver unloading\n");
+			qedf->flogi_pending = 0;
+		}
+
 		if (qedf->flogi_pending >= QEDF_FLOGI_RETRY_CNT) {
 			schedule_delayed_work(&qedf->stag_work, 2);
 			return NULL;
 		}
-		qedf->flogi_pending++;
+
 		return fc_elsct_send(lport, did, fp, op, qedf_flogi_resp,
 		    arg, timeout);
 	}
@@ -913,6 +920,7 @@ void qedf_ctx_soft_reset(struct fc_lport *lport)
 	struct qed_link_output if_link;
 
 	if (lport->vport) {
+		clear_bit(QEDF_STAG_IN_PROGRESS, &qedf->flags);
 		printk_ratelimited("Cannot issue host reset on NPIV port.\n");
 		return;
 	}
@@ -938,6 +946,7 @@ void qedf_ctx_soft_reset(struct fc_lport *lport)
 	if (!if_link.link_up) {
 		QEDF_INFO(&qedf->dbg_ctx, QEDF_LOG_DISC,
 			  "Physical link is not up.\n");
+		clear_bit(QEDF_STAG_IN_PROGRESS, &qedf->flags);
 		return;
 	}
 	/* Flush and wait to make sure link down is processed */
@@ -950,6 +959,7 @@ void qedf_ctx_soft_reset(struct fc_lport *lport)
 		  "Queue link up work.\n");
 	queue_delayed_work(qedf->link_update_wq, &qedf->link_update,
 	    0);
+	clear_bit(QEDF_STAG_IN_PROGRESS, &qedf->flags);
 }
 
 /* Reset the host by gracefully logging out and then logging back in */
@@ -3721,6 +3731,7 @@ static void __qedf_remove(struct pci_dev *pdev, int mode)
 {
 	struct qedf_ctx *qedf;
 	int rc;
+	int cnt = 0;
 
 	if (!pdev) {
 		QEDF_ERR(NULL, "pdev is NULL.\n");
@@ -3738,6 +3749,17 @@ static void __qedf_remove(struct pci_dev *pdev, int mode)
 		return;
 	}
 
+stag_in_prog:
+	if (test_bit(QEDF_STAG_IN_PROGRESS, &qedf->flags)) {
+		QEDF_ERR(&qedf->dbg_ctx, "Stag in progress, cnt=%d.\n", cnt);
+		cnt++;
+
+		if (cnt < 5) {
+			msleep(500);
+			goto stag_in_prog;
+		}
+	}
+
 	if (mode != QEDF_MODE_RECOVERY)
 		set_bit(QEDF_UNLOADING, &qedf->flags);
 
@@ -4013,6 +4035,8 @@ void qedf_stag_change_work(struct work_struct *work)
 		return;
 	}
 
+	set_bit(QEDF_STAG_IN_PROGRESS, &qedf->flags);
+
 	printk_ratelimited("[%s]:[%s:%d]:%d: Performing software context reset.",
 			dev_name(&qedf->pdev->dev), __func__, __LINE__,
 			qedf->dbg_ctx.host_no);
-- 
2.23.1

[PATCH 3/3] qedf: Memset qed_slowpath_params to zero before use.

[Saurav Kashyap]

- Memset qed_slowpath_params to zero before use.

Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qedf/qedf_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index d441054dadb7..d4563d28d98f 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -3473,6 +3473,7 @@ static int __qedf_probe(struct pci_dev *pdev, int mode)
 	}
 
 	/* Start the Slowpath-process */
+	memset(&slowpath_params, 0, sizeof(struct qed_slowpath_params));
 	slowpath_params.int_mode = QED_INT_MODE_MSIX;
 	slowpath_params.drv_major = QEDF_DRIVER_MAJOR_VER;
 	slowpath_params.drv_minor = QEDF_DRIVER_MINOR_VER;
-- 
2.23.1

Re: [PATCH 0/3] qedf misc bug fixes

[Martin Wilck]

On Fri, 2024-03-15 at 15:35 +0530, Saurav Kashyap wrote:
> Hi Martin,
> 
> Please apply the qedf driver fixes to the
> scsi tree at your earliest convenience.
> 
> Thanks,
> Saurav
> 
> Saurav Kashyap (3):
>   qedf: Don't process stag work during unload and recovery.
>   qedf: Wait for stag work during unload.
>   qedf: Memset qed_slowpath_params to zero before use.
> 
>  drivers/scsi/qedf/qedf.h      |  1 +
>  drivers/scsi/qedf/qedf_main.c | 43
> ++++++++++++++++++++++++++++++++++-
>  2 files changed, 43 insertions(+), 1 deletion(-)
> 

This series fixed a well-reproducible kernel crash on one of our
hardware partner's systems.

For the series:
Reviewed-by: Martin Wilck <mwilck@suse.com>

Re: [PATCH 2/3] qedf: Wait for stag work during unload.

[kernel test robot]

Hi Saurav,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next linus/master v6.8 next-20240315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Saurav-Kashyap/qedf-Don-t-process-stag-work-during-unload-and-recovery/20240315-180830
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link:    https://lore.kernel.org/r/20240315100600.19568-3-skashyap%40marvell.com
patch subject: [PATCH 2/3] qedf: Wait for stag work during unload.
config: x86_64-allmodconfig (https://download.01.org/0day-ci/archive/20240316/202403160959.uNobO4UE-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/202403160959.uNobO4UE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403160959.uNobO4UE-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/scsi/qedf/qedf_main.c:923:37: warning: variable 'qedf' is uninitialized when used here [-Wuninitialized]
     923 |                 clear_bit(QEDF_STAG_IN_PROGRESS, &qedf->flags);
         |                                                   ^~~~
   drivers/scsi/qedf/qedf_main.c:919:23: note: initialize the variable 'qedf' to silence this warning
     919 |         struct qedf_ctx *qedf;
         |                              ^
         |                               = NULL
   1 warning generated.


vim +/qedf +923 drivers/scsi/qedf/qedf_main.c

   915	
   916	/* Performs soft reset of qedf_ctx by simulating a link down/up */
   917	void qedf_ctx_soft_reset(struct fc_lport *lport)
   918	{
   919		struct qedf_ctx *qedf;
   920		struct qed_link_output if_link;
   921	
   922		if (lport->vport) {
 > 923			clear_bit(QEDF_STAG_IN_PROGRESS, &qedf->flags);
   924			printk_ratelimited("Cannot issue host reset on NPIV port.\n");
   925			return;
   926		}
   927	
   928		qedf = lport_priv(lport);
   929	
   930		qedf->flogi_pending = 0;
   931		/* For host reset, essentially do a soft link up/down */
   932		atomic_set(&qedf->link_state, QEDF_LINK_DOWN);
   933		QEDF_INFO(&qedf->dbg_ctx, QEDF_LOG_DISC,
   934			  "Queuing link down work.\n");
   935		queue_delayed_work(qedf->link_update_wq, &qedf->link_update,
   936		    0);
   937	
   938		if (qedf_wait_for_upload(qedf) == false) {
   939			QEDF_ERR(&qedf->dbg_ctx, "Could not upload all sessions.\n");
   940			WARN_ON(atomic_read(&qedf->num_offloads));
   941		}
   942	
   943		/* Before setting link up query physical link state */
   944		qed_ops->common->get_link(qedf->cdev, &if_link);
   945		/* Bail if the physical link is not up */
   946		if (!if_link.link_up) {
   947			QEDF_INFO(&qedf->dbg_ctx, QEDF_LOG_DISC,
   948				  "Physical link is not up.\n");
   949			clear_bit(QEDF_STAG_IN_PROGRESS, &qedf->flags);
   950			return;
   951		}
   952		/* Flush and wait to make sure link down is processed */
   953		flush_delayed_work(&qedf->link_update);
   954		msleep(500);
   955	
   956		atomic_set(&qedf->link_state, QEDF_LINK_UP);
   957		qedf->vlan_id  = 0;
   958		QEDF_INFO(&qedf->dbg_ctx, QEDF_LOG_DISC,
   959			  "Queue link up work.\n");
   960		queue_delayed_work(qedf->link_update_wq, &qedf->link_update,
   961		    0);
   962		clear_bit(QEDF_STAG_IN_PROGRESS, &qedf->flags);
   963	}
   964	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 0/3] qedf misc bug fixes

[Lee Duncan]

On Fri, Mar 15, 2024 at 3:06 AM Saurav Kashyap <skashyap@marvell.com> wrote:
>
> Hi Martin,
>
> Please apply the qedf driver fixes to the
> scsi tree at your earliest convenience.
>
> Thanks,
> Saurav
>
> Saurav Kashyap (3):
>   qedf: Don't process stag work during unload and recovery.
>   qedf: Wait for stag work during unload.
>   qedf: Memset qed_slowpath_params to zero before use.
>
>  drivers/scsi/qedf/qedf.h      |  1 +
>  drivers/scsi/qedf/qedf_main.c | 43 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 43 insertions(+), 1 deletion(-)
>
> --
> 2.23.1
>
>

If not too late, for the series:

Reviewed-by: Lee Duncan <lduncan@suse.com>

Re: [PATCH 0/3] qedf misc bug fixes

[Martin Wilck]

On Wed, 2024-05-01 at 15:28 -0700, Lee Duncan wrote:
> On Fri, Mar 15, 2024 at 3:06 AM Saurav Kashyap <skashyap@marvell.com>
> wrote:
> > ...
> 
> If not too late, for the series:
> 
> Reviewed-by: Lee Duncan <lduncan@suse.com>
> 

Martin P., any chance to get this set applied?

Thanks, Martin W.

Re: [PATCH 0/3] qedf misc bug fixes

[Martin K. Petersen]

Martin,

> Martin P., any chance to get this set applied?

The series had a test robot warning and I requested a v2 to be posted
with the fix rolled in.

https://lore.kernel.org/all/yq1h6fq8cv3.fsf@ca-mkp.ca.oracle.com/

-- 
Martin K. Petersen	Oracle Linux Engineering