On 06.08.25 17:41, Alexandra Winter wrote:
[...]
>
> Replace smcd->ops->get_dev(smcd) by dibs_get_dev().
>
Looking at the resulting code, I don't really like this concept of a *_get_dev() function,
that does not call get_device().
I plan to replace that by using dibs->dev directly in the next version.
See below for the code pieces I am referring to:
> diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c
> index 84a6e9ae2e64..0ddfd47a3a7c 100644
> --- a/drivers/s390/net/ism_drv.c
> +++ b/drivers/s390/net/ism_drv.c
[...]
> @@ -697,16 +684,14 @@ static int ism_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> ism_dev_exit(ism);
> err_dibs:
> /* pairs with dibs_dev_alloc() */
> - kfree(dibs);
> + put_device(dibs_get_dev(dibs));
[...]
> @@ -719,13 +704,12 @@ static void ism_remove(struct pci_dev *pdev)
> dibs_dev_del(dibs);
> ism_dev_exit(ism);
> /* pairs with dibs_dev_alloc() */
> - kfree(dibs);
> + put_device(dibs_get_dev(dibs));
>
> pci_release_mem_regions(pdev);
[...]
> @@ -871,13 +855,6 @@ static void smcd_get_local_gid(struct smcd_dev *smcd,
> smcd_gid->gid_ext = 0;
> }
>
> -static inline struct device *smcd_get_dev(struct smcd_dev *dev)
> -{
> - struct ism_dev *ism = dev->priv;
> -
> - return &ism->dev;
> -}
> -
> static const struct smcd_ops ism_smcd_ops = {
> .query_remote_gid = smcd_query_rgid,
> .register_dmb = smcd_register_dmb,
> @@ -890,7 +867,6 @@ static const struct smcd_ops ism_smcd_ops = {
> .move_data = smcd_move,
> .supports_v2 = smcd_supports_v2,
> .get_local_gid = smcd_get_local_gid,
> - .get_dev = smcd_get_dev,
> };
>
> const struct smcd_ops *ism_get_smcd_ops(void)
> diff --git a/include/linux/dibs.h b/include/linux/dibs.h
> index 805ab33271b5..4459b9369dc0 100644
> --- a/include/linux/dibs.h
> +++ b/include/linux/dibs.h
[...]
> @@ -158,6 +159,21 @@ static inline void *dibs_get_priv(struct dibs_dev *dev,
>
> /* ------- End of client-only functions ----------- */
>
> +/* Functions to be called by dibs clients and dibs device drivers:
> + */
> +/**
> + * dibs_get_dev()
> + * @dev: dibs device
> + * @token: dmb token of the remote dmb
> + *
> + * TODO: provide get and put functions
> + * Return: struct device* to be used for device refcounting
> + */
> +static inline struct device *dibs_get_dev(struct dibs_dev *dibs)
> +{
> + return &dibs->dev;
> +}
> +
[...]
> diff --git a/include/net/smc.h b/include/net/smc.h
> index e271891b85e6..05faac83371e 100644
> --- a/include/net/smc.h
> +++ b/include/net/smc.h
> @@ -63,7 +63,6 @@ struct smcd_ops {
> unsigned int size);
> int (*supports_v2)(void);
> void (*get_local_gid)(struct smcd_dev *dev, struct smcd_gid *gid);
> - struct device* (*get_dev)(struct smcd_dev *dev);
>
> /* optional operations */
> int (*add_vlan_id)(struct smcd_dev *dev, u64 vlan_id);
[...]
>
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index 67f9e0b83ebc..71c410dc3658 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -924,7 +924,7 @@ static int smc_lgr_create(struct smc_sock *smc, struct smc_init_info *ini)
> if (ini->is_smcd) {
> /* SMC-D specific settings */
> smcd = ini->ism_dev[ini->ism_selected];
> - get_device(smcd->ops->get_dev(smcd));
> + get_device(dibs_get_dev(smcd->dibs));
> lgr->peer_gid.gid =
> ini->ism_peer_gid[ini->ism_selected].gid;
> lgr->peer_gid.gid_ext =
> @@ -1474,7 +1474,7 @@ static void smc_lgr_free(struct smc_link_group *lgr)
> destroy_workqueue(lgr->tx_wq);
> if (lgr->is_smcd) {
> smc_ism_put_vlan(lgr->smcd, lgr->vlan_id);
> - put_device(lgr->smcd->ops->get_dev(lgr->smcd));
> + put_device(dibs_get_dev(lgr->smcd->dibs));
> }
> smc_lgr_put(lgr); /* theoretically last lgr_put */
> }
[...]
> diff --git a/net/smc/smc_loopback.c b/net/smc/smc_loopback.c
> index 37d8366419f7..262d0d0df4d0 100644
> --- a/net/smc/smc_loopback.c
> +++ b/net/smc/smc_loopback.c
> @@ -23,7 +23,6 @@
> #define SMC_LO_SUPPORT_NOCOPY 0x1
> #define SMC_DMA_ADDR_INVALID (~(dma_addr_t)0)
>
> -static const char smc_lo_dev_name[] = "loopback-ism";
> static struct smc_lo_dev *lo_dev;
>
> static void smc_lo_generate_ids(struct smc_lo_dev *ldev)
> @@ -255,11 +254,6 @@ static void smc_lo_get_local_gid(struct smcd_dev *smcd,
> smcd_gid->gid_ext = ldev->local_gid.gid_ext;
> }
>
> -static struct device *smc_lo_get_dev(struct smcd_dev *smcd)
> -{
> - return &((struct smc_lo_dev *)smcd->priv)->dev;
> -}
> -
> static const struct smcd_ops lo_ops = {
> .query_remote_gid = smc_lo_query_rgid,
> .register_dmb = smc_lo_register_dmb,
> @@ -274,7 +268,6 @@ static const struct smcd_ops lo_ops = {
> .signal_event = NULL,
> .move_data = smc_lo_move_data,
> .get_local_gid = smc_lo_get_local_gid,
> - .get_dev = smc_lo_get_dev,
> };
>
[...]
> diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
> index 76ad29e31d60..bbdd875731f2 100644
> --- a/net/smc/smc_pnet.c
> +++ b/net/smc/smc_pnet.c
> @@ -169,7 +169,7 @@ static int smc_pnet_remove_by_pnetid(struct net *net, char *pnet_name)
> pr_warn_ratelimited("smc: smcd device %s "
> "erased user defined pnetid "
> "%.16s\n",
> - dev_name(smcd->ops->get_dev(smcd)),
> + dev_name(dibs_get_dev(smcd->dibs)),
> smcd->pnetid);
> memset(smcd->pnetid, 0, SMC_MAX_PNETID_LEN);
> smcd->pnetid_by_user = false;
> @@ -332,7 +332,7 @@ static struct smcd_dev *smc_pnet_find_smcd(char *smcd_name)
>
> mutex_lock(&smcd_dev_list.mutex);
> list_for_each_entry(smcd_dev, &smcd_dev_list.list, list) {
> - if (!strncmp(dev_name(smcd_dev->ops->get_dev(smcd_dev)),
> + if (!strncmp(dev_name(dibs_get_dev(smcd_dev->dibs)),
> smcd_name, IB_DEVICE_NAME_MAX - 1))
> goto out;
> }
> @@ -431,7 +431,7 @@ static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
> if (smcd) {
> smcddev_applied = smc_pnet_apply_smcd(smcd, pnet_name);
> if (smcddev_applied) {
> - dev = smcd->ops->get_dev(smcd);
> + dev = dibs_get_dev(smcd->dibs);
> pr_warn_ratelimited("smc: smcd device %s "
> "applied user defined pnetid "
> "%.16s\n", dev_name(dev),
> @@ -1192,7 +1192,7 @@ int smc_pnetid_by_table_ib(struct smc_ib_device *smcibdev, u8 ib_port)
> */
> int smc_pnetid_by_table_smcd(struct smcd_dev *smcddev)
> {
> - const char *ib_name = dev_name(smcddev->ops->get_dev(smcddev));
> + const char *ib_name = dev_name(dibs_get_dev(smcddev->dibs));
> struct smc_pnettable *pnettable;
> struct smc_pnetentry *tmp_pe;
> struct smc_net *sn;
