| Message ID | 20230607010826.960226-1-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 649c3fed36730a53447d8f479c14e431363563b6 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,1/2] eth: bnxt: fix the wake condition | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Single patches do not need cover letters |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/fixes_present | success | Fixes tag present in non-next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 9 this patch: 9 |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/build_clang | success | Errors and warnings before: 8 this patch: 8 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 9 this patch: 9 |
| netdev/checkpatch | warning | WARNING: line length of 88 exceeds 80 columns |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Tue, Jun 6, 2023 at 6:08 PM Jakub Kicinski <kuba@kernel.org> wrote: > > The down condition should be the negation of the wake condition, > IOW when I moved it from: > > if (cond && wake()) > to > if (__netif_txq_completed_wake(cond)) > > Cond should have been negated. Flip it now. > > This bug leads to occasional crashes with netconsole. > It may also lead to queue never waking up in case BQL is not enabled. > > Reported-by: David Wei <davidhwei@meta.com> > Fixes: 08a096780d92 ("bnxt: use new queue try_stop/try_wake macros") > Signed-off-by: Jakub Kicinski <kuba@kernel.org> Thanks. Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Hello: This series was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 6 Jun 2023 18:08:25 -0700 you wrote: > The down condition should be the negation of the wake condition, > IOW when I moved it from: > > if (cond && wake()) > to > if (__netif_txq_completed_wake(cond)) > > [...] Here is the summary with links: - [net,1/2] eth: bnxt: fix the wake condition https://git.kernel.org/netdev/net/c/649c3fed3673 - [net,2/2] eth: ixgbe: fix the wake condition https://git.kernel.org/netdev/net/c/f0d751973f73 You are awesome, thank you!
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index dcd9367f05af..1f04cd4cfab9 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -692,7 +692,7 @@ static void bnxt_tx_int(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts) __netif_txq_completed_wake(txq, nr_pkts, tx_bytes, bnxt_tx_avail(bp, txr), bp->tx_wake_thresh, - READ_ONCE(txr->dev_state) != BNXT_DEV_STATE_CLOSING); + READ_ONCE(txr->dev_state) == BNXT_DEV_STATE_CLOSING); } static struct page *__bnxt_alloc_rx_page(struct bnxt *bp, dma_addr_t *mapping,
The down condition should be the negation of the wake condition, IOW when I moved it from: if (cond && wake()) to if (__netif_txq_completed_wake(cond)) Cond should have been negated. Flip it now. This bug leads to occasional crashes with netconsole. It may also lead to queue never waking up in case BQL is not enabled. Reported-by: David Wei <davidhwei@meta.com> Fixes: 08a096780d92 ("bnxt: use new queue try_stop/try_wake macros") Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- CC: michael.chan@broadcom.com --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)