| Message ID | f18ab6d96263347de227dadcef3aff102f682ab9.1622045130.git.pabeni@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mat Martineau |
| Headers | show |
| Series | [mptcp-next] mptcp: use fast lock for subflows when possible. | expand |
On Wed, 26 May 2021, Paolo Abeni wrote: > There are a bunch of callsite where the ssk socket > lock is acquired using the full-blown version eligible for > the fast variant. Let's move to the latter. > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > The previous patch demonstrated that using lock_sock() > were the fast version is available can have worse side-effect > than slower performances. > > This tries to address the topic. > --- > net/mptcp/pm_netlink.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) Looks good to me, thanks Paolo. Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com> -Mat > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > index 09722598994d..d4732a4f223e 100644 > --- a/net/mptcp/pm_netlink.c > +++ b/net/mptcp/pm_netlink.c > @@ -540,6 +540,7 @@ void mptcp_pm_nl_addr_send_ack(struct mptcp_sock *msk) > subflow = list_first_entry_or_null(&msk->conn_list, typeof(*subflow), node); > if (subflow) { > struct sock *ssk = mptcp_subflow_tcp_sock(subflow); > + bool slow; > > spin_unlock_bh(&msk->pm.lock); > pr_debug("send ack for %s%s%s", > @@ -547,9 +548,9 @@ void mptcp_pm_nl_addr_send_ack(struct mptcp_sock *msk) > mptcp_pm_should_add_signal_ipv6(msk) ? " [ipv6]" : "", > mptcp_pm_should_add_signal_port(msk) ? " [port]" : ""); > > - lock_sock(ssk); > + slow = lock_sock_fast(ssk); > tcp_send_ack(ssk); > - release_sock(ssk); > + unlock_sock_fast(ssk, slow); > spin_lock_bh(&msk->pm.lock); > } > } > @@ -566,6 +567,7 @@ int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk, > struct sock *ssk = mptcp_subflow_tcp_sock(subflow); > struct sock *sk = (struct sock *)msk; > struct mptcp_addr_info local; > + bool slow; > > local_address((struct sock_common *)ssk, &local); > if (!addresses_equal(&local, addr, addr->port)) > @@ -578,9 +580,9 @@ int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk, > > spin_unlock_bh(&msk->pm.lock); > pr_debug("send ack for mp_prio"); > - lock_sock(ssk); > + slow = lock_sock_fast(ssk); > tcp_send_ack(ssk); > - release_sock(ssk); > + unlock_sock_fast(ssk, slow); > spin_lock_bh(&msk->pm.lock); > > return 0; > -- > 2.26.3 > > > -- Mat Martineau Intel
On Wed, 2021-05-26 at 21:19 -0700, Mat Martineau wrote: > On Wed, 26 May 2021, Paolo Abeni wrote: > > > There are a bunch of callsite where the ssk socket > > lock is acquired using the full-blown version eligible for > > the fast variant. Let's move to the latter. > > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > > --- > > The previous patch demonstrated that using lock_sock() > > were the fast version is available can have worse side-effect > > than slower performances. > > > > This tries to address the topic. > > --- > > net/mptcp/pm_netlink.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > Looks good to me, thanks Paolo. > > Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com> I'm sorry, this get mangled, some more needed chunk slipped in another local patch. I'll send a v2 with more fast_lock() /P
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 09722598994d..d4732a4f223e 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -540,6 +540,7 @@ void mptcp_pm_nl_addr_send_ack(struct mptcp_sock *msk) subflow = list_first_entry_or_null(&msk->conn_list, typeof(*subflow), node); if (subflow) { struct sock *ssk = mptcp_subflow_tcp_sock(subflow); + bool slow; spin_unlock_bh(&msk->pm.lock); pr_debug("send ack for %s%s%s", @@ -547,9 +548,9 @@ void mptcp_pm_nl_addr_send_ack(struct mptcp_sock *msk) mptcp_pm_should_add_signal_ipv6(msk) ? " [ipv6]" : "", mptcp_pm_should_add_signal_port(msk) ? " [port]" : ""); - lock_sock(ssk); + slow = lock_sock_fast(ssk); tcp_send_ack(ssk); - release_sock(ssk); + unlock_sock_fast(ssk, slow); spin_lock_bh(&msk->pm.lock); } } @@ -566,6 +567,7 @@ int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk, struct sock *ssk = mptcp_subflow_tcp_sock(subflow); struct sock *sk = (struct sock *)msk; struct mptcp_addr_info local; + bool slow; local_address((struct sock_common *)ssk, &local); if (!addresses_equal(&local, addr, addr->port)) @@ -578,9 +580,9 @@ int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk, spin_unlock_bh(&msk->pm.lock); pr_debug("send ack for mp_prio"); - lock_sock(ssk); + slow = lock_sock_fast(ssk); tcp_send_ack(ssk); - release_sock(ssk); + unlock_sock_fast(ssk, slow); spin_lock_bh(&msk->pm.lock); return 0;
There are a bunch of callsite where the ssk socket lock is acquired using the full-blown version eligible for the fast variant. Let's move to the latter. Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- The previous patch demonstrated that using lock_sock() were the fast version is available can have worse side-effect than slower performances. This tries to address the topic. --- net/mptcp/pm_netlink.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)