netdev - Re: [Bugme-new] [Bug 7974] New: BUG: scheduling while atomic: swapper/0x10000100/0

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 13 Feb 2007 15:33:00 -0800
From:	Jay Vosburgh <fubar@...ibm.com>
To:	Andy Gospodarek <andy@...yhouse.net>
cc:	David Miller <davem@...emloft.net>, akpm@...ux-foundation.org,
	netdev@...r.kernel.org, shemminger@...ux-foundation.org,
	lpiccilli@...re.com.br, bugme-daemon@...zilla.kernel.org
Subject: Re: [Bugme-new] [Bug 7974] New: BUG: scheduling while atomic: swapper/0x10000100/0 

Andy Gospodarek <andy@...yhouse.net> wrote:

>On Tue, Feb 13, 2007 at 02:32:43PM -0800, David Miller wrote:
[...]
>> Maybe if you put the RTNL acquisition deeper into the call
>> path, ie. down into the code that knows RTNL is needed,
>> perhaps it won't be so ugly.  Replace the conditions with
>> functions.
>
>That is almost exactly what I am working on right now.  I'm trying to
>determine where the best place to put this would be so reduce the
>chance that I'd be using conditional locking.  

	It's complicated to do this because the small number of places
that need rtnl are way down at the bottom of the chain, and the top of
the chain can be entered either with or without rtnl, and not knowing if
we'll actually end up doing the "need rtnl" bits or not until we're
pretty far down the chain.  Hence my original prototype that I sent to
Andy that passed down "have rtnl" status to the lower levels.

	Andy, one thought: do you think it would work better to simplify
the locking that is there first, i.e., convert the timers to work
queues, have a single dispatcher that handles everything (and can be
suspended for mutexing purposes), as in the patch I sent you?  The
problem isn't just rtnl; there also has to be a release of the bonding
locks themselves (to handle the might sleep issues), and that's tricky
to do with so many entities operating concurrently.  Reducing the number
of involved parties should make the problem simpler.

>I once put together a patch that used a macro like this (ignore the
>whitespace problems, this was just a cut and paste):
>
>/**
> * bond_rtnl_wrapper - take the rtnl_lock if needed
> * @x:         function with args
> *
> */
>#define RTNL_WRAPPER(x)                \
>({                                     \
>       int __rc__;                     \
>       if (rtnl_trylock()) {           \
>               __rc__ = x;             \
>               rtnl_unlock();          \
>       } else {                        \
>               __rc__ = x;             \
>       }                               \
>       __rc__;                         \
>})
>
>
>and wrapped it around the calls to dev_set_mac_address.  I wasn't
>pleased with it, but it seemed like it worked pretty well based on the
>testing I did.

	The problem with this is that it'll cause failures in the case
that some other unrelated entity holds rtnl ("x" will be performed
concurrently with whomever actually holds rtnl).

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html