From: Jakub Kicinski <kuba@kernel.org>
To: Jiri Pirko <jiri@resnulli.us>
Cc: leon@kernel.org, idosch@idosch.org, edwin.peer@broadcom.com,
netdev@vger.kernel.org
Subject: Re: [RFC 0/5] devlink: add an explicit locking API
Date: Wed, 3 Nov 2021 07:52:31 -0700 [thread overview]
Message-ID: <20211103075231.0a53330c@kicinski-fedora-PC1C0HJN> (raw)
In-Reply-To: <YYJQZIJPdy3WnQ1S@nanopsycho>
On Wed, 3 Nov 2021 10:03:32 +0100 Jiri Pirko wrote:
> 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.
> 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.
> 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.
> 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.
next prev parent reply other threads:[~2021-11-03 14:52 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-30 23:12 [RFC 0/5] devlink: add an explicit locking API Jakub Kicinski
2021-10-30 23:12 ` [RFC 1/5] devlink: add unlocked APIs Jakub Kicinski
2021-10-30 23:12 ` [RFC 2/5] devlink: add API for explicit locking Jakub Kicinski
2021-10-30 23:12 ` [RFC 3/5] devlink: allow locking of all ops Jakub Kicinski
2021-10-30 23:12 ` [RFC 4/5] netdevsim: minor code move Jakub Kicinski
2021-10-30 23:12 ` [RFC 5/5] netdevsim: use devlink locking Jakub Kicinski
2021-10-31 7:23 ` [RFC 0/5] devlink: add an explicit locking API Leon Romanovsky
2021-11-01 14:32 ` Jakub Kicinski
2021-11-01 18:36 ` Leon Romanovsky
2021-11-01 21:16 ` Jakub Kicinski
2021-11-02 8:08 ` Leon Romanovsky
2021-11-02 15:14 ` Jakub Kicinski
2021-11-02 18:14 ` Leon Romanovsky
2021-11-03 0:05 ` Jakub Kicinski
2021-11-03 7:23 ` Leon Romanovsky
2021-11-03 14:12 ` Jakub Kicinski
2021-11-01 20:04 ` Edwin Peer
2021-11-02 7:44 ` Leon Romanovsky
2021-11-02 15:16 ` Jakub Kicinski
2021-11-02 17:50 ` Leon Romanovsky
2021-11-03 9:03 ` Jiri Pirko
2021-11-03 14:52 ` Jakub Kicinski [this message]
2021-11-03 19:19 ` Jiri Pirko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211103075231.0a53330c@kicinski-fedora-PC1C0HJN \
--to=kuba@kernel.org \
--cc=edwin.peer@broadcom.com \
--cc=idosch@idosch.org \
--cc=jiri@resnulli.us \
--cc=leon@kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox