| Message ID | 200902112127.n1BLRJeJ031575@imap1.linux-foundation.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Andrew Morton wrote: > Solve both these problems by switching to in_interrupt(). Now, if someone > runs a gfp_any() allocation from inside spinlock we will get the warning > if CONFIG_PREEMPT=y. > static inline gfp_t gfp_any(void) > { > - return in_atomic() ? GFP_ATOMIC : GFP_KERNEL; > + return in_softirq() ? GFP_ATOMIC : GFP_KERNEL; > } sed -i -e 's/in_softirq/in_interrupt/' ? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 12 Feb 2009 06:38:55 +0900 Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote: > Andrew Morton wrote: > > Solve both these problems by switching to in_interrupt(). Now, if someone > > runs a gfp_any() allocation from inside spinlock we will get the warning > > if CONFIG_PREEMPT=y. > > > static inline gfp_t gfp_any(void) > > { > > - return in_atomic() ? GFP_ATOMIC : GFP_KERNEL; > > + return in_softirq() ? GFP_ATOMIC : GFP_KERNEL; > > } > sed -i -e 's/in_softirq/in_interrupt/' ? heh, good question. I did all my testing with in_softirq() - the changelog didn't catch up. Is gfp_any() supposed to be usable from hardirq context? If so, we should use in_interrupt(). If not, we should use in_softirq(), and we'll then get might_sleep() warnings if anyone uses gfp_any() from hard irq context. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Resend with cc's restored. Please DO NOT edit the Cc: line :( On Thu, 12 Feb 2009 06:38:55 +0900 Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote: > Andrew Morton wrote: > > Solve both these problems by switching to in_interrupt(). Now, if someone > > runs a gfp_any() allocation from inside spinlock we will get the warning > > if CONFIG_PREEMPT=y. > > > static inline gfp_t gfp_any(void) > > { > > - return in_atomic() ? GFP_ATOMIC : GFP_KERNEL; > > + return in_softirq() ? GFP_ATOMIC : GFP_KERNEL; > > } > sed -i -e 's/in_softirq/in_interrupt/' ? heh, good question. I did all my testing with in_softirq() - the changelog didn't catch up. Is gfp_any() supposed to be usable from hardirq context? If so, we should use in_interrupt(). If not, we should use in_softirq(), and we'll then get might_sleep() warnings if anyone uses gfp_any() from hard irq context. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Andrew Morton <akpm@linux-foundation.org> Date: Wed, 11 Feb 2009 13:48:42 -0800 > Is gfp_any() supposed to be usable from hardirq context? > > If so, we should use in_interrupt(). > > If not, we should use in_softirq(), and we'll then get might_sleep() > warnings if anyone uses gfp_any() from hard irq context. It never will be used in hardware IRQ context, at least in the networking. And all uses in the tree are.. networking :-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: akpm@linux-foundation.org Date: Wed, 11 Feb 2009 13:27:19 -0800 > The problem is that in_atomic() will return false inside spinlocks if > CONFIG_PREEMPT=n. This will lead to deadlockable GFP_KERNEL allocations > from spinlocked regions. > > Secondly, if CONFIG_PREEMPT=y, this bug solves itself because networking > will instead use GFP_ATOMIC from this callsite. Hence we won't get the > might_sleep() debugging warnings which would have informed us of the buggy > callsites. > > Solve both these problems by switching to in_interrupt(). Now, if someone > runs a gfp_any() allocation from inside spinlock we will get the warning > if CONFIG_PREEMPT=y. > > I reviewed all callsites and most of them were too complex for my little > brain and none of them documented their interface requirements. I have no > idea what this patch will do. > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff -puN include/net/sock.h~net-dont-use-in_atomic-in-gfp_any include/net/sock.h --- a/include/net/sock.h~net-dont-use-in_atomic-in-gfp_any +++ a/include/net/sock.h @@ -1313,7 +1313,7 @@ static inline int sock_writeable(const s static inline gfp_t gfp_any(void) { - return in_atomic() ? GFP_ATOMIC : GFP_KERNEL; + return in_softirq() ? GFP_ATOMIC : GFP_KERNEL; } static inline long sock_rcvtimeo(const struct sock *sk, int noblock)