sctp: use sk_wmem_queued to check for writable space

sk->sk_wmem_queued is used to count the size of chunks in out queue
while sk->sk_wmem_alloc is for counting the size of chunks has been
sent. sctp is increasing both of them before enqueuing the chunks,
and using sk->sk_wmem_alloc to check for writable space.

However, sk_wmem_alloc is also increased by 1 for the skb allocked
for sending in sctp_packet_transmit() but it will not wake up the
waiters when sk_wmem_alloc is decreased in this skb's destructor.

If msg size is equal to sk_sndbuf and sendmsg is waiting for sndbuf,
the check 'msg_len <= sctp_wspace(asoc)' in sctp_wait_for_sndbuf()
will keep waiting if there's a skb allocked in sctp_packet_transmit,
and later even if this skb got freed, the waiting thread will never
get waked up.

This issue has been there since very beginning, so we change to use
sk->sk_wmem_queued to check for writable space as sk_wmem_queued is
not increased for the skb allocked for sending, also as TCP does.

SOCK_SNDBUF_LOCK check is also removed here as it's for tx buf auto
tuning which I will add in another patch.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
Xin Long 2018-10-17 03:07:51 +08:00 committed by David S. Miller
parent 605c0ac182
commit cd305c74b0

View File

@ -83,7 +83,7 @@
#include <net/sctp/stream_sched.h> #include <net/sctp/stream_sched.h>
/* Forward declarations for internal helper functions. */ /* Forward declarations for internal helper functions. */
static int sctp_writeable(struct sock *sk); static bool sctp_writeable(struct sock *sk);
static void sctp_wfree(struct sk_buff *skb); static void sctp_wfree(struct sk_buff *skb);
static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p, static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
size_t msg_len); size_t msg_len);
@ -119,25 +119,10 @@ static void sctp_enter_memory_pressure(struct sock *sk)
/* Get the sndbuf space available at the time on the association. */ /* Get the sndbuf space available at the time on the association. */
static inline int sctp_wspace(struct sctp_association *asoc) static inline int sctp_wspace(struct sctp_association *asoc)
{ {
int amt; struct sock *sk = asoc->base.sk;
if (asoc->ep->sndbuf_policy) return asoc->ep->sndbuf_policy ? sk->sk_sndbuf - asoc->sndbuf_used
amt = asoc->sndbuf_used; : sk_stream_wspace(sk);
else
amt = sk_wmem_alloc_get(asoc->base.sk);
if (amt >= asoc->base.sk->sk_sndbuf) {
if (asoc->base.sk->sk_userlocks & SOCK_SNDBUF_LOCK)
amt = 0;
else {
amt = sk_stream_wspace(asoc->base.sk);
if (amt < 0)
amt = 0;
}
} else {
amt = asoc->base.sk->sk_sndbuf - amt;
}
return amt;
} }
/* Increment the used sndbuf space count of the corresponding association by /* Increment the used sndbuf space count of the corresponding association by
@ -1925,10 +1910,10 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
asoc->pmtu_pending = 0; asoc->pmtu_pending = 0;
} }
if (sctp_wspace(asoc) < msg_len) if (sctp_wspace(asoc) < (int)msg_len)
sctp_prsctp_prune(asoc, sinfo, msg_len - sctp_wspace(asoc)); sctp_prsctp_prune(asoc, sinfo, msg_len - sctp_wspace(asoc));
if (!sctp_wspace(asoc)) { if (sctp_wspace(asoc) <= 0) {
timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT); timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len); err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len);
if (err) if (err)
@ -8535,7 +8520,7 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
goto do_error; goto do_error;
if (signal_pending(current)) if (signal_pending(current))
goto do_interrupted; goto do_interrupted;
if (msg_len <= sctp_wspace(asoc)) if ((int)msg_len <= sctp_wspace(asoc))
break; break;
/* Let another process have a go. Since we are going /* Let another process have a go. Since we are going
@ -8610,14 +8595,9 @@ void sctp_write_space(struct sock *sk)
* UDP-style sockets or TCP-style sockets, this code should work. * UDP-style sockets or TCP-style sockets, this code should work.
* - Daisy * - Daisy
*/ */
static int sctp_writeable(struct sock *sk) static bool sctp_writeable(struct sock *sk)
{ {
int amt = 0; return sk->sk_sndbuf > sk->sk_wmem_queued;
amt = sk->sk_sndbuf - sk_wmem_alloc_get(sk);
if (amt < 0)
amt = 0;
return amt;
} }
/* Wait for an association to go into ESTABLISHED state. If timeout is 0, /* Wait for an association to go into ESTABLISHED state. If timeout is 0,