From: Andrew Morton <akpm@linux-foundation•org>
To: Jens Axboe <jens.axboe@oracle•com>
Cc: Alexey Dobriyan <adobriyan@gmail•com>,
Linus Torvalds <torvalds@linux-foundation•org>,
linux-kernel@vger•kernel.org, netdev@vger•kernel.org,
mark.fasheh@oracle•com, dan.j.williams@intel•com, "Nelson,
Shannon" <shannon.nelson@intel•com>
Subject: Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()
Date: Tue, 24 Jul 2007 01:34:55 -0700 [thread overview]
Message-ID: <20070724013455.691e6752.akpm@linux-foundation.org> (raw)
In-Reply-To: <20070724082207.GN3287@kernel.dk>
On Tue, 24 Jul 2007 10:22:07 +0200 Jens Axboe <jens.axboe@oracle•com> wrote:
> On Tue, Jul 24 2007, Jens Axboe wrote:
> > On Mon, Jul 23 2007, Andrew Morton wrote:
> > > I worked out that the crash I saw was in
> > >
> > > BUG_ON(!pte_none(*(kmap_pte-idx)));
> > >
> > > in the read of kmap_pte[idx]. Which would be weird as the caller is using
> > > a literal KM_USER0.
> > >
> > > So maybe I goofed, and that BUG_ON is triggering (it scrolled off, and I am
> > > unable to reproduce it now).
> > >
> > > If that BUG_ON _is_ triggering then it might indicate that someone is doing
> > > a __GFP_HIGHMEM|__GFP_ZERO allocation while holding KM_USER0.
> >
> > Or doing double kunmaps, or doing a kunmap_atomic() on the page, not the
> > address. I've seen both of those end up triggering that BUG_ON() in a
> > later kmap.
> >
> > Looking over the 2.6.22..2.6.23-rc1 diff, I found one such error in
> > ocfs2 at least. But you are probably not using that, so I'll keep
> > looking...
>
> What about the new async crypto stuff? I've been looking, but is it
> guarenteed that async_memcpy() runs in process context with interrupts
> enabled always? If not, there's a km type bug there.
I think Shannon maintains that now.
> In general, I think the highmem stuff could do with more safety checks:
>
> - People ALWAYS get the atomic unmaps wrong, passing in the page instead
> of the address. I've seen tons of these. And since kunmap_atomic()
> takes a void pointer, nobody notices until it goes boom.
yeah, it's a real trap. For a while I had a patch which converted
kmap_atomic() to return a char*, and kunmap_atomic() to take a char*, so
misuse got compile warnings. But it was a pig to maintain so I tossed it.
It'd be somewhat easier to do now we've converted a lot of callers to
clear_user_highpage() and similar.
> - People easily get the km type wrong - they use KM_USERx in interrupt
> context, or one of the irq variants without disabling interrupts.
>
> If we could just catch these two types of bugs, we've got a lot of these
> problems covered.
Here's the -mm debug patch:
diff -puN arch/i386/mm/highmem.c~kmap_atomic-debugging arch/i386/mm/highmem.c
--- a/arch/i386/mm/highmem.c~kmap_atomic-debugging
+++ a/arch/i386/mm/highmem.c
@@ -30,7 +30,44 @@ void *kmap_atomic(struct page *page, enu
{
enum fixed_addresses idx;
unsigned long vaddr;
+ static unsigned warn_count = 10;
+ if (unlikely(warn_count == 0))
+ goto skip;
+
+ if (unlikely(in_interrupt())) {
+ if (in_irq()) {
+ if (type != KM_IRQ0 && type != KM_IRQ1 &&
+ type != KM_BIO_SRC_IRQ && type != KM_BIO_DST_IRQ &&
+ type != KM_BOUNCE_READ) {
+ WARN_ON(1);
+ warn_count--;
+ }
+ } else if (!irqs_disabled()) { /* softirq */
+ if (type != KM_IRQ0 && type != KM_IRQ1 &&
+ type != KM_SOFTIRQ0 && type != KM_SOFTIRQ1 &&
+ type != KM_SKB_SUNRPC_DATA &&
+ type != KM_SKB_DATA_SOFTIRQ &&
+ type != KM_BOUNCE_READ) {
+ WARN_ON(1);
+ warn_count--;
+ }
+ }
+ }
+
+ if (type == KM_IRQ0 || type == KM_IRQ1 || type == KM_BOUNCE_READ ||
+ type == KM_BIO_SRC_IRQ || type == KM_BIO_DST_IRQ) {
+ if (!irqs_disabled()) {
+ WARN_ON(1);
+ warn_count--;
+ }
+ } else if (type == KM_SOFTIRQ0 || type == KM_SOFTIRQ1) {
+ if (irq_count() == 0 && !irqs_disabled()) {
+ WARN_ON(1);
+ warn_count--;
+ }
+ }
+skip:
/* even !CONFIG_PREEMPT needs this, for in_atomic in do_page_fault */
pagefault_disable();
_
next prev parent reply other threads:[~2007-07-24 8:35 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <alpine.LFD.0.999.0707221351030.3607@woody.linux-foundation.org>
[not found] ` <20070723183839.GA5874@martell.zuzino.mipt.ru>
[not found] ` <20070723190152.GA5755@martell.zuzino.mipt.ru>
2007-07-23 20:24 ` 2.6.23-rc1: BUG_ON in kmap_atomic_prot() Andrew Morton
2007-07-23 20:40 ` Alexey Dobriyan
2007-07-23 21:01 ` Alexey Dobriyan
2007-07-23 21:11 ` Andrew Morton
2007-07-23 21:28 ` Linus Torvalds
2007-07-23 21:37 ` Sam Ravnborg
2007-07-24 17:59 ` Adrian Bunk
2007-07-24 18:14 ` Linus Torvalds
2007-07-24 18:28 ` Andrew Morton
2007-07-24 19:15 ` Linus Torvalds
2007-07-24 19:40 ` Adrian Bunk
2007-07-24 19:48 ` Linus Torvalds
2007-07-26 18:07 ` Adrian Bunk
2007-07-26 18:19 ` Linus Torvalds
2007-07-24 20:27 ` Andi Kleen
2007-07-24 19:45 ` Linus Torvalds
2007-07-26 6:09 ` commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console H. Peter Anvin
2007-07-23 22:04 ` 2.6.23-rc1: BUG_ON in kmap_atomic_prot() Alexey Dobriyan
2007-07-23 22:27 ` Andrew Morton
2007-07-24 5:20 ` Alexey Dobriyan
2007-07-24 8:17 ` Jens Axboe
2007-07-24 8:22 ` Jens Axboe
2007-07-24 8:34 ` Andrew Morton [this message]
2007-07-24 14:00 ` Dan Williams
2007-07-24 13:55 ` Dan Williams
2007-07-24 10:01 ` Mike Galbraith
2007-07-24 10:37 ` Mike Galbraith
2007-07-24 16:28 ` Andrew Morton
2007-07-24 18:25 ` Linus Torvalds
2007-07-24 20:05 ` Alexey Dobriyan
2007-07-25 5:09 ` Mike Galbraith
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=20070724013455.691e6752.akpm@linux-foundation.org \
--to=akpm@linux-foundation$(echo .)org \
--cc=adobriyan@gmail$(echo .)com \
--cc=dan.j.williams@intel$(echo .)com \
--cc=jens.axboe@oracle$(echo .)com \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=mark.fasheh@oracle$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=shannon.nelson@intel$(echo .)com \
--cc=torvalds@linux-foundation$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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