On Wed, May 20, 2020 at 09:55:07PM +0200, Christoph Hellwig wrote:
> Add a helper to directly set the SCTP_NODELAY sockopt from kernel space
> without going through a fake uaccess.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/dlm/lowcomms.c | 10 ++--------
> include/net/sctp/sctp.h | 7 +++++++
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> index 69333728d871b..9f1c3cdc9d653 100644
> --- a/fs/dlm/lowcomms.c
> +++ b/fs/dlm/lowcomms.c
> @@ -914,7 +914,6 @@ static int sctp_bind_addrs(struct connection *con, uint16_t port)
> static void sctp_connect_to_sock(struct connection *con)
> {
> struct sockaddr_storage daddr;
> - int one = 1;
> int result;
> int addr_len;
> struct socket *sock;
> @@ -961,8 +960,7 @@ static void sctp_connect_to_sock(struct connection *con)
> log_print("connecting to %d", con->nodeid);
>
> /* Turn off Nagle's algorithm */
> - kernel_setsockopt(sock, SOL_SCTP, SCTP_NODELAY, (char *)&one,
> - sizeof(one));
> + sctp_sock_set_nodelay(sock->sk);
>
> /*
> * Make sock->ops->connect() function return in specified time,
> @@ -1176,7 +1174,6 @@ static int sctp_listen_for_all(void)
> struct socket *sock = NULL;
> int result = -EINVAL;
> struct connection *con = nodeid2con(0, GFP_NOFS);
> - int one = 1;
>
> if (!con)
> return -ENOMEM;
> @@ -1191,10 +1188,7 @@ static int sctp_listen_for_all(void)
> }
>
> sock_set_rcvbuf(sock->sk, NEEDED_RMEM);
> - result = kernel_setsockopt(sock, SOL_SCTP, SCTP_NODELAY, (char *)&one,
> - sizeof(one));
> - if (result < 0)
> - log_print("Could not set SCTP NODELAY error %d\n", result);
> + sctp_sock_set_nodelay(sock->sk);
>
> write_lock_bh(&sock->sk->sk_callback_lock);
> /* Init con struct */
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index 3ab5c6bbb90bd..f8bcb75bb0448 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -615,4 +615,11 @@ static inline bool sctp_newsk_ready(const struct sock *sk)
> return sock_flag(sk, SOCK_DEAD) || sk->sk_socket;
> }
>
> +static inline void sctp_sock_set_nodelay(struct sock *sk)
> +{
> + lock_sock(sk);
> + sctp_sk(sk)->nodelay = true;
> + release_sock(sk);
> +}
> +
The duplication with sctp_setsockopt_nodelay() is quite silly/bad.
Also, why have the 'true' hardcoded? It's what dlm uses, yes, but the
API could be a bit more complete than that.
Like for the other patch, this one could build on David's patch, do
the ternary check before calling sctp_setsockopt_nodelay and then move
sctp_setsockopt_nodelay to the header, or something like that.
