From: Jakub Kicinski <kuba@kernel.org>
To: Kuniyuki Iwashima <kuniyu@amazon.com>
Cc: <davem@davemloft.net>, <dsahern@gmail.com>, <edumazet@google.com>,
<jiri@resnulli.us>, <netdev@vger.kernel.org>, <pabeni@redhat.com>
Subject: Re: [PATCH net-next 1/2] rtnetlink: move rtnl_lock handling out of af_netlink
Date: Thu, 6 Jun 2024 17:04:53 -0700 [thread overview]
Message-ID: <20240606170453.53f20d5b@kernel.org> (raw)
In-Reply-To: <20240606233303.37245-1-kuniyu@amazon.com>
On Thu, 6 Jun 2024 16:33:03 -0700 Kuniyuki Iwashima wrote:
> > + if (needs_lock)
> > + rtnl_lock();
> > err = dumpit(skb, cb);
> > + if (needs_lock)
> > + rtnl_unlock();
>
> This calls netdev_run_todo() now, is this change intended ?
Nice catch / careful thinking, indeed we're moving from pure unlock to
run_todo. I don't really recall if I thought of this when writing the
change (it was few days back). My guess is that the fact we weren't
calling full rtnl_unlock() was unintentional / out of laziness in the
first place. It didn't matter since dumps are unlikely to changes /
unregister / free things. But still, someone may get caught off guard
as some point that we're holding rtnl but won't go via the usual unlock
path.
Would you like me to add a note to the commit message?
next prev parent reply other threads:[~2024-06-07 0:04 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-06 19:29 [PATCH net-next 0/2] rtnetlink: move rtnl_lock handling out of af_netlink Jakub Kicinski
2024-06-06 19:29 ` [PATCH net-next 1/2] " Jakub Kicinski
2024-06-06 23:33 ` Kuniyuki Iwashima
2024-06-07 0:04 ` Jakub Kicinski [this message]
2024-06-07 0:18 ` Kuniyuki Iwashima
2024-06-07 9:24 ` Eric Dumazet
2024-06-06 19:29 ` [PATCH net-next 2/2] net: netlink: remove the cb_mutex "injection" from netlink core Jakub Kicinski
2024-06-06 23:35 ` Kuniyuki Iwashima
2024-06-06 22:31 ` [PATCH net-next 0/2] rtnetlink: move rtnl_lock handling out of af_netlink David Ahern
2024-06-10 12:20 ` patchwork-bot+netdevbpf
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=20240606170453.53f20d5b@kernel.org \
--to=kuba@kernel.org \
--cc=davem@davemloft.net \
--cc=dsahern@gmail.com \
--cc=edumazet@google.com \
--cc=jiri@resnulli.us \
--cc=kuniyu@amazon.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/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;
as well as URLs for NNTP newsgroup(s).