Skip to content

Commit

Permalink
mptcp: avoid atomic bit manipulation when possible
Browse files Browse the repository at this point in the history 
Currently the msk->flags bitmask carries both state for the
mptcp_release_cb() - mostly touched under the mptcp data lock
- and others state info touched even outside such lock scope.

As a consequence, msk->flags is always manipulated with
atomic operations.

This change splits such bitmask in two separate fields, so
that we use plain bit operations when touching the
cb-related info.

The MPTCP_PUSH_PENDING bit needs additional care, as it is the
only CB related field currently accessed either under the mptcp
data lock or the mptcp socket lock.
Let's add another mask just for such bit's sake.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Paolo Abeni authored and David S. Miller committed Jan 7, 2022
1 parent 3e50149 commit e9d09ba
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 31 deletions.
47 changes: 25 additions & 22 deletions net/mptcp/protocol.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ static bool move_skbs_to_msk(struct mptcp_sock *msk, struct sock *ssk)
if (!sock_owned_by_user(sk))
__mptcp_error_report(sk);
else
set_bit(MPTCP_ERROR_REPORT, &msk->flags);
__set_bit(MPTCP_ERROR_REPORT, &msk->cb_flags);
}

/* If the moves have caught up with the DATA_FIN sequence number
Expand Down Expand Up @@ -1517,9 +1517,8 @@ static void mptcp_update_post_push(struct mptcp_sock *msk,

void mptcp_check_and_set_pending(struct sock *sk)
{
if (mptcp_send_head(sk) &&
!test_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->flags))
set_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->flags);
if (mptcp_send_head(sk))
mptcp_sk(sk)->push_pending |= BIT(MPTCP_PUSH_PENDING);
}

void __mptcp_push_pending(struct sock *sk, unsigned int flags)
Expand Down Expand Up @@ -2134,7 +2133,7 @@ static void mptcp_retransmit_timer(struct timer_list *t)
mptcp_schedule_work(sk);
} else {
/* delegate our work to tcp_release_cb() */
set_bit(MPTCP_RETRANSMIT, &msk->flags);
__set_bit(MPTCP_RETRANSMIT, &msk->cb_flags);
}
bh_unlock_sock(sk);
sock_put(sk);
Expand Down Expand Up @@ -2840,7 +2839,9 @@ static int mptcp_disconnect(struct sock *sk, int flags)

mptcp_destroy_common(msk);
msk->last_snd = NULL;
msk->flags = 0;
WRITE_ONCE(msk->flags, 0);
msk->cb_flags = 0;
msk->push_pending = 0;
msk->recovery = false;
msk->can_ack = false;
msk->fully_established = false;
Expand Down Expand Up @@ -3021,7 +3022,7 @@ void __mptcp_data_acked(struct sock *sk)
if (!sock_owned_by_user(sk))
__mptcp_clean_una(sk);
else
set_bit(MPTCP_CLEAN_UNA, &mptcp_sk(sk)->flags);
__set_bit(MPTCP_CLEAN_UNA, &mptcp_sk(sk)->cb_flags);

if (mptcp_pending_data_fin_ack(sk))
mptcp_schedule_work(sk);
Expand All @@ -3040,22 +3041,23 @@ void __mptcp_check_push(struct sock *sk, struct sock *ssk)
else if (xmit_ssk)
mptcp_subflow_delegate(mptcp_subflow_ctx(xmit_ssk), MPTCP_DELEGATE_SEND);
} else {
set_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->flags);
__set_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->cb_flags);
}
}

#define MPTCP_FLAGS_PROCESS_CTX_NEED (BIT(MPTCP_PUSH_PENDING) | \
BIT(MPTCP_RETRANSMIT) | \
BIT(MPTCP_FLUSH_JOIN_LIST))

/* processes deferred events and flush wmem */
static void mptcp_release_cb(struct sock *sk)
__must_hold(&sk->sk_lock.slock)
{
struct mptcp_sock *msk = mptcp_sk(sk);

for (;;) {
unsigned long flags = 0;

if (test_and_clear_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->flags))
flags |= BIT(MPTCP_PUSH_PENDING);
if (test_and_clear_bit(MPTCP_RETRANSMIT, &mptcp_sk(sk)->flags))
flags |= BIT(MPTCP_RETRANSMIT);
if (test_and_clear_bit(MPTCP_FLUSH_JOIN_LIST, &mptcp_sk(sk)->flags))
flags |= BIT(MPTCP_FLUSH_JOIN_LIST);
unsigned long flags = (msk->cb_flags & MPTCP_FLAGS_PROCESS_CTX_NEED) |
msk->push_pending;
if (!flags)
break;

Expand All @@ -3066,7 +3068,8 @@ static void mptcp_release_cb(struct sock *sk)
* datapath acquires the msk socket spinlock while helding
* the subflow socket lock
*/

msk->push_pending = 0;
msk->cb_flags &= ~flags;
spin_unlock_bh(&sk->sk_lock.slock);
if (flags & BIT(MPTCP_FLUSH_JOIN_LIST))
__mptcp_flush_join_list(sk);
Expand All @@ -3082,11 +3085,11 @@ static void mptcp_release_cb(struct sock *sk)
/* be sure to set the current sk state before tacking actions
* depending on sk_state
*/
if (test_and_clear_bit(MPTCP_CONNECTED, &mptcp_sk(sk)->flags))
if (__test_and_clear_bit(MPTCP_CONNECTED, &msk->cb_flags))
__mptcp_set_connected(sk);
if (test_and_clear_bit(MPTCP_CLEAN_UNA, &mptcp_sk(sk)->flags))
if (__test_and_clear_bit(MPTCP_CLEAN_UNA, &msk->cb_flags))
__mptcp_clean_una_wakeup(sk);
if (test_and_clear_bit(MPTCP_ERROR_REPORT, &mptcp_sk(sk)->flags))
if (__test_and_clear_bit(MPTCP_ERROR_REPORT, &msk->cb_flags))
__mptcp_error_report(sk);

__mptcp_update_rmem(sk);
Expand Down Expand Up @@ -3128,7 +3131,7 @@ void mptcp_subflow_process_delegated(struct sock *ssk)
if (!sock_owned_by_user(sk))
__mptcp_subflow_push_pending(sk, ssk);
else
set_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->flags);
__set_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->cb_flags);
mptcp_data_unlock(sk);
mptcp_subflow_delegated_done(subflow, MPTCP_DELEGATE_SEND);
}
Expand Down Expand Up @@ -3247,7 +3250,7 @@ bool mptcp_finish_join(struct sock *ssk)
} else {
sock_hold(ssk);
list_add_tail(&subflow->node, &msk->join_list);
set_bit(MPTCP_FLUSH_JOIN_LIST, &msk->flags);
__set_bit(MPTCP_FLUSH_JOIN_LIST, &msk->cb_flags);
}
mptcp_data_unlock(parent);

Expand Down
18 changes: 11 additions & 7 deletions net/mptcp/protocol.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,18 +110,20 @@
/* MPTCP TCPRST flags */
#define MPTCP_RST_TRANSIENT BIT(0)

/* MPTCP socket flags */
/* MPTCP socket atomic flags */
#define MPTCP_NOSPACE 1
#define MPTCP_WORK_RTX 2
#define MPTCP_WORK_EOF 3
#define MPTCP_FALLBACK_DONE 4
#define MPTCP_WORK_CLOSE_SUBFLOW 5
#define MPTCP_PUSH_PENDING 6
#define MPTCP_CLEAN_UNA 7
#define MPTCP_ERROR_REPORT 8
#define MPTCP_RETRANSMIT 9
#define MPTCP_FLUSH_JOIN_LIST 10
#define MPTCP_CONNECTED 11

/* MPTCP socket release cb flags */
#define MPTCP_PUSH_PENDING 1
#define MPTCP_CLEAN_UNA 2
#define MPTCP_ERROR_REPORT 3
#define MPTCP_RETRANSMIT 4
#define MPTCP_FLUSH_JOIN_LIST 5
#define MPTCP_CONNECTED 6

static inline bool before64(__u64 seq1, __u64 seq2)
{
Expand Down Expand Up @@ -250,6 +252,8 @@ struct mptcp_sock {
u32 token;
int rmem_released;
unsigned long flags;
unsigned long cb_flags;
unsigned long push_pending;
bool recovery; /* closing subflow write queue reinjected */
bool can_ack;
bool fully_established;
Expand Down
4 changes: 2 additions & 2 deletions net/mptcp/subflow.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ static void mptcp_set_connected(struct sock *sk)
if (!sock_owned_by_user(sk))
__mptcp_set_connected(sk);
else
set_bit(MPTCP_CONNECTED, &mptcp_sk(sk)->flags);
__set_bit(MPTCP_CONNECTED, &mptcp_sk(sk)->cb_flags);
mptcp_data_unlock(sk);
}

Expand Down Expand Up @@ -1274,7 +1274,7 @@ static void subflow_error_report(struct sock *ssk)
if (!sock_owned_by_user(sk))
__mptcp_error_report(sk);
else
set_bit(MPTCP_ERROR_REPORT, &mptcp_sk(sk)->flags);
__set_bit(MPTCP_ERROR_REPORT, &mptcp_sk(sk)->cb_flags);
mptcp_data_unlock(sk);
}

Expand Down

0 comments on commit e9d09ba

Please sign in to comment.