[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0724c357-1d74-47b8-85cb-bf344eb4cc52@linux.alibaba.com>
Date: Thu, 4 Apr 2024 16:46:24 +0800
From: Wen Gu <guwen@...ux.alibaba.com>
To: Gerd Bayer <gbayer@...ux.ibm.com>, wintera@...ux.ibm.com,
twinkler@...ux.ibm.com, hca@...ux.ibm.com, gor@...ux.ibm.com,
agordeev@...ux.ibm.com, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, wenjia@...ux.ibm.com, jaka@...ux.ibm.com
Cc: borntraeger@...ux.ibm.com, svens@...ux.ibm.com,
alibuda@...ux.alibaba.com, tonylu@...ux.alibaba.com,
linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
netdev@...r.kernel.org
Subject: Re: [RFC PATCH net-next v5 02/11] net/smc: introduce loopback-ism for
SMC intra-OS shortcut
On 2024/4/3 19:27, Gerd Bayer wrote:
> On Sun, 2024-03-24 at 21:55 +0800, Wen Gu wrote:
>> Configuration of this feature is managed through the config SMC_LO.
>
> Hi Wen,
>
> you could omit building smc_loopback.o if CONFIG_SMC_LO was not set if
> you included the following fixup-patch. I think it's cleaner to put a
> few parts of net/smc/af_smc.c under conditional compile rather than
> have most of the contents of net/smc/smc_loopback.c under #ifdef.
>
Hi Gerd. That is a really good suggestion. It is much cleaner in this way.
I will improve it in next version. Thank you very much!
> From 11a9cfce550f0c4df10eafdd30aa4226d4d522a8 Mon Sep 17 00:00:00 2001
> From: Gerd Bayer <gbayer@...ux.ibm.com>
> Date: Wed, 3 Apr 2024 11:43:36 +0200
> Subject: [PATCH] fixup! net/smc: introduce loopback-ism for SMC intra-
> OS
> shortcut
>
> ---
> net/smc/Makefile | 5 +++--
> net/smc/af_smc.c | 6 ++++++
> net/smc/smc_loopback.c | 8 --------
> 3 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/net/smc/Makefile b/net/smc/Makefile
> index a8c37111abe1..3b73c9d561bb 100644
> --- a/net/smc/Makefile
> +++ b/net/smc/Makefile
> @@ -4,5 +4,6 @@ obj-$(CONFIG_SMC) += smc.o
> obj-$(CONFIG_SMC_DIAG) += smc_diag.o
> smc-y := af_smc.o smc_pnet.o smc_ib.o smc_clc.o smc_core.o smc_wr.o
> smc_llc.o
> smc-y += smc_cdc.o smc_tx.o smc_rx.o smc_close.o smc_ism.o
> smc_netlink.o smc_stats.o
> -smc-y += smc_tracepoint.o smc_loopback.o
> -smc-$(CONFIG_SYSCTL) += smc_sysctl.o
> +smc-y += smc_tracepoint.o
> +smc-$(CONFIG_SMC_LO) += smc_loopback.o
> +smc-$(CONFIG_SYSCTL) += smc_sysctl.o
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index fce7a5b2ce5c..bcbf600cd271 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -3574,11 +3574,13 @@ static int __init smc_init(void)
> goto out_sock;
> }
>
> +#if IS_ENABLED(CONFIG_SMC_LO)
> rc = smc_loopback_init();
> if (rc) {
> pr_err("%s: smc_loopback_init fails with %d\n",
> __func__, rc);
> goto out_ib;
> }
> +#endif
>
> rc = tcp_register_ulp(&smc_ulp_ops);
> if (rc) {
> @@ -3590,8 +3592,10 @@ static int __init smc_init(void)
> return 0;
>
> out_lo:
> +#if IS_ENABLED(CONFIG_SMC_LO)
> smc_loopback_exit();
> out_ib:
> +#endif
> smc_ib_unregister_client();
> out_sock:
> sock_unregister(PF_SMC);
> @@ -3628,7 +3632,9 @@ static void __exit smc_exit(void)
> tcp_unregister_ulp(&smc_ulp_ops);
> sock_unregister(PF_SMC);
> smc_core_exit();
> +#if IS_ENABLED(CONFIG_SMC_LO)
> smc_loopback_exit();
> +#endif
> smc_ib_unregister_client();
> smc_ism_exit();
> destroy_workqueue(smc_close_wq);
> diff --git a/net/smc/smc_loopback.c b/net/smc/smc_loopback.c
> index 994fe39930ad..8d0181635ded 100644
> --- a/net/smc/smc_loopback.c
> +++ b/net/smc/smc_loopback.c
> @@ -19,7 +19,6 @@
> #include "smc_ism.h"
> #include "smc_loopback.h"
>
> -#if IS_ENABLED(CONFIG_SMC_LO)
> #define SMC_LO_V2_CAPABLE 0x1 /* loopback-ism acts as ISMv2 */
> #define SMC_LO_SUPPORT_NOCOPY 0x1
> #define SMC_DMA_ADDR_INVALID (~(dma_addr_t)0)
> @@ -442,20 +441,13 @@ static void smc_lo_dev_remove(void)
> smc_lo_dev_exit(lo_dev);
> put_device(&lo_dev->dev); /* device_initialize in
> smc_lo_dev_probe */
> }
> -#endif
>
> int smc_loopback_init(void)
> {
> -#if IS_ENABLED(CONFIG_SMC_LO)
> return smc_lo_dev_probe();
> -#else
> - return 0;
> -#endif
> }
>
> void smc_loopback_exit(void)
> {
> -#if IS_ENABLED(CONFIG_SMC_LO)
> smc_lo_dev_remove();
> -#endif
> }
Powered by blists - more mailing lists