Re: [PATCH net-next 1/2] net/smc: refatoring initialization of smc sock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 5/11/24 8:21 PM, Zhu Yanjun wrote:
在 2024/5/10 6:12, D. Wythe 写道:
From: "D. Wythe" <alibuda@xxxxxxxxxxxxxxxxx>

This patch aims to isolate the shared components of SMC socket
allocation by introducing smc_sock_init() for sock initialization
and __smc_create_clcsk() for the initialization of clcsock.

This is in preparation for the subsequent implementation of the
AF_INET version of SMC.

Signed-off-by: D. Wythe <alibuda@xxxxxxxxxxxxxxxxx>
---
  net/smc/af_smc.c | 93 +++++++++++++++++++++++++++++++-------------------------
  1 file changed, 52 insertions(+), 41 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 9389f0c..1f03724 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -361,34 +361,43 @@ static void smc_destruct(struct sock *sk)
          return;
  }
  -static struct sock *smc_sock_alloc(struct net *net, struct socket *sock,
-                   int protocol)
+static void smc_sock_init(struct net *net, struct sock *sk, int protocol)
  {
-    struct smc_sock *smc;
-    struct proto *prot;
-    struct sock *sk;
-
-    prot = (protocol == SMCPROTO_SMC6) ? &smc_proto6 : &smc_proto;
-    sk = sk_alloc(net, PF_SMC, GFP_KERNEL, prot, 0);
-    if (!sk)
-        return NULL;
+    struct smc_sock *smc = smc_sk(sk);
  -    sock_init_data(sock, sk); /* sets sk_refcnt to 1 */
      sk->sk_state = SMC_INIT;
-    sk->sk_destruct = smc_destruct;
      sk->sk_protocol = protocol;
+    mutex_init(&smc->clcsock_release_lock);

Please add mutex_destroy(&smc->clcsock_release_lock); when smc->clcsock_release_lock is no longer used.

Or else some tools will notify errors.

Zhu Yanjun


It seems that the problem you mentioned is not caused by this patch, after all, this patch is solely for refactoring. Adding the fix you mentioned in this refactoring patch would not be appropriate. Perhaps, you could submit a separate
patch to address the issue. What do you think?

D. Wythe


      WRITE_ONCE(sk->sk_sndbuf, 2 * READ_ONCE(net->smc.sysctl_wmem));
      WRITE_ONCE(sk->sk_rcvbuf, 2 * READ_ONCE(net->smc.sysctl_rmem));
-    smc = smc_sk(sk);
      INIT_WORK(&smc->tcp_listen_work, smc_tcp_listen_work);
      INIT_WORK(&smc->connect_work, smc_connect_work);
      INIT_DELAYED_WORK(&smc->conn.tx_work, smc_tx_work);
      INIT_LIST_HEAD(&smc->accept_q);
      spin_lock_init(&smc->accept_q_lock);
      spin_lock_init(&smc->conn.send_lock);
-    sk->sk_prot->hash(sk);
-    mutex_init(&smc->clcsock_release_lock);
      smc_init_saved_callbacks(smc);
+    smc->limit_smc_hs = net->smc.limit_smc_hs;
+    smc->use_fallback = false; /* assume rdma capability first */
+    smc->fallback_rsn = 0;
+
+    sk->sk_destruct = smc_destruct;
+    sk->sk_prot->hash(sk);
+}
+
+static struct sock *smc_sock_alloc(struct net *net, struct socket *sock,
+                   int protocol)
+{
+    struct proto *prot;
+    struct sock *sk;
+
+    prot = (protocol == SMCPROTO_SMC6) ? &smc_proto6 : &smc_proto;
+    sk = sk_alloc(net, PF_SMC, GFP_KERNEL, prot, 0);
+    if (!sk)
+        return NULL;
+
+    sock_init_data(sock, sk); /* sets sk_refcnt to 1 */
+    smc_sock_init(net, sk, protocol);
        return sk;
  }
@@ -3321,6 +3330,31 @@ static ssize_t smc_splice_read(struct socket *sock, loff_t *ppos,
      .splice_read    = smc_splice_read,
  };
  +static int __smc_create_clcsk(struct net *net, struct sock *sk, int family)
+{
+    struct smc_sock *smc = smc_sk(sk);
+    int rc;
+
+    rc = sock_create_kern(net, family, SOCK_STREAM, IPPROTO_TCP,
+                  &smc->clcsock);
+    if (rc) {
+        sk_common_release(sk);
+        return rc;
+    }
+
+    /* smc_clcsock_release() does not wait smc->clcsock->sk's
+     * destruction;  its sk_state might not be TCP_CLOSE after
+     * smc->sk is close()d, and TCP timers can be fired later,
+     * which need net ref.
+     */
+    sk = smc->clcsock->sk;
+    __netns_tracker_free(net, &sk->ns_tracker, false);
+    sk->sk_net_refcnt = 1;
+    get_net_track(net, &sk->ns_tracker, GFP_KERNEL);
+    sock_inuse_add(net, 1);
+    return 0;
+}
+
  static int __smc_create(struct net *net, struct socket *sock, int protocol,
              int kern, struct socket *clcsock)
  {
@@ -3346,35 +3380,12 @@ static int __smc_create(struct net *net, struct socket *sock, int protocol,
        /* create internal TCP socket for CLC handshake and fallback */
      smc = smc_sk(sk);
-    smc->use_fallback = false; /* assume rdma capability first */
-    smc->fallback_rsn = 0;
-
-    /* default behavior from limit_smc_hs in every net namespace */
-    smc->limit_smc_hs = net->smc.limit_smc_hs;
        rc = 0;
-    if (!clcsock) {
-        rc = sock_create_kern(net, family, SOCK_STREAM, IPPROTO_TCP,
-                      &smc->clcsock);
-        if (rc) {
-            sk_common_release(sk);
-            goto out;
-        }
-
-        /* smc_clcsock_release() does not wait smc->clcsock->sk's
-         * destruction;  its sk_state might not be TCP_CLOSE after
-         * smc->sk is close()d, and TCP timers can be fired later,
-         * which need net ref.
-         */
-        sk = smc->clcsock->sk;
-        __netns_tracker_free(net, &sk->ns_tracker, false);
-        sk->sk_net_refcnt = 1;
-        get_net_track(net, &sk->ns_tracker, GFP_KERNEL);
-        sock_inuse_add(net, 1);
-    } else {
+    if (!clcsock)
+        rc = __smc_create_clcsk(net, sk, family);
+    else
          smc->clcsock = clcsock;
-    }
-
  out:
      return rc;
  }





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux