developer | 4f3e9a2 | 2022-05-30 17:33:30 +0800 | [diff] [blame] | 1 | From git@z Thu Jan 1 00:00:00 1970 |
| 2 | Subject: [PATCH v2] net: make napi_disable() symmetric with enable |
| 3 | From: Jakub Kicinski <kuba@kernel.org> |
| 4 | Date: Fri, 24 Sep 2021 13:24:53 -0700 |
| 5 | Message-Id: <20210924202453.1051687-1-kuba@kernel.org> |
| 6 | To: davem@davemloft.net |
| 7 | Cc: netdev@vger.kernel.org, eric.dumazet@gmail.com, weiwan@google.com, xuanzhuo@linux.alibaba.com, Jakub Kicinski <kuba@kernel.org> |
| 8 | List-Id: <netdev.vger.kernel.org> |
| 9 | MIME-Version: 1.0 |
| 10 | Content-Type: text/plain; charset="utf-8" |
| 11 | Content-Transfer-Encoding: 7bit |
| 12 | |
| 13 | Commit 3765996e4f0b ("napi: fix race inside napi_enable") fixed |
| 14 | an ordering bug in napi_enable() and made the napi_enable() diverge |
| 15 | from napi_disable(). The state transitions done on disable are |
| 16 | not symmetric to enable. |
| 17 | |
| 18 | There is no known bug in napi_disable() this is just refactoring. |
| 19 | |
| 20 | Eric suggests we can also replace msleep(1) with a more opportunistic |
| 21 | usleep_range(). |
| 22 | |
| 23 | Signed-off-by: Jakub Kicinski <kuba@kernel.org> |
| 24 | --- |
| 25 | net/core/dev.c | 17 ++++++++++++----- |
| 26 | 1 file changed, 12 insertions(+), 5 deletions(-) |
| 27 | |
| 28 | diff --git a/net/core/dev.c b/net/core/dev.c |
| 29 | index f24c3a9..f0a556a 100644 |
| 30 | --- a/net/core/dev.c |
| 31 | +++ b/net/core/dev.c |
| 32 | @@ -6386,18 +6386,25 @@ EXPORT_SYMBOL(netif_napi_add); |
| 33 | |
| 34 | void napi_disable(struct napi_struct *n) |
| 35 | { |
| 36 | + unsigned long val, new; |
| 37 | + |
| 38 | might_sleep(); |
| 39 | set_bit(NAPI_STATE_DISABLE, &n->state); |
| 40 | |
| 41 | - while (test_and_set_bit(NAPI_STATE_SCHED, &n->state)) |
| 42 | - msleep(1); |
| 43 | - while (test_and_set_bit(NAPI_STATE_NPSVC, &n->state)) |
| 44 | - msleep(1); |
| 45 | + do { |
| 46 | + val = READ_ONCE(n->state); |
| 47 | + if (val & (NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC)) { |
| 48 | + usleep_range(20, 200); |
| 49 | + continue; |
| 50 | + } |
| 51 | + |
| 52 | + new = val | NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC; |
| 53 | + new &= ~(NAPIF_STATE_THREADED); |
| 54 | + } while (cmpxchg(&n->state, val, new) != val); |
| 55 | |
| 56 | hrtimer_cancel(&n->timer); |
| 57 | |
| 58 | clear_bit(NAPI_STATE_DISABLE, &n->state); |
| 59 | - clear_bit(NAPI_STATE_THREADED, &n->state); |
| 60 | } |
| 61 | EXPORT_SYMBOL(napi_disable); |
| 62 | |
| 63 | -- |
| 64 | 2.31.1 |