Re: [RFC net-next 10/17] net/dibs: Define dibs_client_ops and dibs_dev_ops

[Dust Li <dust.li@linux.alibaba.com>]

On 2025-08-06 17:41:15, Alexandra Winter wrote:
>Move the device add() and remove() functions from ism_client to
>dibs_client_ops and call add_dev()/del_dev() for ism devices and
>dibs_loopback devices. dibs_client_ops->add_dev() = smcd_register_dev() for
>the smc_dibs_client. This is the first step to handle ism and loopback
>devices alike (as dibs devices) in the smc dibs client.
>
>Define dibs_dev->ops and move smcd_ops->get_chid to
>dibs_dev_ops->get_fabric_id() for ism and loopback devices. See below for
>why this needs to be in the same patch as dibs_client_ops->add_dev().
>
>The following changes contain intermediate steps, that will be obsoleted by
>follow-on patches, once more functionality has been moved to dibs:
>
>Use different smcd_ops and max_dmbs for ism and loopback. Follow-on patches
>will change SMC-D to directly use dibs_ops instead of smcd_ops.
>
>In smcd_register_dev() it is now necessary to identify a dibs_loopback
>device before smcd_dev and smcd_ops->get_chid() are available. So provide
>dibs_dev_ops->get_fabric_id() in this patch and evaluate it in
>smc_ism_is_loopback().
>
>Call smc_loopback_init() in smcd_register_dev() and call
>smc_loopback_exit() in smcd_unregister_dev() to handle the functionality
>that is still in smc_loopback. Follow-on patches will move all smc_loopback
>code to dibs_loopback.
>
>In smcd_unregister_dev() use only ism device name, this will be replaced by
>dibs device name by a follow-on patch.
>
>End of changes with intermediate parts.
>
>Allocate an smcd event workqueue for all dibs devices, although
>dibs_loopback does not generate events.
>
>Use kernel memory instead of devres memory for smcd_dev and smcd->conn.
>Since commit a72178cfe855 ("net/smc: Fix dependency of SMC on ISM") an ism
>device and its driver can have a longer lifetime than the smc module, so
>smc should not rely on devres to free its resources [1]. It is now the
>responsibility of the smc client to free smcd and smcd->conn for all dibs
>devices, ism devices as well as loopback. Call client->ops->del_dev() for
>all existing dibs devices in dibs_unregister_client(), so all device
>related structures can be freed in the client.
>
>When dibs_unregister_client() is called in the context of smc_exit() or
>smc_core_reboot_event(), these functions have already called
>smc_lgrs_shutdown() which calls smc_smcd_terminate_all(smcd) and sets
>going_away. This is done a second time in smcd_unregister_dev(). This is
>analogous to how smcr is handled in these functions, by calling first
>smc_lgrs_shutdown() and then smc_ib_unregister_client() >
>smc_ib_remove_dev(), so leave it that way. It may be worth investigating,
>whether smc_lgrs_shutdown() is still required or useful.
>
>Remove CONFIG_SMC_LO. CONFIG_DIBS_LO now controls whether a dibs loopback
>device exists or not.
>
>Link: https://www.kernel.org/doc/Documentation/driver-model/devres.txt [1]
>Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>
>Reviewed-by: Mahanta Jambigi <mjambigi@linux.ibm.com>

Hi Winter,

I feel a bit hard for me to review the code especially with so many
intermediate parts. I may need more time to review these.

Seperate such a big refine patch is hard. Maybe put the
small parts in the front and the final one in the last to reduce
the intermediate part as much as possible ? I'm not sure.

Best regards,
Dust