[dpdk-stable] patch 'net/bnxt: add separate mutex for FW health check' has been queued to stable release 19.11.6
Somnath Kotur
somnath.kotur at broadcom.com
Thu Oct 29 07:30:29 CET 2020
On Wed, Oct 28, 2020 at 4:18 PM <luca.boccassi at gmail.com> wrote:
>
> Hi,
>
> FYI, your patch has been queued to stable release 19.11.6
>
> Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
> It will be pushed if I get no objections before 10/30/20. So please
> shout if anyone has objections.
>
> Also note that after the patch there's a diff of the upstream commit vs the
> patch applied to the branch. This will indicate if there was any rebasing
> needed to apply to the stable branch. If there were code changes for rebasing
> (ie: not only metadata diffs), please double check that the rebase was
> correctly done.
>
> Thanks.
>
> Luca Boccassi
>
> ---
> From beb74f5be42aaa494ea6b278091b028043931f69 Mon Sep 17 00:00:00 2001
> From: Somnath Kotur <somnath.kotur at broadcom.com>
> Date: Thu, 10 Sep 2020 18:56:03 -0700
> Subject: [PATCH] net/bnxt: add separate mutex for FW health check
>
> [ upstream commit 2993075dc253484ba3c87996981f32468aa86cbd ]
>
> def_cp_lock was added to sync race between dev_configure and
> int_handler. It should not be used to synchronize scheduling of FW
> health check between dev_start and async event handler as well,
> use a separate mutex for the same.
>
> Fixes: a73b8e939f10 ("net/bnxt: fix race between start and interrupt handler")
>
> Signed-off-by: Somnath Kotur <somnath.kotur at broadcom.com>
> Reviewed-by: Sriharsha Basavapatna <sriharsha.basavapatna at broadcom.com>
> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil at broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde at broadcom.com>
> ---
> drivers/net/bnxt/bnxt.h | 1 +
> drivers/net/bnxt/bnxt_ethdev.c | 17 +++++++++++++----
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
> index 46cf418647..cdb3e2d3ff 100644
> --- a/drivers/net/bnxt/bnxt.h
> +++ b/drivers/net/bnxt/bnxt.h
> @@ -593,6 +593,7 @@ struct bnxt {
> rte_iova_t hwrm_short_cmd_req_dma_addr;
> rte_spinlock_t hwrm_lock;
> pthread_mutex_t def_cp_lock;
> + pthread_mutex_t health_check_lock;
> uint16_t max_req_len;
> uint16_t max_resp_len;
> uint16_t hwrm_max_ext_req_len;
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index fe240b6ccc..d6afd03e56 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -885,9 +885,8 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
> eth_dev->rx_pkt_burst = bnxt_receive_function(eth_dev);
> eth_dev->tx_pkt_burst = bnxt_transmit_function(eth_dev);
>
> - pthread_mutex_lock(&bp->def_cp_lock);
> bnxt_schedule_fw_health_check(bp);
> - pthread_mutex_unlock(&bp->def_cp_lock);
> +
> return 0;
>
> error:
> @@ -4205,17 +4204,22 @@ void bnxt_schedule_fw_health_check(struct bnxt *bp)
> {
> uint32_t polling_freq;
>
> + pthread_mutex_lock(&bp->health_check_lock);
> +
> if (!bnxt_is_recovery_enabled(bp))
> - return;
> + goto done;
>
> if (bp->flags & BNXT_FLAG_FW_HEALTH_CHECK_SCHEDULED)
> - return;
> + goto done;
>
> polling_freq = bp->recovery_info->driver_polling_freq;
>
> rte_eal_alarm_set(US_PER_MS * polling_freq,
> bnxt_check_fw_health, (void *)bp);
> bp->flags |= BNXT_FLAG_FW_HEALTH_CHECK_SCHEDULED;
> +
> +done:
> + pthread_mutex_unlock(&bp->health_check_lock);
> }
>
> static void bnxt_cancel_fw_health_check(struct bnxt *bp)
> @@ -4747,6 +4751,10 @@ bnxt_init_locks(struct bnxt *bp)
> err = pthread_mutex_init(&bp->def_cp_lock, NULL);
> if (err)
> PMD_DRV_LOG(ERR, "Unable to initialize def_cp_lock\n");
> +
> + err = pthread_mutex_init(&bp->health_check_lock, NULL);
> + if (err)
> + PMD_DRV_LOG(ERR, "Unable to initialize health_check_lock\n");
> return err;
> }
>
> @@ -4888,6 +4896,7 @@ bnxt_uninit_locks(struct bnxt *bp)
> {
> pthread_mutex_destroy(&bp->flow_lock);
> pthread_mutex_destroy(&bp->def_cp_lock);
> + pthread_mutex_destroy(&bp->health_check_lock);
> }
>
> static int
> --
> 2.20.1
>
> ---
> Diff of the applied patch vs upstream commit (please double-check if non-empty:
> ---
> --- - 2020-10-28 10:35:13.193181379 +0000
> +++ 0045-net-bnxt-add-separate-mutex-for-FW-health-check.patch 2020-10-28 10:35:11.508830082 +0000
> @@ -1,15 +1,16 @@
> -From 2993075dc253484ba3c87996981f32468aa86cbd Mon Sep 17 00:00:00 2001
> +From beb74f5be42aaa494ea6b278091b028043931f69 Mon Sep 17 00:00:00 2001
> From: Somnath Kotur <somnath.kotur at broadcom.com>
> Date: Thu, 10 Sep 2020 18:56:03 -0700
> Subject: [PATCH] net/bnxt: add separate mutex for FW health check
>
> +[ upstream commit 2993075dc253484ba3c87996981f32468aa86cbd ]
> +
> def_cp_lock was added to sync race between dev_configure and
> int_handler. It should not be used to synchronize scheduling of FW
> health check between dev_start and async event handler as well,
> use a separate mutex for the same.
>
> Fixes: a73b8e939f10 ("net/bnxt: fix race between start and interrupt handler")
> -Cc: stable at dpdk.org
>
> Signed-off-by: Somnath Kotur <somnath.kotur at broadcom.com>
> Reviewed-by: Sriharsha Basavapatna <sriharsha.basavapatna at broadcom.com>
> @@ -17,14 +18,14 @@
> Reviewed-by: Ajit Khaparde <ajit.khaparde at broadcom.com>
> ---
> drivers/net/bnxt/bnxt.h | 1 +
> - drivers/net/bnxt/bnxt_ethdev.c | 16 ++++++++++++----
> - 2 files changed, 13 insertions(+), 4 deletions(-)
> + drivers/net/bnxt/bnxt_ethdev.c | 17 +++++++++++++----
> + 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
> -index f0b0800151..bb265999d4 100644
> +index 46cf418647..cdb3e2d3ff 100644
> --- a/drivers/net/bnxt/bnxt.h
> +++ b/drivers/net/bnxt/bnxt.h
> -@@ -712,6 +712,7 @@ struct bnxt {
> +@@ -593,6 +593,7 @@ struct bnxt {
> rte_iova_t hwrm_short_cmd_req_dma_addr;
> rte_spinlock_t hwrm_lock;
> pthread_mutex_t def_cp_lock;
> @@ -33,20 +34,21 @@
> uint16_t max_resp_len;
> uint16_t hwrm_max_ext_req_len;
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> -index 7c27e2435a..05e9a6abbf 100644
> +index fe240b6ccc..d6afd03e56 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> -@@ -1252,9 +1252,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
> +@@ -885,9 +885,8 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
> eth_dev->rx_pkt_burst = bnxt_receive_function(eth_dev);
> eth_dev->tx_pkt_burst = bnxt_transmit_function(eth_dev);
>
> - pthread_mutex_lock(&bp->def_cp_lock);
> bnxt_schedule_fw_health_check(bp);
> - pthread_mutex_unlock(&bp->def_cp_lock);
> -
> ++
> return 0;
>
> -@@ -4675,17 +4673,22 @@ void bnxt_schedule_fw_health_check(struct bnxt *bp)
> + error:
> +@@ -4205,17 +4204,22 @@ void bnxt_schedule_fw_health_check(struct bnxt *bp)
> {
> uint32_t polling_freq;
>
> @@ -71,7 +73,7 @@
> }
>
> static void bnxt_cancel_fw_health_check(struct bnxt *bp)
> -@@ -5473,6 +5476,10 @@ bnxt_init_locks(struct bnxt *bp)
> +@@ -4747,6 +4751,10 @@ bnxt_init_locks(struct bnxt *bp)
> err = pthread_mutex_init(&bp->def_cp_lock, NULL);
> if (err)
> PMD_DRV_LOG(ERR, "Unable to initialize def_cp_lock\n");
> @@ -82,14 +84,14 @@
> return err;
> }
>
> -@@ -5884,6 +5891,7 @@ bnxt_uninit_locks(struct bnxt *bp)
> +@@ -4888,6 +4896,7 @@ bnxt_uninit_locks(struct bnxt *bp)
> {
> pthread_mutex_destroy(&bp->flow_lock);
> pthread_mutex_destroy(&bp->def_cp_lock);
> + pthread_mutex_destroy(&bp->health_check_lock);
> - if (bp->rep_info) {
> - pthread_mutex_destroy(&bp->rep_info->vfr_lock);
> - pthread_mutex_destroy(&bp->rep_info->vfr_start_lock);
> + }
> +
> + static int
> --
> 2.20.1
>
Luca,
I see that the rebase is not correct, you'd want me to
re-spin the corrected version and send it to the 'stable' mailing
list, right?
Thanks
Som
More information about the stable
mailing list