On 3/28/23 3:17 PM, Jakub Kicinski wrote:

On Mon, 27 Mar 2023 23:19:28 -0700 Shannon Nelson wrote:

quoted

quoted

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.

It's not supposed to be off to the side, that's my point.
It's a central interface for device control.

quoted

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

The devlink alloc and registration are obviously a part of the probe and 
thus device control setup, so I’m not sure why this is an issue.

As is suggested in coding style, the smaller functions make for easier 
reading, and keeps the related locking in a nice little package.  Having 
the devlink registration code gathered in one place in the devlink.c 
file seems to follow most conventions, which then allows the helper 
functions to be static to that file.  This seems to be what about half 
the drivers that use devlink have chosen to do.

Sure, I could move that function into main.c and make the helper 
functions more public if that is what you’re looking for.  This seems to 
be the choice for a few of the other drivers.

Or are you looking to have all of the devlink.c code get rolled into main.c?

sln