On Tue, 2 Oct 2007, Herbert Xu wrote:

On Sun, Sep 30, 2007 at 05:47:30PM +0200, Patrick McHardy wrote:

I'm a bit uncomfortable with having a function (change_flags)
that's sometimes in a sleepable context and sometimes running
with BH disabled.

So how about splitting up the unicast/multicast entry points
as follows?


[NET]: Restore multicast-only __dev_mc_upload

As it is dev_set_rx_mode needs to cope with being called with
or without the RTNL.  This is rather confusing when it comes
to understanding what locks are being held at a particular
point.

Since the only path that calls it without the RTNL is in the
multicast code, we could avoid the confusion by splitting that
out.

This patch restores the old __dev_mc_upload as a multicast-only
variant of dev_set_rx_mode.  It also gets rid of the now-unused
__dev_set_rx_mod and makes dev_set_rx_mode static since it's
only used in net/core/dev.c.

As a side-effect this fixes the warning triggered by the
RTNL_ASSERT in __dev_set_promiscuity.


I think this doesn't completely fix it, when dev_unicast_add is
interrupted by dev_mc_add before the unicast changes are performed,
they will get committed in the dev_mc_add context, so we might still
call change_flags with BH disabled. Taking the TX lock around the
dev->uc_count and dev->uc_promisc checks and changes in __dev_set_rx_mode
should fix this.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to