lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <369a292c-c8c5-4002-a116-f9e1b4a436ba@linux.ibm.com>
Date: Thu, 14 Aug 2025 10:51:27 +0200
From: Alexandra Winter <wintera@...ux.ibm.com>
To: David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
        Andrew Lunn <andrew+netdev@...n.ch>,
        "D. Wythe" <alibuda@...ux.alibaba.com>,
        Dust Li <dust.li@...ux.alibaba.com>,
        Sidraya Jayagond
 <sidraya@...ux.ibm.com>,
        Wenjia Zhang <wenjia@...ux.ibm.com>,
        Julian Ruess <julianr@...ux.ibm.com>
Cc: netdev@...r.kernel.org, linux-s390@...r.kernel.org,
        Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Sven Schnelle <svens@...ux.ibm.com>,
        Thorsten Winkler <twinkler@...ux.ibm.com>,
        Simon Horman <horms@...nel.org>,
        Mahanta Jambigi <mjambigi@...ux.ibm.com>,
        Tony Lu
 <tonylu@...ux.alibaba.com>, Wen Gu <guwen@...ux.alibaba.com>,
        Halil Pasic <pasic@...ux.ibm.com>, linux-rdma@...r.kernel.org
Subject: Re: [RFC net-next 11/17] net/dibs: Move struct device to dibs_dev



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;


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