| Message ID | 20200324141348.7897-2-ybason@marvell.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | qed: Fix and enhance slowpath workqueue mechanism. | expand |
From: Yuval Basson <ybason@marvell.com> Date: Tue, 24 Mar 2020 16:13:46 +0200 > The atomic opertaion might prevent a potential race condition. > > Signed-off-by: Yuval Basson <ybason@marvell.com> > Signed-off-by: Denis Bolotin <dbolotin@marvell.com> There is no basis in fact behind this change. Either explain clearly and precisely what race is fixed by this change or remove this change. Thank you.
> -----Original Message----- > From: David Miller <davem@davemloft.net> > Sent: Wednesday, March 25, 2020 1:36 AM > To: Yuval Basson <ybason@marvell.com> > Cc: netdev@vger.kernel.org; Denis Bolotin <dbolotin@marvell.com> > Subject: [EXT] Re: [PATCH net-next 1/3] qed: Replace wq_active Boolean > with an atomic QED_SLOWPATH_ACTIVE flag > > External Email > > ---------------------------------------------------------------------- > From: Yuval Basson <ybason@marvell.com> > Date: Tue, 24 Mar 2020 16:13:46 +0200 > > > The atomic opertaion might prevent a potential race condition. > > > > Signed-off-by: Yuval Basson <ybason@marvell.com> > > Signed-off-by: Denis Bolotin <dbolotin@marvell.com> > > There is no basis in fact behind this change. > > thanks for the comment, indeed it seems the only patch required to solve the race is the third one >> Sending a v2 with just the third patch. > Either explain clearly and precisely what race is fixed by this change or > remove this change. > > Thank you.
diff --git a/drivers/net/ethernet/qlogic/qed/qed.h b/drivers/net/ethernet/qlogic/qed/qed.h index fa41bf0..ca866c2 100644 --- a/drivers/net/ethernet/qlogic/qed/qed.h +++ b/drivers/net/ethernet/qlogic/qed/qed.h @@ -565,6 +565,7 @@ struct qed_simd_fp_handler { enum qed_slowpath_wq_flag { QED_SLOWPATH_MFW_TLV_REQ, QED_SLOWPATH_PERIODIC_DB_REC, + QED_SLOWPATH_ACTIVE, }; struct qed_hwfn { @@ -700,7 +701,6 @@ struct qed_hwfn { unsigned long iov_task_flags; #endif struct z_stream_s *stream; - bool slowpath_wq_active; struct workqueue_struct *slowpath_wq; struct delayed_work slowpath_task; unsigned long slowpath_task_flags; diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c index 2c189c6..016d658 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_main.c +++ b/drivers/net/ethernet/qlogic/qed/qed_main.c @@ -1095,7 +1095,7 @@ static int qed_slowpath_delayed_work(struct qed_hwfn *hwfn, enum qed_slowpath_wq_flag wq_flag, unsigned long delay) { - if (!hwfn->slowpath_wq_active) + if (!test_bit(QED_SLOWPATH_ACTIVE, &hwfn->slowpath_task_flags)) return -EINVAL; /* Memory barrier for setting atomic bit */ @@ -1133,7 +1133,8 @@ static void qed_slowpath_wq_stop(struct qed_dev *cdev) continue; /* Stop queuing new delayed works */ - cdev->hwfns[i].slowpath_wq_active = false; + clear_bit(QED_SLOWPATH_ACTIVE, + &cdev->hwfns[i].slowpath_task_flags); /* Wait until the last periodic doorbell recovery is executed */ while (test_bit(QED_SLOWPATH_PERIODIC_DB_REC, @@ -1153,7 +1154,7 @@ static void qed_slowpath_task(struct work_struct *work) struct qed_ptt *ptt = qed_ptt_acquire(hwfn); if (!ptt) { - if (hwfn->slowpath_wq_active) + if (test_bit(QED_SLOWPATH_ACTIVE, &hwfn->slowpath_task_flags)) queue_delayed_work(hwfn->slowpath_wq, &hwfn->slowpath_task, 0); @@ -1199,7 +1200,7 @@ static int qed_slowpath_wq_start(struct qed_dev *cdev) } INIT_DELAYED_WORK(&hwfn->slowpath_task, qed_slowpath_task); - hwfn->slowpath_wq_active = true; + set_bit(QED_SLOWPATH_ACTIVE, &hwfn->slowpath_task_flags); } return 0;