From 545cd5e5ec5477c325e4098b6fd21213dceda408 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Fri, 24 Mar 2017 10:07:53 -0700 Subject: net: Busy polling should ignore sender CPUs This patch is a cleanup/fix for NAPI IDs following the changes that made it so that sender_cpu and napi_id were doing a better job of sharing the same location in the sk_buff. One issue I found is that we weren't validating the napi_id as being valid before we started trying to setup the busy polling. This change corrects that by using the MIN_NAPI_ID value that is now used in both allocating the NAPI IDs, as well as validating them. Signed-off-by: Alexander Duyck Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- net/core/dev.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'net/core/dev.c') diff --git a/net/core/dev.c b/net/core/dev.c index 7869ae3837ca..ab337bf5bbf4 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5066,15 +5066,20 @@ bool sk_busy_loop(struct sock *sk, int nonblock) int (*napi_poll)(struct napi_struct *napi, int budget); void *have_poll_lock = NULL; struct napi_struct *napi; + unsigned int napi_id; int rc; restart: + napi_id = READ_ONCE(sk->sk_napi_id); + if (napi_id < MIN_NAPI_ID) + return 0; + rc = false; napi_poll = NULL; rcu_read_lock(); - napi = napi_by_id(sk->sk_napi_id); + napi = napi_by_id(napi_id); if (!napi) goto out; @@ -5143,10 +5148,10 @@ static void napi_hash_add(struct napi_struct *napi) spin_lock(&napi_hash_lock); - /* 0..NR_CPUS+1 range is reserved for sender_cpu use */ + /* 0..NR_CPUS range is reserved for sender_cpu use */ do { - if (unlikely(++napi_gen_id < NR_CPUS + 1)) - napi_gen_id = NR_CPUS + 1; + if (unlikely(++napi_gen_id < MIN_NAPI_ID)) + napi_gen_id = MIN_NAPI_ID; } while (napi_by_id(napi_gen_id)); napi->napi_id = napi_gen_id; -- cgit v1.2.3 From 2b5cd0dfa384242f78a396b90087368c9440cc9a Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Fri, 24 Mar 2017 10:08:12 -0700 Subject: net: Change return type of sk_busy_loop from bool to void checking the return value of sk_busy_loop. As there are only a few consumers of that data, and the data being checked for can be replaced with a check for !skb_queue_empty() we might as well just pull the code out of sk_busy_loop and place it in the spots that actually need it. Signed-off-by: Alexander Duyck Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- include/net/busy_poll.h | 5 ++--- net/core/datagram.c | 8 ++++++-- net/core/dev.c | 25 +++++++++++-------------- net/sctp/socket.c | 9 ++++++--- 4 files changed, 25 insertions(+), 22 deletions(-) (limited to 'net/core/dev.c') diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h index b82d6ba70a14..c55760f4820f 100644 --- a/include/net/busy_poll.h +++ b/include/net/busy_poll.h @@ -74,7 +74,7 @@ static inline bool busy_loop_timeout(unsigned long end_time) return time_after(now, end_time); } -bool sk_busy_loop(struct sock *sk, int nonblock); +void sk_busy_loop(struct sock *sk, int nonblock); #else /* CONFIG_NET_RX_BUSY_POLL */ static inline unsigned long net_busy_loop_on(void) @@ -97,9 +97,8 @@ static inline bool busy_loop_timeout(unsigned long end_time) return true; } -static inline bool sk_busy_loop(struct sock *sk, int nonblock) +static inline void sk_busy_loop(struct sock *sk, int nonblock) { - return false; } #endif /* CONFIG_NET_RX_BUSY_POLL */ diff --git a/net/core/datagram.c b/net/core/datagram.c index ea633342ab0d..4608aa245410 100644 --- a/net/core/datagram.c +++ b/net/core/datagram.c @@ -256,8 +256,12 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags, } spin_unlock_irqrestore(&queue->lock, cpu_flags); - } while (sk_can_busy_loop(sk) && - sk_busy_loop(sk, flags & MSG_DONTWAIT)); + + if (!sk_can_busy_loop(sk)) + break; + + sk_busy_loop(sk, flags & MSG_DONTWAIT); + } while (!skb_queue_empty(&sk->sk_receive_queue)); error = -EAGAIN; diff --git a/net/core/dev.c b/net/core/dev.c index ab337bf5bbf4..af70eb6ba682 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5060,21 +5060,19 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock) do_softirq(); } -bool sk_busy_loop(struct sock *sk, int nonblock) +void sk_busy_loop(struct sock *sk, int nonblock) { unsigned long end_time = !nonblock ? sk_busy_loop_end_time(sk) : 0; int (*napi_poll)(struct napi_struct *napi, int budget); void *have_poll_lock = NULL; struct napi_struct *napi; unsigned int napi_id; - int rc; restart: napi_id = READ_ONCE(sk->sk_napi_id); if (napi_id < MIN_NAPI_ID) - return 0; + return; - rc = false; napi_poll = NULL; rcu_read_lock(); @@ -5085,7 +5083,8 @@ restart: preempt_disable(); for (;;) { - rc = 0; + int work = 0; + local_bh_disable(); if (!napi_poll) { unsigned long val = READ_ONCE(napi->state); @@ -5103,12 +5102,12 @@ restart: have_poll_lock = netpoll_poll_lock(napi); napi_poll = napi->poll; } - rc = napi_poll(napi, BUSY_POLL_BUDGET); - trace_napi_poll(napi, rc, BUSY_POLL_BUDGET); + work = napi_poll(napi, BUSY_POLL_BUDGET); + trace_napi_poll(napi, work, BUSY_POLL_BUDGET); count: - if (rc > 0) + if (work > 0) __NET_ADD_STATS(sock_net(sk), - LINUX_MIB_BUSYPOLLRXPACKETS, rc); + LINUX_MIB_BUSYPOLLRXPACKETS, work); local_bh_enable(); if (nonblock || !skb_queue_empty(&sk->sk_receive_queue) || @@ -5121,9 +5120,9 @@ count: preempt_enable(); rcu_read_unlock(); cond_resched(); - rc = !skb_queue_empty(&sk->sk_receive_queue); - if (rc || busy_loop_timeout(end_time)) - return rc; + if (!skb_queue_empty(&sk->sk_receive_queue) || + busy_loop_timeout(end_time)) + return; goto restart; } cpu_relax(); @@ -5131,10 +5130,8 @@ count: if (napi_poll) busy_poll_stop(napi, have_poll_lock); preempt_enable(); - rc = !skb_queue_empty(&sk->sk_receive_queue); out: rcu_read_unlock(); - return rc; } EXPORT_SYMBOL(sk_busy_loop); diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 72cc3ecf6516..ccc08fc39722 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -7518,9 +7518,12 @@ struct sk_buff *sctp_skb_recv_datagram(struct sock *sk, int flags, if (sk->sk_shutdown & RCV_SHUTDOWN) break; - if (sk_can_busy_loop(sk) && - sk_busy_loop(sk, noblock)) - continue; + if (sk_can_busy_loop(sk)) { + sk_busy_loop(sk, noblock); + + if (!skb_queue_empty(&sk->sk_receive_queue)) + continue; + } /* User doesn't want to wait. */ error = -EAGAIN; -- cgit v1.2.3 From 37056719bba500d0d2b8216fdf641e5507ec9a0e Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Fri, 24 Mar 2017 10:08:18 -0700 Subject: net: Track start of busy loop instead of when it should end This patch flips the logic we were using to determine if the busy polling has timed out. The main motivation for this is that we will need to support two different possible timeout values in the future and by recording the start time rather than when we would want to end we can focus on making the end_time specific to the task be it epoll or socket based polling. Signed-off-by: Alexander Duyck Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- fs/select.c | 16 ++++++------ include/net/busy_poll.h | 68 +++++++++++++++++++++++++++---------------------- net/core/dev.c | 6 ++--- 3 files changed, 49 insertions(+), 41 deletions(-) (limited to 'net/core/dev.c') diff --git a/fs/select.c b/fs/select.c index e2112270d75a..9287d3a96e35 100644 --- a/fs/select.c +++ b/fs/select.c @@ -409,7 +409,7 @@ int do_select(int n, fd_set_bits *fds, struct timespec64 *end_time) int retval, i, timed_out = 0; u64 slack = 0; unsigned int busy_flag = net_busy_loop_on() ? POLL_BUSY_LOOP : 0; - unsigned long busy_end = 0; + unsigned long busy_start = 0; rcu_read_lock(); retval = max_select_fd(n, fds); @@ -512,11 +512,11 @@ int do_select(int n, fd_set_bits *fds, struct timespec64 *end_time) /* only if found POLL_BUSY_LOOP sockets && not out of time */ if (can_busy_loop && !need_resched()) { - if (!busy_end) { - busy_end = busy_loop_end_time(); + if (!busy_start) { + busy_start = busy_loop_current_time(); continue; } - if (!busy_loop_timeout(busy_end)) + if (!busy_loop_timeout(busy_start)) continue; } busy_flag = 0; @@ -800,7 +800,7 @@ static int do_poll(struct poll_list *list, struct poll_wqueues *wait, int timed_out = 0, count = 0; u64 slack = 0; unsigned int busy_flag = net_busy_loop_on() ? POLL_BUSY_LOOP : 0; - unsigned long busy_end = 0; + unsigned long busy_start = 0; /* Optimise the no-wait case */ if (end_time && !end_time->tv_sec && !end_time->tv_nsec) { @@ -853,11 +853,11 @@ static int do_poll(struct poll_list *list, struct poll_wqueues *wait, /* only if found POLL_BUSY_LOOP sockets && not out of time */ if (can_busy_loop && !need_resched()) { - if (!busy_end) { - busy_end = busy_loop_end_time(); + if (!busy_start) { + busy_start = busy_loop_current_time(); continue; } - if (!busy_loop_timeout(busy_end)) + if (!busy_loop_timeout(busy_start)) continue; } busy_flag = 0; diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h index c55760f4820f..72c82f2ea536 100644 --- a/include/net/busy_poll.h +++ b/include/net/busy_poll.h @@ -46,62 +46,70 @@ static inline bool net_busy_loop_on(void) return sysctl_net_busy_poll; } -static inline u64 busy_loop_us_clock(void) +static inline bool sk_can_busy_loop(const struct sock *sk) { - return local_clock() >> 10; + return sk->sk_ll_usec && !signal_pending(current); } -static inline unsigned long sk_busy_loop_end_time(struct sock *sk) -{ - return busy_loop_us_clock() + ACCESS_ONCE(sk->sk_ll_usec); -} +void sk_busy_loop(struct sock *sk, int nonblock); -/* in poll/select we use the global sysctl_net_ll_poll value */ -static inline unsigned long busy_loop_end_time(void) +#else /* CONFIG_NET_RX_BUSY_POLL */ +static inline unsigned long net_busy_loop_on(void) { - return busy_loop_us_clock() + ACCESS_ONCE(sysctl_net_busy_poll); + return 0; } -static inline bool sk_can_busy_loop(const struct sock *sk) +static inline bool sk_can_busy_loop(struct sock *sk) { - return sk->sk_ll_usec && !signal_pending(current); + return false; } -static inline bool busy_loop_timeout(unsigned long end_time) +static inline void sk_busy_loop(struct sock *sk, int nonblock) { - unsigned long now = busy_loop_us_clock(); - - return time_after(now, end_time); } -void sk_busy_loop(struct sock *sk, int nonblock); +#endif /* CONFIG_NET_RX_BUSY_POLL */ -#else /* CONFIG_NET_RX_BUSY_POLL */ -static inline unsigned long net_busy_loop_on(void) +static inline unsigned long busy_loop_current_time(void) { +#ifdef CONFIG_NET_RX_BUSY_POLL + return (unsigned long)(local_clock() >> 10); +#else return 0; +#endif } -static inline unsigned long busy_loop_end_time(void) +/* in poll/select we use the global sysctl_net_ll_poll value */ +static inline bool busy_loop_timeout(unsigned long start_time) { - return 0; -} +#ifdef CONFIG_NET_RX_BUSY_POLL + unsigned long bp_usec = READ_ONCE(sysctl_net_busy_poll); -static inline bool sk_can_busy_loop(struct sock *sk) -{ - return false; -} + if (bp_usec) { + unsigned long end_time = start_time + bp_usec; + unsigned long now = busy_loop_current_time(); -static inline bool busy_loop_timeout(unsigned long end_time) -{ + return time_after(now, end_time); + } +#endif return true; } -static inline void sk_busy_loop(struct sock *sk, int nonblock) +static inline bool sk_busy_loop_timeout(struct sock *sk, + unsigned long start_time) { -} +#ifdef CONFIG_NET_RX_BUSY_POLL + unsigned long bp_usec = READ_ONCE(sk->sk_ll_usec); -#endif /* CONFIG_NET_RX_BUSY_POLL */ + if (bp_usec) { + unsigned long end_time = start_time + bp_usec; + unsigned long now = busy_loop_current_time(); + + return time_after(now, end_time); + } +#endif + return true; +} /* used in the NIC receive handler to mark the skb */ static inline void skb_mark_napi_id(struct sk_buff *skb, diff --git a/net/core/dev.c b/net/core/dev.c index af70eb6ba682..2d1b5613b7fd 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5062,7 +5062,7 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock) void sk_busy_loop(struct sock *sk, int nonblock) { - unsigned long end_time = !nonblock ? sk_busy_loop_end_time(sk) : 0; + unsigned long start_time = nonblock ? 0 : busy_loop_current_time(); int (*napi_poll)(struct napi_struct *napi, int budget); void *have_poll_lock = NULL; struct napi_struct *napi; @@ -5111,7 +5111,7 @@ count: local_bh_enable(); if (nonblock || !skb_queue_empty(&sk->sk_receive_queue) || - busy_loop_timeout(end_time)) + sk_busy_loop_timeout(sk, start_time)) break; if (unlikely(need_resched())) { @@ -5121,7 +5121,7 @@ count: rcu_read_unlock(); cond_resched(); if (!skb_queue_empty(&sk->sk_receive_queue) || - busy_loop_timeout(end_time)) + sk_busy_loop_timeout(sk, start_time)) return; goto restart; } -- cgit v1.2.3 From 7db6b048da3b9f84fe1d22fb29ff7e7c2ec6c0e5 Mon Sep 17 00:00:00 2001 From: Sridhar Samudrala Date: Fri, 24 Mar 2017 10:08:24 -0700 Subject: net: Commonize busy polling code to focus on napi_id instead of socket Move the core functionality in sk_busy_loop() to napi_busy_loop() and make it independent of sk. This enables re-using this function in epoll busy loop implementation. Signed-off-by: Sridhar Samudrala Signed-off-by: Alexander Duyck Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- include/net/busy_poll.h | 20 +++++++++++++++----- net/core/dev.c | 21 ++++++++------------- net/core/sock.c | 11 +++++++++++ 3 files changed, 34 insertions(+), 18 deletions(-) (limited to 'net/core/dev.c') diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h index 72c82f2ea536..8ffd434676b7 100644 --- a/include/net/busy_poll.h +++ b/include/net/busy_poll.h @@ -51,7 +51,11 @@ static inline bool sk_can_busy_loop(const struct sock *sk) return sk->sk_ll_usec && !signal_pending(current); } -void sk_busy_loop(struct sock *sk, int nonblock); +bool sk_busy_loop_end(void *p, unsigned long start_time); + +void napi_busy_loop(unsigned int napi_id, + bool (*loop_end)(void *, unsigned long), + void *loop_end_arg); #else /* CONFIG_NET_RX_BUSY_POLL */ static inline unsigned long net_busy_loop_on(void) @@ -64,10 +68,6 @@ static inline bool sk_can_busy_loop(struct sock *sk) return false; } -static inline void sk_busy_loop(struct sock *sk, int nonblock) -{ -} - #endif /* CONFIG_NET_RX_BUSY_POLL */ static inline unsigned long busy_loop_current_time(void) @@ -111,6 +111,16 @@ static inline bool sk_busy_loop_timeout(struct sock *sk, return true; } +static inline void sk_busy_loop(struct sock *sk, int nonblock) +{ +#ifdef CONFIG_NET_RX_BUSY_POLL + unsigned int napi_id = READ_ONCE(sk->sk_napi_id); + + if (napi_id >= MIN_NAPI_ID) + napi_busy_loop(napi_id, nonblock ? NULL : sk_busy_loop_end, sk); +#endif +} + /* used in the NIC receive handler to mark the skb */ static inline void skb_mark_napi_id(struct sk_buff *skb, struct napi_struct *napi) diff --git a/net/core/dev.c b/net/core/dev.c index 2d1b5613b7fd..ef9fe60ee294 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5060,19 +5060,16 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock) do_softirq(); } -void sk_busy_loop(struct sock *sk, int nonblock) +void napi_busy_loop(unsigned int napi_id, + bool (*loop_end)(void *, unsigned long), + void *loop_end_arg) { - unsigned long start_time = nonblock ? 0 : busy_loop_current_time(); + unsigned long start_time = loop_end ? busy_loop_current_time() : 0; int (*napi_poll)(struct napi_struct *napi, int budget); void *have_poll_lock = NULL; struct napi_struct *napi; - unsigned int napi_id; restart: - napi_id = READ_ONCE(sk->sk_napi_id); - if (napi_id < MIN_NAPI_ID) - return; - napi_poll = NULL; rcu_read_lock(); @@ -5106,12 +5103,11 @@ restart: trace_napi_poll(napi, work, BUSY_POLL_BUDGET); count: if (work > 0) - __NET_ADD_STATS(sock_net(sk), + __NET_ADD_STATS(dev_net(napi->dev), LINUX_MIB_BUSYPOLLRXPACKETS, work); local_bh_enable(); - if (nonblock || !skb_queue_empty(&sk->sk_receive_queue) || - sk_busy_loop_timeout(sk, start_time)) + if (!loop_end || loop_end(loop_end_arg, start_time)) break; if (unlikely(need_resched())) { @@ -5120,8 +5116,7 @@ count: preempt_enable(); rcu_read_unlock(); cond_resched(); - if (!skb_queue_empty(&sk->sk_receive_queue) || - sk_busy_loop_timeout(sk, start_time)) + if (loop_end(loop_end_arg, start_time)) return; goto restart; } @@ -5133,7 +5128,7 @@ count: out: rcu_read_unlock(); } -EXPORT_SYMBOL(sk_busy_loop); +EXPORT_SYMBOL(napi_busy_loop); #endif /* CONFIG_NET_RX_BUSY_POLL */ diff --git a/net/core/sock.c b/net/core/sock.c index 1b9030ee6f4b..4b762f2a3552 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -3237,3 +3237,14 @@ static int __init proto_init(void) subsys_initcall(proto_init); #endif /* PROC_FS */ + +#ifdef CONFIG_NET_RX_BUSY_POLL +bool sk_busy_loop_end(void *p, unsigned long start_time) +{ + struct sock *sk = p; + + return !skb_queue_empty(&sk->sk_receive_queue) || + sk_busy_loop_timeout(sk, start_time); +} +EXPORT_SYMBOL(sk_busy_loop_end); +#endif /* CONFIG_NET_RX_BUSY_POLL */ -- cgit v1.2.3