Skip to content

Commit 84b767f

Browse files
emuslnkuba-moo
authored andcommitted
ionic: use dev_consume_skb_any outside of napi
If we're not in a NAPI softirq context, we need to be careful about how we call napi_consume_skb(), specifically we need to call it with budget==0 to signal to it that we're not in a safe context. This was found while running some configuration stress testing of traffic and a change queue config loop running, and this curious note popped out: [ 4371.402645] BUG: using smp_processor_id() in preemptible [00000000] code: ethtool/20545 [ 4371.402897] caller is napi_skb_cache_put+0x16/0x80 [ 4371.403120] CPU: 25 PID: 20545 Comm: ethtool Kdump: loaded Tainted: G OE 6.10.0-rc3-netnext+ svenkatr#8 [ 4371.403302] Hardware name: HPE ProLiant DL360 Gen10/ProLiant DL360 Gen10, BIOS U32 01/23/2021 [ 4371.403460] Call Trace: [ 4371.403613] <TASK> [ 4371.403758] dump_stack_lvl+0x4f/0x70 [ 4371.403904] check_preemption_disabled+0xc1/0xe0 [ 4371.404051] napi_skb_cache_put+0x16/0x80 [ 4371.404199] ionic_tx_clean+0x18a/0x240 [ionic] [ 4371.404354] ionic_tx_cq_service+0xc4/0x200 [ionic] [ 4371.404505] ionic_tx_flush+0x15/0x70 [ionic] [ 4371.404653] ? ionic_lif_qcq_deinit.isra.23+0x5b/0x70 [ionic] [ 4371.404805] ionic_txrx_deinit+0x71/0x190 [ionic] [ 4371.404956] ionic_reconfigure_queues+0x5f5/0xff0 [ionic] [ 4371.405111] ionic_set_ringparam+0x2e8/0x3e0 [ionic] [ 4371.405265] ethnl_set_rings+0x1f1/0x300 [ 4371.405418] ethnl_default_set_doit+0xbb/0x160 [ 4371.405571] genl_family_rcv_msg_doit+0xff/0x130 [...] I found that ionic_tx_clean() calls napi_consume_skb() which calls napi_skb_cache_put(), but before that last call is the note /* Zero budget indicate non-NAPI context called us, like netpoll */ and DEBUG_NET_WARN_ON_ONCE(!in_softirq()); Those are pretty big hints that we're doing it wrong. We can pass a context hint down through the calls to let ionic_tx_clean() know what we're doing so it can call napi_consume_skb() correctly. Fixes: 386e698 ("ionic: Make use napi_consume_skb") Signed-off-by: Shannon Nelson <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent b1c4b4d commit 84b767f

File tree

3 files changed

+21
-13
lines changed

3 files changed

+21
-13
lines changed

drivers/net/ethernet/pensando/ionic/ionic_dev.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,9 @@ typedef void (*ionic_cq_done_cb)(void *done_arg);
375375
unsigned int ionic_cq_service(struct ionic_cq *cq, unsigned int work_to_do,
376376
ionic_cq_cb cb, ionic_cq_done_cb done_cb,
377377
void *done_arg);
378-
unsigned int ionic_tx_cq_service(struct ionic_cq *cq, unsigned int work_to_do);
378+
unsigned int ionic_tx_cq_service(struct ionic_cq *cq,
379+
unsigned int work_to_do,
380+
bool in_napi);
379381

380382
int ionic_q_init(struct ionic_lif *lif, struct ionic_dev *idev,
381383
struct ionic_queue *q, unsigned int index, const char *name,

drivers/net/ethernet/pensando/ionic/ionic_lif.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1189,7 +1189,7 @@ static int ionic_adminq_napi(struct napi_struct *napi, int budget)
11891189
ionic_rx_service, NULL, NULL);
11901190

11911191
if (lif->hwstamp_txq)
1192-
tx_work = ionic_tx_cq_service(&lif->hwstamp_txq->cq, budget);
1192+
tx_work = ionic_tx_cq_service(&lif->hwstamp_txq->cq, budget, !!budget);
11931193

