From 9f4ad55602210f68e5628f18246bee649fbfb241 Mon Sep 17 00:00:00 2001 From: Chris Lew Date: Mon, 4 May 2020 11:33:56 -0700 Subject: [PATCH 1/2] net: qrtr: Do not take sock lock on blocking recv Socket client's should be able to block on recv without blocking other socket operations such as sending. The qrtr core logic should also be able to operate on the sock while a client is blocked in recv. Remove the sock lock from around the recv_datagram to allow execution to continue while a client is blocked in recv. Change-Id: I6adb09b8e44c5ff3542b88db87b376a3ae5cb5ba Signed-off-by: Chris Lew --- net/qrtr/qrtr.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c index f413384bcccf..57469f4898b9 100644 --- a/net/qrtr/qrtr.c +++ b/net/qrtr/qrtr.c @@ -1656,19 +1656,16 @@ static int qrtr_recvmsg(struct socket *sock, struct msghdr *msg, struct qrtr_cb *cb; int copied, rc; - lock_sock(sk); - if (sock_flag(sk, SOCK_ZAPPED)) { - release_sock(sk); + if (sock_flag(sk, SOCK_ZAPPED)) return -EADDRNOTAVAIL; - } skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT, flags & MSG_DONTWAIT, &rc); - if (!skb) { - release_sock(sk); + if (!skb) return rc; - } + + lock_sock(sk); cb = (struct qrtr_cb *)skb->cb; copied = skb->len; From 28ed3ab34ffabbcd3957ff85d5941b39c50b05db Mon Sep 17 00:00:00 2001 From: Chris Lew Date: Tue, 19 May 2020 18:54:43 -0700 Subject: [PATCH 2/2] net: qrtr: Remove rcu logic for port lookup/remove synchronize_rcu() is slow and can create delays on creating/removing sockets. Some clients have latency requirements for spinning off a thread that creates a client and lookup sockets for some sessions. They close the lookup socket during this sequence which causes delays. Each call to synchronize_rcu takes 20-40 ms which is too long. Remove the rcu logic and change qrtr_port_lock to a spinlock so it can be used in qrtr_port_lookup which may be used in atomic context. Change-Id: I3244e31d3ca0599df794d3936033735bcdd3e972 Signed-off-by: Chris Lew --- net/qrtr/qrtr.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c index 57469f4898b9..1753201d4ac4 100644 --- a/net/qrtr/qrtr.c +++ b/net/qrtr/qrtr.c @@ -128,7 +128,7 @@ static DECLARE_RWSEM(qrtr_epts_lock); /* local port allocation management */ static DEFINE_IDR(qrtr_ports); -static DEFINE_MUTEX(qrtr_port_lock); +static DEFINE_SPINLOCK(qrtr_port_lock); /** * struct qrtr_node - endpoint node @@ -1191,15 +1191,16 @@ EXPORT_SYMBOL_GPL(qrtr_endpoint_unregister); static struct qrtr_sock *qrtr_port_lookup(int port) { struct qrtr_sock *ipc; + unsigned long flags; if (port == QRTR_PORT_CTRL) port = 0; - rcu_read_lock(); + spin_lock_irqsave(&qrtr_port_lock, flags); ipc = idr_find(&qrtr_ports, port); if (ipc) sock_hold(&ipc->sk); - rcu_read_unlock(); + spin_unlock_irqrestore(&qrtr_port_lock, flags); return ipc; } @@ -1261,6 +1262,7 @@ exit: static void qrtr_port_remove(struct qrtr_sock *ipc) { int port = ipc->us.sq_port; + unsigned long flags; qrtr_send_del_client(ipc); if (port == QRTR_PORT_CTRL) @@ -1268,13 +1270,9 @@ static void qrtr_port_remove(struct qrtr_sock *ipc) __sock_put(&ipc->sk); - mutex_lock(&qrtr_port_lock); + spin_lock_irqsave(&qrtr_port_lock, flags); idr_remove(&qrtr_ports, port); - mutex_unlock(&qrtr_port_lock); - - /* Ensure that if qrtr_port_lookup() did enter the RCU read section we - * wait for it to up increment the refcount */ - synchronize_rcu(); + spin_unlock_irqrestore(&qrtr_port_lock, flags); } /* Assign port number to socket. @@ -1348,6 +1346,7 @@ static int __qrtr_bind(struct socket *sock, { struct qrtr_sock *ipc = qrtr_sk(sock->sk); struct sock *sk = sock->sk; + unsigned long flags; int port; int rc; @@ -1355,17 +1354,18 @@ static int __qrtr_bind(struct socket *sock, if (!zapped && addr->sq_port == ipc->us.sq_port) return 0; - mutex_lock(&qrtr_port_lock); + spin_lock_irqsave(&qrtr_port_lock, flags); port = addr->sq_port; rc = qrtr_port_assign(ipc, &port); if (rc) { - mutex_unlock(&qrtr_port_lock); + spin_unlock_irqrestore(&qrtr_port_lock, flags); return rc; } /* Notify all open ports about the new controller */ if (port == QRTR_PORT_CTRL) qrtr_reset_ports(); - mutex_unlock(&qrtr_port_lock); + spin_unlock_irqrestore(&qrtr_port_lock, flags); + if (port == QRTR_PORT_CTRL) { struct qrtr_node *node;