* [RFC PATCH net-next 0/1] cxgb4: fix missing unlock on ETHOFLD desc collect fail path
@ 2022-09-22 5:32 Rafael Mendonca
2022-09-22 5:32 ` [RFC PATCH net-next 1/1] " Rafael Mendonca
0 siblings, 1 reply; 3+ messages in thread
From: Rafael Mendonca @ 2022-09-22 5:32 UTC (permalink / raw)
To: Raju Rangoju, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rahul Lakkireddy
Cc: Rafael Mendonca, netdev, linux-kernel
I'm sending this as a RFC because I'm not familiar with the chelsio
cxgb4 code, sorry if this is nonsense.
I noticed that the 'out' label is passed to the QDESC_GET for the
ETHOFLD TXQ, RXQ, and FLQ, which skips the 'out_unlock' label on error,
and thus doesn't unlock the 'uld_mutex' before returning.
I was thinking the solution would be to simply change the label to
'out_unlock'. However, since commit 5148e5950c67 ("cxgb4: add EOTID
tracking and software context dump"), I was wondering if the access to
these ETHOFLD hardware queues should be protected by the 'mqprio_mutex'
instead of the 'uld_mutex'.
Rafael Mendonca (1):
cxgb4: fix missing unlock on ETHOFLD desc collect fail path
.../net/ethernet/chelsio/cxgb4/cudbg_lib.c | 28 +++++++++++++------
1 file changed, 19 insertions(+), 9 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [RFC PATCH net-next 1/1] cxgb4: fix missing unlock on ETHOFLD desc collect fail path
2022-09-22 5:32 [RFC PATCH net-next 0/1] cxgb4: fix missing unlock on ETHOFLD desc collect fail path Rafael Mendonca
@ 2022-09-22 5:32 ` Rafael Mendonca
2022-09-22 9:21 ` Rahul Lakkireddy
0 siblings, 1 reply; 3+ messages in thread
From: Rafael Mendonca @ 2022-09-22 5:32 UTC (permalink / raw)
To: Raju Rangoju, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rahul Lakkireddy
Cc: Rafael Mendonca, netdev, linux-kernel
The label passed to the QDESC_GET for the ETHOFLD TXQ, RXQ, and FLQ, is the
'out' one, which skips the 'out_unlock' label, and thus doesn't unlock the
'uld_mutex' before returning. Additionally, since commit 5148e5950c67
("cxgb4: add EOTID tracking and software context dump"), the access to
these ETHOFLD hardware queues should be protected by the 'mqprio_mutex'
instead.
Fixes: 2d0cb84dd973 ("cxgb4: add ETHOFLD hardware queue support")
Fixes: 5148e5950c67 ("cxgb4: add EOTID tracking and software context dump")
Signed-off-by: Rafael Mendonca <rafaelmendsr@gmail.com>
---
.../net/ethernet/chelsio/cxgb4/cudbg_lib.c | 28 +++++++++++++------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
index a7f291c89702..557c591a6ce3 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
@@ -14,6 +14,7 @@
#include "cudbg_entity.h"
#include "cudbg_lib.h"
#include "cudbg_zlib.h"
+#include "cxgb4_tc_mqprio.h"
static const u32 t6_tp_pio_array[][IREG_NUM_ELEM] = {
{0x7e40, 0x7e44, 0x020, 28}, /* t6_tp_pio_regs_20_to_3b */
@@ -3458,7 +3459,7 @@ int cudbg_collect_qdesc(struct cudbg_init *pdbg_init,
for (i = 0; i < utxq->ntxq; i++)
QDESC_GET_TXQ(&utxq->uldtxq[i].q,
cudbg_uld_txq_to_qtype(j),
- out_unlock);
+ out_unlock_uld);
}
}
@@ -3475,7 +3476,7 @@ int cudbg_collect_qdesc(struct cudbg_init *pdbg_init,
for (i = 0; i < urxq->nrxq; i++)
QDESC_GET_RXQ(&urxq->uldrxq[i].rspq,
cudbg_uld_rxq_to_qtype(j),
- out_unlock);
+ out_unlock_uld);
}
/* ULD FLQ */
@@ -3487,7 +3488,7 @@ int cudbg_collect_qdesc(struct cudbg_init *pdbg_init,
for (i = 0; i < urxq->nrxq; i++)
QDESC_GET_FLQ(&urxq->uldrxq[i].fl,
cudbg_uld_flq_to_qtype(j),
- out_unlock);
+ out_unlock_uld);
}
/* ULD CIQ */
@@ -3500,29 +3501,34 @@ int cudbg_collect_qdesc(struct cudbg_init *pdbg_init,
for (i = 0; i < urxq->nciq; i++)
QDESC_GET_RXQ(&urxq->uldrxq[base + i].rspq,
cudbg_uld_ciq_to_qtype(j),
- out_unlock);
+ out_unlock_uld);
}
}
+ mutex_unlock(&uld_mutex);
+
+ if (!padap->tc_mqprio)
+ goto out;
+ mutex_lock(&padap->tc_mqprio->mqprio_mutex);
/* ETHOFLD TXQ */
if (s->eohw_txq)
for (i = 0; i < s->eoqsets; i++)
QDESC_GET_TXQ(&s->eohw_txq[i].q,
- CUDBG_QTYPE_ETHOFLD_TXQ, out);
+ CUDBG_QTYPE_ETHOFLD_TXQ, out_unlock_mqprio);
/* ETHOFLD RXQ and FLQ */
if (s->eohw_rxq) {
for (i = 0; i < s->eoqsets; i++)
QDESC_GET_RXQ(&s->eohw_rxq[i].rspq,
- CUDBG_QTYPE_ETHOFLD_RXQ, out);
+ CUDBG_QTYPE_ETHOFLD_RXQ, out_unlock_mqprio);
for (i = 0; i < s->eoqsets; i++)
QDESC_GET_FLQ(&s->eohw_rxq[i].fl,
- CUDBG_QTYPE_ETHOFLD_FLQ, out);
+ CUDBG_QTYPE_ETHOFLD_FLQ, out_unlock_mqprio);
}
-out_unlock:
- mutex_unlock(&uld_mutex);
+out_unlock_mqprio:
+ mutex_unlock(&padap->tc_mqprio->mqprio_mutex);
out:
qdesc_info->qdesc_entry_size = sizeof(*qdesc_entry);
@@ -3559,6 +3565,10 @@ int cudbg_collect_qdesc(struct cudbg_init *pdbg_init,
#undef QDESC_GET
return rc;
+
+out_unlock_uld:
+ mutex_unlock(&uld_mutex);
+ goto out;
}
int cudbg_collect_flash(struct cudbg_init *pdbg_init,
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC PATCH net-next 1/1] cxgb4: fix missing unlock on ETHOFLD desc collect fail path
2022-09-22 5:32 ` [RFC PATCH net-next 1/1] " Rafael Mendonca
@ 2022-09-22 9:21 ` Rahul Lakkireddy
0 siblings, 0 replies; 3+ messages in thread
From: Rahul Lakkireddy @ 2022-09-22 9:21 UTC (permalink / raw)
To: Rafael Mendonca
Cc: Raju Rangoju, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, linux-kernel
On Thursday, September 09/22/22, 2022 at 02:32:36 -0300, Rafael Mendonca wrote:
> The label passed to the QDESC_GET for the ETHOFLD TXQ, RXQ, and FLQ, is the
> 'out' one, which skips the 'out_unlock' label, and thus doesn't unlock the
> 'uld_mutex' before returning. Additionally, since commit 5148e5950c67
> ("cxgb4: add EOTID tracking and software context dump"), the access to
> these ETHOFLD hardware queues should be protected by the 'mqprio_mutex'
> instead.
>
> Fixes: 2d0cb84dd973 ("cxgb4: add ETHOFLD hardware queue support")
> Fixes: 5148e5950c67 ("cxgb4: add EOTID tracking and software context dump")
> Signed-off-by: Rafael Mendonca <rafaelmendsr@gmail.com>
Yes, the ETHOFLD queues do need to be accessed after unlocking the
uld_mutex and then taking the mqprio_mutex. Thanks for the fix!
Reviewed-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> ---
> .../net/ethernet/chelsio/cxgb4/cudbg_lib.c | 28 +++++++++++++------
> 1 file changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
> index a7f291c89702..557c591a6ce3 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
> @@ -14,6 +14,7 @@
> #include "cudbg_entity.h"
> #include "cudbg_lib.h"
> #include "cudbg_zlib.h"
> +#include "cxgb4_tc_mqprio.h"
>
> static const u32 t6_tp_pio_array[][IREG_NUM_ELEM] = {
> {0x7e40, 0x7e44, 0x020, 28}, /* t6_tp_pio_regs_20_to_3b */
> @@ -3458,7 +3459,7 @@ int cudbg_collect_qdesc(struct cudbg_init *pdbg_init,
> for (i = 0; i < utxq->ntxq; i++)
> QDESC_GET_TXQ(&utxq->uldtxq[i].q,
> cudbg_uld_txq_to_qtype(j),
> - out_unlock);
> + out_unlock_uld);
> }
> }
>
> @@ -3475,7 +3476,7 @@ int cudbg_collect_qdesc(struct cudbg_init *pdbg_init,
> for (i = 0; i < urxq->nrxq; i++)
> QDESC_GET_RXQ(&urxq->uldrxq[i].rspq,
> cudbg_uld_rxq_to_qtype(j),
> - out_unlock);
> + out_unlock_uld);
> }
>
> /* ULD FLQ */
> @@ -3487,7 +3488,7 @@ int cudbg_collect_qdesc(struct cudbg_init *pdbg_init,
> for (i = 0; i < urxq->nrxq; i++)
> QDESC_GET_FLQ(&urxq->uldrxq[i].fl,
> cudbg_uld_flq_to_qtype(j),
> - out_unlock);
> + out_unlock_uld);
> }
>
> /* ULD CIQ */
> @@ -3500,29 +3501,34 @@ int cudbg_collect_qdesc(struct cudbg_init *pdbg_init,
> for (i = 0; i < urxq->nciq; i++)
> QDESC_GET_RXQ(&urxq->uldrxq[base + i].rspq,
> cudbg_uld_ciq_to_qtype(j),
> - out_unlock);
> + out_unlock_uld);
> }
> }
> + mutex_unlock(&uld_mutex);
> +
> + if (!padap->tc_mqprio)
> + goto out;
>
> + mutex_lock(&padap->tc_mqprio->mqprio_mutex);
> /* ETHOFLD TXQ */
> if (s->eohw_txq)
> for (i = 0; i < s->eoqsets; i++)
> QDESC_GET_TXQ(&s->eohw_txq[i].q,
> - CUDBG_QTYPE_ETHOFLD_TXQ, out);
> + CUDBG_QTYPE_ETHOFLD_TXQ, out_unlock_mqprio);
>
> /* ETHOFLD RXQ and FLQ */
> if (s->eohw_rxq) {
> for (i = 0; i < s->eoqsets; i++)
> QDESC_GET_RXQ(&s->eohw_rxq[i].rspq,
> - CUDBG_QTYPE_ETHOFLD_RXQ, out);
> + CUDBG_QTYPE_ETHOFLD_RXQ, out_unlock_mqprio);
>
> for (i = 0; i < s->eoqsets; i++)
> QDESC_GET_FLQ(&s->eohw_rxq[i].fl,
> - CUDBG_QTYPE_ETHOFLD_FLQ, out);
> + CUDBG_QTYPE_ETHOFLD_FLQ, out_unlock_mqprio);
> }
>
> -out_unlock:
> - mutex_unlock(&uld_mutex);
> +out_unlock_mqprio:
> + mutex_unlock(&padap->tc_mqprio->mqprio_mutex);
>
> out:
> qdesc_info->qdesc_entry_size = sizeof(*qdesc_entry);
> @@ -3559,6 +3565,10 @@ int cudbg_collect_qdesc(struct cudbg_init *pdbg_init,
> #undef QDESC_GET
>
> return rc;
> +
> +out_unlock_uld:
> + mutex_unlock(&uld_mutex);
> + goto out;
> }
>
> int cudbg_collect_flash(struct cudbg_init *pdbg_init,
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-22 10:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22 5:32 [RFC PATCH net-next 0/1] cxgb4: fix missing unlock on ETHOFLD desc collect fail path Rafael Mendonca
2022-09-22 5:32 ` [RFC PATCH net-next 1/1] " Rafael Mendonca
2022-09-22 9:21 ` Rahul Lakkireddy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.