11941194
work_done = max(max(n_work, a_work), max(rx_work, tx_work));
11951195
if (work_done < budget && napi_complete_done(napi, work_done)) {

drivers/net/ethernet/pensando/ionic/ionic_txrx.c

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ static void ionic_tx_desc_unmap_bufs(struct ionic_queue *q,
2323

2424
static void ionic_tx_clean(struct ionic_queue *q,
2525
struct ionic_tx_desc_info *desc_info,
26-
struct ionic_txq_comp *comp);
26+
struct ionic_txq_comp *comp,
27+
bool in_napi);
2728

2829
static inline void ionic_txq_post(struct ionic_queue *q, bool ring_dbell)
2930
{
@@ -944,7 +945,7 @@ int ionic_tx_napi(struct napi_struct *napi, int budget)
944945
u32 work_done = 0;
945946
u32 flags = 0;
946947

947-
work_done = ionic_tx_cq_service(cq, budget);
948+
work_done = ionic_tx_cq_service(cq, budget, !!budget);
948949

949950
if (unlikely(!budget))
950951
return budget;
@@ -1028,7 +1029,7 @@ int ionic_txrx_napi(struct napi_struct *napi, int budget)
10281029
txqcq = lif->txqcqs[qi];
10291030
txcq = &lif->txqcqs[qi]->cq;
10301031

1031-
tx_work_done = ionic_tx_cq_service(txcq, IONIC_TX_BUDGET_DEFAULT);
1032+
tx_work_done = ionic_tx_cq_service(txcq, IONIC_TX_BUDGET_DEFAULT, !!budget);
10321033

10331034
if (unlikely(!budget))
10341035
return budget;
@@ -1161,7 +1162,8 @@ static void ionic_tx_desc_unmap_bufs(struct ionic_queue *q,
11611162

11621163
static void ionic_tx_clean(struct ionic_queue *q,
11631164
struct ionic_tx_desc_info *desc_info,
1164-
struct ionic_txq_comp *comp)
1165+
struct ionic_txq_comp *comp,
1166+
bool in_napi)
11651167
{
11661168
struct ionic_tx_stats *stats = q_to_tx_stats(q);
11671169
struct ionic_qcq *qcq = q_to_qcq(q);
@@ -1213,11 +1215,13 @@ static void ionic_tx_clean(struct ionic_queue *q,
12131215
desc_info->bytes = skb->len;
12141216
stats->clean++;
12151217

1216-
napi_consume_skb(skb, 1);
1218+
napi_consume_skb(skb, likely(in_napi) ? 1 : 0);
12171219
}
12181220

12191221
static bool ionic_tx_service(struct ionic_cq *cq,
1220-
unsigned int *total_pkts, unsigned int *total_bytes)
1222+
unsigned int *total_pkts,
1223+
unsigned int *total_bytes,
1224+
bool in_napi)
12211225
{
12221226
struct ionic_tx_desc_info *desc_info;
12231227
struct ionic_queue *q = cq->bound_q;
@@ -1239,7 +1243,7 @@ static bool ionic_tx_service(struct ionic_cq *cq,
12391243
desc_info->bytes = 0;
12401244
index = q->tail_idx;
12411245
q->tail_idx = (q->tail_idx + 1) & (q->num_descs - 1);
1242-
ionic_tx_clean(q, desc_info, comp);
1246+
ionic_tx_clean(q, desc_info, comp, in_napi);
12431247
if (desc_info->skb) {
12441248
pkts++;
12451249
bytes += desc_info->bytes;
@@ -1253,7 +1257,9 @@ static bool ionic_tx_service(struct ionic_cq *cq,
12531257
return true;
12541258
}
12551259

1256-
unsigned int ionic_tx_cq_service(struct ionic_cq *cq, unsigned int work_to_do)
1260+
unsigned int ionic_tx_cq_service(struct ionic_cq *cq,
1261+
unsigned int work_to_do,
1262+
bool in_napi)
12571263
{
12581264
unsigned int work_done = 0;
12591265
unsigned int bytes = 0;
@@ -1262,7 +1268,7 @@ unsigned int ionic_tx_cq_service(struct ionic_cq *cq, unsigned int work_to_do)
12621268
if (work_to_do == 0)
12631269
return 0;
12641270

1265-
while (ionic_tx_service(cq, &pkts, &bytes)) {
1271+
while (ionic_tx_service(cq, &pkts, &bytes, in_napi)) {
12661272
if (cq->tail_idx == cq->num_descs - 1)
12671273
cq->done_color = !cq->done_color;
12681274
cq->tail_idx = (cq->tail_idx + 1) & (cq->num_descs - 1);
@@ -1288,7 +1294,7 @@ void ionic_tx_flush(struct ionic_cq *cq)
12881294
{
12891295
u32 work_done;
12901296

1291-
work_done = ionic_tx_cq_service(cq, cq->num_descs);
1297+
work_done = ionic_tx_cq_service(cq, cq->num_descs, false);
12921298
if (work_done)
12931299
ionic_intr_credits(cq->idev->intr_ctrl, cq->bound_intr->index,
12941300
work_done, IONIC_INTR_CRED_RESET_COALESCE);
@@ -1305,7 +1311,7 @@ void ionic_tx_empty(struct ionic_queue *q)
13051311
desc_info = &q->tx_info[q->tail_idx];
13061312
desc_info->bytes = 0;
13071313
q->tail_idx = (q->tail_idx + 1) & (q->num_descs - 1);
1308-
ionic_tx_clean(q, desc_info, NULL);
1314+
ionic_tx_clean(q, desc_info, NULL, false);
13091315
if (desc_info->skb) {
13101316
pkts++;
13111317
bytes += desc_info->bytes;

0 commit comments

Comments
 (0)