* [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.