* socket poll related cleanups v2
@ 2018-07-30 7:42 Christoph Hellwig
2018-07-30 7:42 ` [PATCH 1/4] net: simplify sock_poll_wait Christoph Hellwig
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Christoph Hellwig @ 2018-07-30 7:42 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
A couple of cleanups I stumbled upon when studying the networking
poll code.
Changes since v1:
- drop a dispute patch from this series (to be sent separately)
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/4] net: simplify sock_poll_wait 2018-07-30 7:42 socket poll related cleanups v2 Christoph Hellwig @ 2018-07-30 7:42 ` Christoph Hellwig 2018-07-30 7:42 ` [PATCH 2/4] net: don not detour through struct sock to find the poll waitqueue Christoph Hellwig ` (3 subsequent siblings) 4 siblings, 0 replies; 7+ messages in thread From: Christoph Hellwig @ 2018-07-30 7:42 UTC (permalink / raw) To: netdev; +Cc: linux-kernel The wait_address argument is always directly derived from the filp argument, so remove it. Signed-off-by: Christoph Hellwig <hch@lst.de> --- crypto/af_alg.c | 2 +- include/net/sock.h | 11 ++++++----- net/atm/common.c | 2 +- net/caif/caif_socket.c | 2 +- net/core/datagram.c | 2 +- net/dccp/proto.c | 2 +- net/ipv4/tcp.c | 2 +- net/iucv/af_iucv.c | 2 +- net/nfc/llcp_sock.c | 2 +- net/rxrpc/af_rxrpc.c | 2 +- net/smc/af_smc.c | 2 +- net/tipc/socket.c | 2 +- net/unix/af_unix.c | 4 ++-- 13 files changed, 19 insertions(+), 18 deletions(-) diff --git a/crypto/af_alg.c b/crypto/af_alg.c index c166f424871c..b053179e0bc5 100644 --- a/crypto/af_alg.c +++ b/crypto/af_alg.c @@ -1071,7 +1071,7 @@ __poll_t af_alg_poll(struct file *file, struct socket *sock, struct af_alg_ctx *ctx = ask->private; __poll_t mask; - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); mask = 0; if (!ctx->more || ctx->used) diff --git a/include/net/sock.h b/include/net/sock.h index b3b75419eafe..762069bb0d8c 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -1994,16 +1994,17 @@ static inline bool skwq_has_sleeper(struct socket_wq *wq) /** * sock_poll_wait - place memory barrier behind the poll_wait call. * @filp: file - * @wait_address: socket wait queue * @p: poll_table * * See the comments in the wq_has_sleeper function. */ -static inline void sock_poll_wait(struct file *filp, - wait_queue_head_t *wait_address, poll_table *p) +static inline void sock_poll_wait(struct file *filp, poll_table *p) { - if (!poll_does_not_wait(p) && wait_address) { - poll_wait(filp, wait_address, p); + struct socket *sock = filp->private_data; + wait_queue_head_t *wq = sk_sleep(sock->sk); + + if (!poll_does_not_wait(p) && wq) { + poll_wait(filp, wq, p); /* We need to be sure we are in sync with the * socket flags modification. * diff --git a/net/atm/common.c b/net/atm/common.c index a7a68e509628..9f8cb0d2e71e 100644 --- a/net/atm/common.c +++ b/net/atm/common.c @@ -653,7 +653,7 @@ __poll_t vcc_poll(struct file *file, struct socket *sock, poll_table *wait) struct atm_vcc *vcc; __poll_t mask; - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); mask = 0; vcc = ATM_SD(sock); diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c index a6fb1b3bcad9..d18965f3291f 100644 --- a/net/caif/caif_socket.c +++ b/net/caif/caif_socket.c @@ -941,7 +941,7 @@ static __poll_t caif_poll(struct file *file, __poll_t mask; struct caifsock *cf_sk = container_of(sk, struct caifsock, sk); - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); mask = 0; /* exceptional events? */ diff --git a/net/core/datagram.c b/net/core/datagram.c index 9938952c5c78..9aac0d63d53e 100644 --- a/net/core/datagram.c +++ b/net/core/datagram.c @@ -837,7 +837,7 @@ __poll_t datagram_poll(struct file *file, struct socket *sock, struct sock *sk = sock->sk; __poll_t mask; - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); mask = 0; /* exceptional events? */ diff --git a/net/dccp/proto.c b/net/dccp/proto.c index 0d56e36a6db7..875858c8b059 100644 --- a/net/dccp/proto.c +++ b/net/dccp/proto.c @@ -325,7 +325,7 @@ __poll_t dccp_poll(struct file *file, struct socket *sock, __poll_t mask; struct sock *sk = sock->sk; - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); if (sk->sk_state == DCCP_LISTEN) return inet_csk_listen_poll(sk); diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 4491faf83f4f..0b93290c9255 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -507,7 +507,7 @@ __poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait) const struct tcp_sock *tp = tcp_sk(sk); int state; - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); state = inet_sk_state_load(sk); if (state == TCP_LISTEN) diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c index 893a022f9620..e7b93cd14b52 100644 --- a/net/iucv/af_iucv.c +++ b/net/iucv/af_iucv.c @@ -1494,7 +1494,7 @@ __poll_t iucv_sock_poll(struct file *file, struct socket *sock, struct sock *sk = sock->sk; __poll_t mask = 0; - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); if (sk->sk_state == IUCV_LISTEN) return iucv_accept_poll(sk); diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c index ea0c0c6f1874..dd4adf8b1167 100644 --- a/net/nfc/llcp_sock.c +++ b/net/nfc/llcp_sock.c @@ -556,7 +556,7 @@ static __poll_t llcp_sock_poll(struct file *file, struct socket *sock, pr_debug("%p\n", sk); - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); if (sk->sk_state == LLCP_LISTEN) return llcp_accept_poll(sk); diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c index 2b463047dd7b..ac44d8afffb1 100644 --- a/net/rxrpc/af_rxrpc.c +++ b/net/rxrpc/af_rxrpc.c @@ -741,7 +741,7 @@ static __poll_t rxrpc_poll(struct file *file, struct socket *sock, struct rxrpc_sock *rx = rxrpc_sk(sk); __poll_t mask; - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); mask = 0; /* the socket is readable if there are any messages waiting on the Rx diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c index 05e4ffe5aabd..09df79313f60 100644 --- a/net/smc/af_smc.c +++ b/net/smc/af_smc.c @@ -1351,7 +1351,7 @@ static __poll_t smc_poll(struct file *file, struct socket *sock, mask |= EPOLLERR; } else { if (sk->sk_state != SMC_CLOSED) - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); if (sk->sk_err) mask |= EPOLLERR; if ((sk->sk_shutdown == SHUTDOWN_MASK) || diff --git a/net/tipc/socket.c b/net/tipc/socket.c index 930852c54d7a..9a9050131724 100644 --- a/net/tipc/socket.c +++ b/net/tipc/socket.c @@ -716,7 +716,7 @@ static __poll_t tipc_poll(struct file *file, struct socket *sock, struct tipc_sock *tsk = tipc_sk(sk); __poll_t revents = 0; - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); if (sk->sk_shutdown & RCV_SHUTDOWN) revents |= EPOLLRDHUP | EPOLLIN | EPOLLRDNORM; diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index e5473c03d667..1772a0e32665 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -2635,7 +2635,7 @@ static __poll_t unix_poll(struct file *file, struct socket *sock, poll_table *wa struct sock *sk = sock->sk; __poll_t mask; - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); mask = 0; /* exceptional events? */ @@ -2672,7 +2672,7 @@ static __poll_t unix_dgram_poll(struct file *file, struct socket *sock, unsigned int writable; __poll_t mask; - sock_poll_wait(file, sk_sleep(sk), wait); + sock_poll_wait(file, wait); mask = 0; /* exceptional events? */ -- 2.18.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] net: don not detour through struct sock to find the poll waitqueue 2018-07-30 7:42 socket poll related cleanups v2 Christoph Hellwig 2018-07-30 7:42 ` [PATCH 1/4] net: simplify sock_poll_wait Christoph Hellwig @ 2018-07-30 7:42 ` Christoph Hellwig 2018-07-30 7:42 ` [PATCH 3/4] net: remove sock_poll_busy_loop Christoph Hellwig ` (2 subsequent siblings) 4 siblings, 0 replies; 7+ messages in thread From: Christoph Hellwig @ 2018-07-30 7:42 UTC (permalink / raw) To: netdev; +Cc: linux-kernel For any open socket file descriptor sock->sk->sk_wq->wait will always point to sock->wq->wait. That means we can do the shorter dereference and removal a NULL check and don't have to not worry about any RCU protection. Signed-off-by: Christoph Hellwig <hch@lst.de> --- include/net/sock.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/net/sock.h b/include/net/sock.h index 762069bb0d8c..7927541101a8 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -2001,10 +2001,9 @@ static inline bool skwq_has_sleeper(struct socket_wq *wq) static inline void sock_poll_wait(struct file *filp, poll_table *p) { struct socket *sock = filp->private_data; - wait_queue_head_t *wq = sk_sleep(sock->sk); - if (!poll_does_not_wait(p) && wq) { - poll_wait(filp, wq, p); + if (!poll_does_not_wait(p)) { + poll_wait(filp, &sock->wq->wait, p); /* We need to be sure we are in sync with the * socket flags modification. * -- 2.18.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/4] net: remove sock_poll_busy_loop 2018-07-30 7:42 socket poll related cleanups v2 Christoph Hellwig 2018-07-30 7:42 ` [PATCH 1/4] net: simplify sock_poll_wait Christoph Hellwig 2018-07-30 7:42 ` [PATCH 2/4] net: don not detour through struct sock to find the poll waitqueue Christoph Hellwig @ 2018-07-30 7:42 ` Christoph Hellwig 2018-07-30 7:42 ` [PATCH 4/4] net: remove sock_poll_busy_flag Christoph Hellwig 2018-07-30 16:13 ` socket poll related cleanups v2 David Miller 4 siblings, 0 replies; 7+ messages in thread From: Christoph Hellwig @ 2018-07-30 7:42 UTC (permalink / raw) To: netdev; +Cc: linux-kernel There is no point in hiding this logic in a helper. Also remove the useless events != 0 check and only busy loop once we know we actually have a poll method. Signed-off-by: Christoph Hellwig <hch@lst.de> --- include/net/busy_poll.h | 9 --------- net/socket.c | 5 ++++- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h index c5187438af38..25d762cf47f2 100644 --- a/include/net/busy_poll.h +++ b/include/net/busy_poll.h @@ -121,15 +121,6 @@ static inline void sk_busy_loop(struct sock *sk, int nonblock) #endif } -static inline void sock_poll_busy_loop(struct socket *sock, __poll_t events) -{ - if (sk_can_busy_loop(sock->sk) && - events && (events & POLL_BUSY_LOOP)) { - /* once, only if requested by syscall */ - sk_busy_loop(sock->sk, 1); - } -} - /* if this socket can poll_ll, tell the system call */ static inline __poll_t sock_poll_busy_flag(struct socket *sock) { diff --git a/net/socket.c b/net/socket.c index 85633622c94d..674434127b3a 100644 --- a/net/socket.c +++ b/net/socket.c @@ -1132,9 +1132,12 @@ static __poll_t sock_poll(struct file *file, poll_table *wait) struct socket *sock = file->private_data; __poll_t events = poll_requested_events(wait); - sock_poll_busy_loop(sock, events); if (!sock->ops->poll) return 0; + + /* poll once if requested by the syscall */ + if (sk_can_busy_loop(sock->sk) && (events & POLL_BUSY_LOOP)) + sk_busy_loop(sock->sk, 1); return sock->ops->poll(file, sock, wait) | sock_poll_busy_flag(sock); } -- 2.18.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/4] net: remove sock_poll_busy_flag 2018-07-30 7:42 socket poll related cleanups v2 Christoph Hellwig ` (2 preceding siblings ...) 2018-07-30 7:42 ` [PATCH 3/4] net: remove sock_poll_busy_loop Christoph Hellwig @ 2018-07-30 7:42 ` Christoph Hellwig 2018-07-30 16:13 ` socket poll related cleanups v2 David Miller 4 siblings, 0 replies; 7+ messages in thread From: Christoph Hellwig @ 2018-07-30 7:42 UTC (permalink / raw) To: netdev; +Cc: linux-kernel Fold it into the only caller to make the code simpler and easier to read. Signed-off-by: Christoph Hellwig <hch@lst.de> --- include/net/busy_poll.h | 6 ------ net/socket.c | 16 +++++++++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h index 25d762cf47f2..71c72a939bf8 100644 --- a/include/net/busy_poll.h +++ b/include/net/busy_poll.h @@ -121,12 +121,6 @@ static inline void sk_busy_loop(struct sock *sk, int nonblock) #endif } -/* if this socket can poll_ll, tell the system call */ -static inline __poll_t sock_poll_busy_flag(struct socket *sock) -{ - return sk_can_busy_loop(sock->sk) ? POLL_BUSY_LOOP : 0; -} - /* 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/socket.c b/net/socket.c index 674434127b3a..5b7df6695f4f 100644 --- a/net/socket.c +++ b/net/socket.c @@ -1130,15 +1130,21 @@ EXPORT_SYMBOL(sock_create_lite); static __poll_t sock_poll(struct file *file, poll_table *wait) { struct socket *sock = file->private_data; - __poll_t events = poll_requested_events(wait); + __poll_t events = poll_requested_events(wait), flag = 0; if (!sock->ops->poll) return 0; - /* poll once if requested by the syscall */ - if (sk_can_busy_loop(sock->sk) && (events & POLL_BUSY_LOOP)) - sk_busy_loop(sock->sk, 1); - return sock->ops->poll(file, sock, wait) | sock_poll_busy_flag(sock); + if (sk_can_busy_loop(sock->sk)) { + /* poll once if requested by the syscall */ + if (events & POLL_BUSY_LOOP) + sk_busy_loop(sock->sk, 1); + + /* if this socket can poll_ll, tell the system call */ + flag = POLL_BUSY_LOOP; + } + + return sock->ops->poll(file, sock, wait) | flag; } static int sock_mmap(struct file *file, struct vm_area_struct *vma) -- 2.18.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: socket poll related cleanups v2 2018-07-30 7:42 socket poll related cleanups v2 Christoph Hellwig ` (3 preceding siblings ...) 2018-07-30 7:42 ` [PATCH 4/4] net: remove sock_poll_busy_flag Christoph Hellwig @ 2018-07-30 16:13 ` David Miller 4 siblings, 0 replies; 7+ messages in thread From: David Miller @ 2018-07-30 16:13 UTC (permalink / raw) To: hch; +Cc: netdev, linux-kernel From: Christoph Hellwig <hch@lst.de> Date: Mon, 30 Jul 2018 09:42:09 +0200 > A couple of cleanups I stumbled upon when studying the networking > poll code. > > Changes since v1: > - drop a dispute patch from this series (to be sent separately) Series applied, thanks! ^ permalink raw reply [flat|nested] 7+ messages in thread
* socket poll related cleanups v2 @ 2018-07-30 7:45 Christoph Hellwig 0 siblings, 0 replies; 7+ messages in thread From: Christoph Hellwig @ 2018-07-30 7:45 UTC (permalink / raw) To: Eric Dumazet, Paul E. McKenney, netdev; +Cc: linux-kernel Eric, Paul, can you please look over the patch below? The RCU annotations here don't make any sense as we never RCU access through the annotated pointer ( we do access the underlying object through RCU and thus need to RCU free, but that is a different story). Dave doesn't seem to be convinced, though. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-07-30 16:13 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-07-30 7:42 socket poll related cleanups v2 Christoph Hellwig 2018-07-30 7:42 ` [PATCH 1/4] net: simplify sock_poll_wait Christoph Hellwig 2018-07-30 7:42 ` [PATCH 2/4] net: don not detour through struct sock to find the poll waitqueue Christoph Hellwig 2018-07-30 7:42 ` [PATCH 3/4] net: remove sock_poll_busy_loop Christoph Hellwig 2018-07-30 7:42 ` [PATCH 4/4] net: remove sock_poll_busy_flag Christoph Hellwig 2018-07-30 16:13 ` socket poll related cleanups v2 David Miller -- strict thread matches above, loose matches on Subject: below -- 2018-07-30 7:45 Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox