Subject: Re: [PATCH net-next 4/5] devlink: embed driver's priv data callback param into devlink_resource

From: Jakub Kicinski <[email-masked]>
Date: Thu, 8 Aug 2024 19:41:50 -0700

On Wed, 7 Aug 2024 08:49:57 +0200 Jiri Pirko wrote:
> > 	lockdep_assert_held(&devlink->lock);
> > 
> > 	resource = devlink_resource_find(devlink, NULL, resource_id);
> >-	if (WARN_ON(!resource))
> >+	if (WARN_ON(!resource || occ_priv_size > resource->priv_size))  
> 
> Very odd. You allocate a mem in devl_resource_register() and here you
> copy data to it. Why the void pointer is not enough for you? You can
> easily alloc struct in the driver and pass a pointer to it.
> 
> This is quite weird. Please don't.

The patch is a bit of a half measure, true.

Could you shed more light on the design choices for the resource API,
tho? Why the tying of objects by driver-defined IDs? It looks like 
the callback for getting resources occupancy is "added" later once 
the resource is registered? Is this some legacy of the old locking
scheme? It's quite unusual.