From: David Miller <davem@davemloft.net>
To: panther@balabit.hu
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH 1/2] Remove unnecessary locks from rtnetlink
Date: Tue, 05 Feb 2008 17:53:37 -0800 (PST) [thread overview]
Message-ID: <20080205.175337.266368079.davem@davemloft.net> (raw)
In-Reply-To: <12018820542721-git-send-email-panther@balabit.hu>
From: Laszlo Attila Toth <panther@balabit.hu>
Date: Fri, 1 Feb 2008 17:07:33 +0100
> The do_setlink() function is protected by rtnl, additional locks are unnecessary.
> and the set_operstate() function is called from protected parts. Locks removed
> from both functions.
>
> The set_operstate() is also called from rtnl_create_link() and from no other places.
> In rtnl_create_link() none of the changes is protected by set_lock_bh() except
> inside set_operstate(), different locking scheme is not necessary
> for the operstate.
>
> Signed-off-by: Laszlo Attila Toth <panther@balabit.hu>
The protection using dev_base_lock() is needed.
When analyzing cases like this you need to also look at other code
paths outside of rtnetlink that access ->operstate and ->link_mode,
you obviously didn't do this.
For example, net/core/net-sysfs.c takes a read lock on dev_base_lock
in order to fetch a stable copy of both netif_running() and
dev->operstate at the same time.
Similar write locking to protect dev->operstate is made by
net/core/link_watch.c:rfc2863_policy(), for the same reason rtnetlink
has to make this locking.
You therefore cannot remove it.
This invalidates your second patch so I'm dropping that as well.
next prev parent reply other threads:[~2008-02-06 1:53 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-02-01 16:06 [PATCH 0/2] rtnetlink locking and notification fixes Laszlo Attila Toth
2008-02-01 16:07 ` [PATCH 1/2] Remove unnecessary locks from rtnetlink Laszlo Attila Toth
2008-02-06 1:53 ` David Miller [this message]
2008-02-07 11:57 ` [PATCH] rtnetlink.c: send a single notification on device state changes Laszlo Attila Toth
2008-02-13 6:42 ` David Miller
2008-02-07 12:00 ` [PATCH 1/2] Remove unnecessary locks from rtnetlink Laszlo Attila Toth
2008-02-01 16:07 ` [PATCH 2/2] rtnetlink: send a single notification on device state changes Laszlo Attila Toth
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=20080205.175337.266368079.davem@davemloft.net \
--to=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
--cc=panther@balabit.hu \
/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).