[PATCH net-next] devlink: unify devlink_shd_get_priv() into devlink_priv()

[PATCH net-next] devlink: unify devlink_shd_get_priv() into devlink_priv()

[Przemek Kitszel]

Unify access API to shared devlink priv data with normal devlink.

Thanks to Jiri Piro, we now have ability to create shared devlink
instances [1]. Introduction series have added usege of those for mlx, but
without priv data attached to the shared devlink.

Current API makes it possible to access shared devlink instance's priv
data:

	void *devlink_shd_get_priv(struct devlink *devlink);

but it is easy to to forget (especially during rebase from "before shared
devlinks" era) and call:

	void *devlink_priv(struct devlink *devlink);

which even has the same signature, so it's hard to catch the error.

New proposed API unifies both calls into one, without any increase in the
observerd struct size. (Alternative could be to store additional pointer,
set during devlink_alloc).

Unexport the less convenient API call.

[1] commit 411ad0605875 ("Merge branch 'devlink-introduce-shared-devlink-instance-for-pfs-on-same-chip'")
[1] https://lore.kernel.org/all/20260312100407.551173-1-jiri@resnulli.us

Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
sorry that I have not mentioned it during [1] review, it got obvious
for me only after working with the code
---
 net/devlink/devl_internal.h |  4 ++++
 net/devlink/core.c          |  3 +++
 net/devlink/sh_dev.c        | 11 ++++++++---
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index 7dfb7cdd2d23..abbe308ee9bd 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -58,6 +58,7 @@ struct devlink {
 	struct mutex lock;
 	struct lock_class_key lock_key;
 	u8 reload_failed:1;
+	u8 is_shd:1;
 	refcount_t refcount;
 	struct rcu_work rwork;
 	struct devlink_rel *rel;
@@ -72,6 +73,9 @@ struct devlink *__devlink_alloc(const struct devlink_ops *ops, size_t priv_size,
 				struct net *net, struct device *dev,
 				const struct device_driver *dev_driver);
 
+/* Get private data from shared devlink instance */
+void *devlink_shd_get_priv(struct devlink *devlink);
+
 #define devl_warn(devlink, format, args...)				\
 	do {								\
 		if ((devlink)->dev)					\
diff --git a/net/devlink/core.c b/net/devlink/core.c
index eeb6a71f5f56..4c97e2e2c073 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -232,6 +232,9 @@ int devlink_rel_devlink_handle_put(struct sk_buff *msg, struct devlink *devlink,
 
 void *devlink_priv(struct devlink *devlink)
 {
+	if (devlink->is_shd)
+		return devlink_shd_get_priv(devlink);
+
 	return &devlink->priv;
 }
 EXPORT_SYMBOL_GPL(devlink_priv);
diff --git a/net/devlink/sh_dev.c b/net/devlink/sh_dev.c
index 9b7456d64b57..450e3218759e 100644
--- a/net/devlink/sh_dev.c
+++ b/net/devlink/sh_dev.c
@@ -31,6 +31,11 @@ static struct devlink_shd *devlink_shd_lookup(const char *id)
 	return NULL;
 }
 
+static struct devlink_shd *devlink_to_shd(struct devlink *devlink)
+{
+	return &devlink->priv;
+}
+
 static struct devlink_shd *devlink_shd_create(const char *id,
 					      const struct devlink_ops *ops,
 					      size_t priv_size,
@@ -43,13 +48,14 @@ static struct devlink_shd *devlink_shd_create(const char *id,
 				  &init_net, NULL, driver);
 	if (!devlink)
 		return NULL;
-	shd = devlink_priv(devlink);
+	shd = devlink_to_shd(devlink);
 
 	shd->id = kstrdup(id, GFP_KERNEL);
 	if (!shd->id)
 		goto err_devlink_free;
 	shd->priv_size = priv_size;
 	refcount_set(&shd->refcount, 1);
+	devlink->is_shd = 1;
 
 	devl_lock(devlink);
 	devl_register(devlink);
@@ -137,7 +143,7 @@ void devlink_shd_put(struct devlink *devlink)
 	struct devlink_shd *shd;
 
 	mutex_lock(&shd_mutex);
-	shd = devlink_priv(devlink);
+	shd = devlink_to_shd(devlink);
 	if (refcount_dec_and_test(&shd->refcount))
 		devlink_shd_destroy(shd);
 	mutex_unlock(&shd_mutex);
@@ -159,4 +165,3 @@ void *devlink_shd_get_priv(struct devlink *devlink)
 
 	return shd->priv;
 }
-EXPORT_SYMBOL_GPL(devlink_shd_get_priv);
-- 
2.39.3

RE: [Intel-wired-lan] [PATCH net-next] devlink: unify devlink_shd_get_priv() into devlink_priv()

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Przemek Kitszel
> Sent: Monday, March 23, 2026 2:22 PM
> To: Jiri Pirko <jiri@resnulli.us>; netdev@vger.kernel.org; Jakub
> Kicinski <kuba@kernel.org>
> Cc: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Eric Dumazet
> <edumazet@google.com>; intel-wired-lan@lists.osuosl.org; Simon Horman
> <horms@kernel.org>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>;
> Paolo Abeni <pabeni@redhat.com>; David S. Miller <davem@davemloft.net>
> Subject: [Intel-wired-lan] [PATCH net-next] devlink: unify
> devlink_shd_get_priv() into devlink_priv()
> 
> Unify access API to shared devlink priv data with normal devlink.
> 
> Thanks to Jiri Piro, we now have ability to create shared devlink
Piro -> Pirko


> instances [1]. Introduction series have added usege of those for mlx,
usege -> usage


> but without priv data attached to the shared devlink.
> 
> Current API makes it possible to access shared devlink instance's priv
> data:
> 
> 	void *devlink_shd_get_priv(struct devlink *devlink);
> 
> but it is easy to to forget (especially during rebase from "before
easy to to forget -> easy to forget


> shared devlinks" era) and call:
> 
> 	void *devlink_priv(struct devlink *devlink);
> 
> which even has the same signature, so it's hard to catch the error.
> 
> New proposed API unifies both calls into one, without any increase in
> the observerd struct size. (Alternative could be to store additional
observerd -> observed


> pointer, set during devlink_alloc).
> 
> Unexport the less convenient API call.
> 
> [1] commit 411ad0605875 ("Merge branch 'devlink-introduce-shared-
> devlink-instance-for-pfs-on-same-chip'")
> [1] https://lore.kernel.org/all/20260312100407.551173-1-
> jiri@resnulli.us
> 
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
> sorry that I have not mentioned it during [1] review, it got obvious
> for me only after working with the code
> ---
>  net/devlink/devl_internal.h |  4 ++++
>  net/devlink/core.c          |  3 +++
>  net/devlink/sh_dev.c        | 11 ++++++++---
>  3 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
> index 7dfb7cdd2d23..abbe308ee9bd 100644
> --- a/net/devlink/devl_internal.h
> +++ b/net/devlink/devl_internal.h
> @@ -58,6 +58,7 @@ struct devlink {
>  	struct mutex lock;
>  	struct lock_class_key lock_key;
>  	u8 reload_failed:1;
> +	u8 is_shd:1;
>  	refcount_t refcount;
>  	struct rcu_work rwork;
>  	struct devlink_rel *rel;
> @@ -72,6 +73,9 @@ struct devlink *__devlink_alloc(const struct
> devlink_ops *ops, size_t priv_size,
>  				struct net *net, struct device *dev,
>  				const struct device_driver *dev_driver);
> 
> +/* Get private data from shared devlink instance */ void
> +*devlink_shd_get_priv(struct devlink *devlink);
> +
>  #define devl_warn(devlink, format, args...)				\
>  	do {								\
>  		if ((devlink)->dev)					\
> diff --git a/net/devlink/core.c b/net/devlink/core.c index
> eeb6a71f5f56..4c97e2e2c073 100644
> --- a/net/devlink/core.c
> +++ b/net/devlink/core.c
> @@ -232,6 +232,9 @@ int devlink_rel_devlink_handle_put(struct sk_buff
> *msg, struct devlink *devlink,
> 
>  void *devlink_priv(struct devlink *devlink)  {
> +	if (devlink->is_shd)
> +		return devlink_shd_get_priv(devlink);
I'm afraid it can cause recursion, if you didn't modify the function:
void *devlink_shd_get_priv(struct devlink *devlink)
{
    struct devlink_shd *shd = devlink_priv(devlink);

> +
>  	return &devlink->priv;
>  }

...

>  	if (refcount_dec_and_test(&shd->refcount))
>  		devlink_shd_destroy(shd);
>  	mutex_unlock(&shd_mutex);
> @@ -159,4 +165,3 @@ void *devlink_shd_get_priv(struct devlink
> *devlink)
> 
>  	return shd->priv;
>  }
> -EXPORT_SYMBOL_GPL(devlink_shd_get_priv);
> --
> 2.39.3

Re: [PATCH net-next] devlink: unify devlink_shd_get_priv() into devlink_priv()

[Jiri Pirko]

Mon, Mar 23, 2026 at 02:21:36PM +0100, przemyslaw.kitszel@intel.com wrote:
>Unify access API to shared devlink priv data with normal devlink.
>
>Thanks to Jiri Piro, we now have ability to create shared devlink
>instances [1]. Introduction series have added usege of those for mlx, but
>without priv data attached to the shared devlink.
>
>Current API makes it possible to access shared devlink instance's priv
>data:
>
>	void *devlink_shd_get_priv(struct devlink *devlink);
>
>but it is easy to to forget (especially during rebase from "before shared
>devlinks" era) and call:
>
>	void *devlink_priv(struct devlink *devlink);
>
>which even has the same signature, so it's hard to catch the error.
>
>New proposed API unifies both calls into one, without any increase in the
>observerd struct size. (Alternative could be to store additional pointer,
>set during devlink_alloc).

You should use some AI corrector :) too many typos.

>
>Unexport the less convenient API call.
>
>[1] commit 411ad0605875 ("Merge branch 'devlink-introduce-shared-devlink-instance-for-pfs-on-same-chip'")
>[1] https://lore.kernel.org/all/20260312100407.551173-1-jiri@resnulli.us
>
>Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>---
>sorry that I have not mentioned it during [1] review, it got obvious
>for me only after working with the code
>---
> net/devlink/devl_internal.h |  4 ++++
> net/devlink/core.c          |  3 +++
> net/devlink/sh_dev.c        | 11 ++++++++---
> 3 files changed, 15 insertions(+), 3 deletions(-)
>
>diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
>index 7dfb7cdd2d23..abbe308ee9bd 100644
>--- a/net/devlink/devl_internal.h
>+++ b/net/devlink/devl_internal.h
>@@ -58,6 +58,7 @@ struct devlink {
> 	struct mutex lock;
> 	struct lock_class_key lock_key;
> 	u8 reload_failed:1;
>+	u8 is_shd:1;
> 	refcount_t refcount;
> 	struct rcu_work rwork;
> 	struct devlink_rel *rel;
>@@ -72,6 +73,9 @@ struct devlink *__devlink_alloc(const struct devlink_ops *ops, size_t priv_size,
> 				struct net *net, struct device *dev,
> 				const struct device_driver *dev_driver);
> 
>+/* Get private data from shared devlink instance */
>+void *devlink_shd_get_priv(struct devlink *devlink);
>+
> #define devl_warn(devlink, format, args...)				\
> 	do {								\
> 		if ((devlink)->dev)					\
>diff --git a/net/devlink/core.c b/net/devlink/core.c
>index eeb6a71f5f56..4c97e2e2c073 100644
>--- a/net/devlink/core.c
>+++ b/net/devlink/core.c
>@@ -232,6 +232,9 @@ int devlink_rel_devlink_handle_put(struct sk_buff *msg, struct devlink *devlink,
> 
> void *devlink_priv(struct devlink *devlink)
> {
>+	if (devlink->is_shd)
>+		return devlink_shd_get_priv(devlink);
>+
> 	return &devlink->priv;
> }
> EXPORT_SYMBOL_GPL(devlink_priv);
>diff --git a/net/devlink/sh_dev.c b/net/devlink/sh_dev.c
>index 9b7456d64b57..450e3218759e 100644
>--- a/net/devlink/sh_dev.c
>+++ b/net/devlink/sh_dev.c
>@@ -31,6 +31,11 @@ static struct devlink_shd *devlink_shd_lookup(const char *id)
> 	return NULL;
> }
> 
>+static struct devlink_shd *devlink_to_shd(struct devlink *devlink)
>+{
>+	return &devlink->priv;
>+}

Can you rather have unexported __devlink_priv() above existing
devlink_priv()?


>+
> static struct devlink_shd *devlink_shd_create(const char *id,
> 					      const struct devlink_ops *ops,
> 					      size_t priv_size,
>@@ -43,13 +48,14 @@ static struct devlink_shd *devlink_shd_create(const char *id,
> 				  &init_net, NULL, driver);
> 	if (!devlink)
> 		return NULL;
>-	shd = devlink_priv(devlink);
>+	shd = devlink_to_shd(devlink);
> 
> 	shd->id = kstrdup(id, GFP_KERNEL);
> 	if (!shd->id)
> 		goto err_devlink_free;
> 	shd->priv_size = priv_size;
> 	refcount_set(&shd->refcount, 1);
>+	devlink->is_shd = 1;
> 
> 	devl_lock(devlink);
> 	devl_register(devlink);
>@@ -137,7 +143,7 @@ void devlink_shd_put(struct devlink *devlink)
> 	struct devlink_shd *shd;
> 
> 	mutex_lock(&shd_mutex);
>-	shd = devlink_priv(devlink);
>+	shd = devlink_to_shd(devlink);
> 	if (refcount_dec_and_test(&shd->refcount))
> 		devlink_shd_destroy(shd);
> 	mutex_unlock(&shd_mutex);
>@@ -159,4 +165,3 @@ void *devlink_shd_get_priv(struct devlink *devlink)
> 
> 	return shd->priv;
> }
>-EXPORT_SYMBOL_GPL(devlink_shd_get_priv);
>-- 
>2.39.3
>

Re: [PATCH net-next] devlink: unify devlink_shd_get_priv() into devlink_priv()

[Przemek Kitszel]

On 3/23/26 16:03, Jiri Pirko wrote:
> Mon, Mar 23, 2026 at 02:21:36PM +0100, przemyslaw.kitszel@intel.com wrote:
>> Unify access API to shared devlink priv data with normal devlink.
>>
>> Thanks to Jiri Piro, we now have ability to create shared devlink
>> instances [1]. Introduction series have added usege of those for mlx, but
>> without priv data attached to the shared devlink.
>>
>> Current API makes it possible to access shared devlink instance's priv
>> data:
>>
>> 	void *devlink_shd_get_priv(struct devlink *devlink);
>>
>> but it is easy to to forget (especially during rebase from "before shared
>> devlinks" era) and call:
>>
>> 	void *devlink_priv(struct devlink *devlink);
>>
>> which even has the same signature, so it's hard to catch the error.
>>
>> New proposed API unifies both calls into one, without any increase in the
>> observerd struct size. (Alternative could be to store additional pointer,
>> set during devlink_alloc).
> 
> You should use some AI corrector :) too many typos.

faux pas instead of faux bus, apologies for the most shameful of typos
("new machine"-kind of issue)

and thank you for looking

I have one more trivial related patch, will append to v2 tommorow

> 
>>
>> Unexport the less convenient API call.
>>
>> [1] commit 411ad0605875 ("Merge branch 'devlink-introduce-shared-devlink-instance-for-pfs-on-same-chip'")
>> [1] https://lore.kernel.org/all/20260312100407.551173-1-jiri@resnulli.us
>>
>> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>> ---
>> sorry that I have not mentioned it during [1] review, it got obvious
>> for me only after working with the code
>> ---
>> net/devlink/devl_internal.h |  4 ++++
>> net/devlink/core.c          |  3 +++
>> net/devlink/sh_dev.c        | 11 ++++++++---
>> 3 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
>> index 7dfb7cdd2d23..abbe308ee9bd 100644
>> --- a/net/devlink/devl_internal.h
>> +++ b/net/devlink/devl_internal.h
>> @@ -58,6 +58,7 @@ struct devlink {
>> 	struct mutex lock;
>> 	struct lock_class_key lock_key;
>> 	u8 reload_failed:1;
>> +	u8 is_shd:1;
>> 	refcount_t refcount;
>> 	struct rcu_work rwork;
>> 	struct devlink_rel *rel;
>> @@ -72,6 +73,9 @@ struct devlink *__devlink_alloc(const struct devlink_ops *ops, size_t priv_size,
>> 				struct net *net, struct device *dev,
>> 				const struct device_driver *dev_driver);
>>
>> +/* Get private data from shared devlink instance */
>> +void *devlink_shd_get_priv(struct devlink *devlink);
>> +
>> #define devl_warn(devlink, format, args...)				\
>> 	do {								\
>> 		if ((devlink)->dev)					\
>> diff --git a/net/devlink/core.c b/net/devlink/core.c
>> index eeb6a71f5f56..4c97e2e2c073 100644
>> --- a/net/devlink/core.c
>> +++ b/net/devlink/core.c
>> @@ -232,6 +232,9 @@ int devlink_rel_devlink_handle_put(struct sk_buff *msg, struct devlink *devlink,
>>
>> void *devlink_priv(struct devlink *devlink)
>> {
>> +	if (devlink->is_shd)
>> +		return devlink_shd_get_priv(devlink);
>> +
>> 	return &devlink->priv;
>> }
>> EXPORT_SYMBOL_GPL(devlink_priv);
>> diff --git a/net/devlink/sh_dev.c b/net/devlink/sh_dev.c
>> index 9b7456d64b57..450e3218759e 100644
>> --- a/net/devlink/sh_dev.c
>> +++ b/net/devlink/sh_dev.c
>> @@ -31,6 +31,11 @@ static struct devlink_shd *devlink_shd_lookup(const char *id)
>> 	return NULL;
>> }
>>
>> +static struct devlink_shd *devlink_to_shd(struct devlink *devlink)
>> +{
>> +	return &devlink->priv;
>> +}
> 
> Can you rather have unexported __devlink_priv() above existing
> devlink_priv()?

makes sense, will do

(this will not remove the need of devlink_shd_get_priv() in sh_dev.c,
as the struct devlink_shd is defined here)

> 
> 
>> +
>> static struct devlink_shd *devlink_shd_create(const char *id,
>> 					      const struct devlink_ops *ops,
>> 					      size_t priv_size,
>> @@ -43,13 +48,14 @@ static struct devlink_shd *devlink_shd_create(const char *id,
>> 				  &init_net, NULL, driver);
>> 	if (!devlink)
>> 		return NULL;
>> -	shd = devlink_priv(devlink);
>> +	shd = devlink_to_shd(devlink);
>>
>> 	shd->id = kstrdup(id, GFP_KERNEL);
>> 	if (!shd->id)
>> 		goto err_devlink_free;
>> 	shd->priv_size = priv_size;
>> 	refcount_set(&shd->refcount, 1);
>> +	devlink->is_shd = 1;
>>
>> 	devl_lock(devlink);
>> 	devl_register(devlink);
>> @@ -137,7 +143,7 @@ void devlink_shd_put(struct devlink *devlink)
>> 	struct devlink_shd *shd;
>>
>> 	mutex_lock(&shd_mutex);
>> -	shd = devlink_priv(devlink);
>> +	shd = devlink_to_shd(devlink);
>> 	if (refcount_dec_and_test(&shd->refcount))
>> 		devlink_shd_destroy(shd);
>> 	mutex_unlock(&shd_mutex);
>> @@ -159,4 +165,3 @@ void *devlink_shd_get_priv(struct devlink *devlink)
>>
>> 	return shd->priv;
>> }
>> -EXPORT_SYMBOL_GPL(devlink_shd_get_priv);
>> -- 
>> 2.39.3
>>

Re: [Intel-wired-lan] [PATCH net-next] devlink: unify devlink_shd_get_priv() into devlink_priv()

[Przemek Kitszel]

>>   void *devlink_priv(struct devlink *devlink)  {
>> +	if (devlink->is_shd)
>> +		return devlink_shd_get_priv(devlink);
> I'm afraid it can cause recursion, if you didn't modify the function:

right, thanks
I should have really send this as an RFC :/

> void *devlink_shd_get_priv(struct devlink *devlink)
> {
>      struct devlink_shd *shd = devlink_priv(devlink);
> 
>> +
>>   	return &devlink->priv;
>>   }
> 
> ...
> 
>>   	if (refcount_dec_and_test(&shd->refcount))
>>   		devlink_shd_destroy(shd);
>>   	mutex_unlock(&shd_mutex);
>> @@ -159,4 +165,3 @@ void *devlink_shd_get_priv(struct devlink
>> *devlink)
>>
>>   	return shd->priv;
>>   }
>> -EXPORT_SYMBOL_GPL(devlink_shd_get_priv);
>> --
>> 2.39.3
> 

Re: [Intel-wired-lan] [PATCH net-next] devlink: unify devlink_shd_get_priv() into devlink_priv()

[kernel test robot]

Hi Przemek,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Przemek-Kitszel/devlink-unify-devlink_shd_get_priv-into-devlink_priv/20260325-130848
base:   net-next/main
patch link:    https://lore.kernel.org/r/20260323132136.13191-1-przemyslaw.kitszel%40intel.com
patch subject: [Intel-wired-lan] [PATCH net-next] devlink: unify devlink_shd_get_priv() into devlink_priv()
config: x86_64-rhel-9.4-ltp (https://download.01.org/0day-ci/archive/20260325/202603251148.Sjx5pVxc-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260325/202603251148.Sjx5pVxc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603251148.Sjx5pVxc-lkp@intel.com/

All errors (new ones prefixed by >>):

   net/devlink/sh_dev.c: In function 'devlink_to_shd':
>> net/devlink/sh_dev.c:36:16: error: returning 'char (*)[]' from a function with incompatible return type 'struct devlink_shd *' [-Wincompatible-pointer-types]
      36 |         return &devlink->priv;
         |                ^~~~~~~~~~~~~~


vim +36 net/devlink/sh_dev.c

    33	
    34	static struct devlink_shd *devlink_to_shd(struct devlink *devlink)
    35	{
  > 36		return &devlink->priv;
    37	}
    38	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki