Re: [RFC 0/5] devlink: add an explicit locking API

From
Jiri Pirko <jiri@resnulli.us>
Date
2021-11-03 19:19:47
Reply to
Jakub Kicinski, 2021-11-03
Wed, Nov 03, 2021 at 03:52:31PM CET, kuba@kernel.org wrote:
On Wed, 3 Nov 2021 10:03:32 +0100 Jiri Pirko wrote:
quoted
Hi Jakub.

I took my time to read this thread and talked with Leon as well.
My original intention of locking in devlink was to maintain the locking
inside devlink.c to avoid the rtnl_lock-scenario.

However, I always feared that eventually we'll get to the point,
when it won't be possible to maintain any longer. I think may be it.
Indeed, the two things I think we can avoid from rtnl_lock pitfalls 
is that lock should be per-instance and that we should not wait for
all refs to be gone at unregister time.

Both are rather trivial to achieve with devlink.
quoted
In general, I like your approach. It is very clean and explicit. The
driver knows what to do, in which context it is and it can behave
accordingly. In theory or course, but the reality of drivers code tells
us often something different :)
Right. I'll convert a few more drivers but the real test will be
seeing if potential races are gone - hard to measure.
quoted
One small thing I don't fully undestand is the "opt-out" scenario which
makes things a bit tangled. But perhaps you can explain it a bit more.
Do you mean the .lock_flags? That's a transitional thing so that people
can convert drivers callback by callback to make prettier patch sets. 
I may collapse all those flags into one, remains to be seen how useful
it is when I create proper patches. This RFC is more of a code dump.

The whole opt-out is to create the entire new API at once, and then
convert drivers one-by-one (or allow the maintainers who care to do 
it themselves). I find that easier and more friendly than slicing 
the API and drivers multiple times.

Long story short I expect the opt-out would be gone by the time 5.17
merge window rolls around.
Okay, got it.

quoted
Leon claims that he thinks that he would be able to solve the locking
scheme leaving all locking internal to devlink.c. I suggest to give
him a week or 2 to present the solution. If he is not successful, lets
continue on your approach.

What do you think?
I do worry a little bit that our goals differ. Seems like Leon wants to
fix devlink for devlink and I want drivers to be able to lean on it.

But I'm not attached to the exact approach or code, so as long as
nobody is attached to theirs more RFCs can only help.

Please be courteous and send as RFCs, tho.
Makes sense. Thanks!
Thread (23 messages) 23 messages, 4 authors, last reply 2021-11-03
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help