On 3/27/23 5:43 PM, Jakub Kicinski wrote:

On Sat, 25 Mar 2023 21:07:22 -0700 Shannon Nelson wrote:

quoted

quoted

Don't put core devlink functionality in a separate file.
You're not wrapping all pci_* calls in your own wrappers, why are you
wrapping delvink? And use explicit locking, please. devl_* APIs.

Wrapping the devlink_register gives me the ability to abstract out the
bit of additional logic that gets added in a later patch, and now the
locking logic you mention, and is much like how other relatively current
drivers have done it, such as in ionic, ice, and mlx5.

What are you "abstracting away", exactly? Which "later patch"?
I'm not going to read the 5k LoC submission to figure out what
you're trying to say :(

I'm saying that more code is added in later patches around the 
devlink_register() for the health (patch 4) and parameters (patch 11). 
This allows me to have a simple line in the main probe logic that does 
the devlink-register related things, and then have the details collected 
together off to the side.

Obviously, when I update the code for using the devl_* interfaces and 
explicit locking, those two patches will change a little.

sln