Re: 2.6.18: Kernel BUG at mm/rmap.c:522

[Nick Piggin <nickpiggin@yahoo.com.au>]

Dave Jones wrote:
> On Wed, Oct 04, 2006 at 05:49:00PM +0200, Peter Zijlstra wrote:
>
>  > > > It is also nice if we can work out where the page actually came from. The
>  > > > following attached patch should help out a bit with that, if you could
>  > > > run with it?
>  > > Okay. I'll reboot with your patch and let you know if it crashes again.
>  > enable CONFIG_DEBUG_VM to get that.
>
> Given this warnings still pops up from time to time, I question whether
> putting that check under DEBUG_VM was such a good idea.  It's not as
> if it's a major performance impact.  This has potential for us to lose
> valuable debugging info for a few nanoseconds performance increase in
> an already costly path.

Frustratingly, it usually doesn't tell us much (without my previous
patch), because it is normally some driver that has stuffed up their
refcounting and the page-> fields don't tell us where the page has
come from.

But..

> This patch brings it back unconditionally, and moves the BUG()
> into the if arm.

... this shouldn't hurt if gcc moves the unlikely code out of the
linear instruction stream, which I think it usually does. It shouldn't
cost _anything_ because we're already doing the branch for the BUG_ON
which you remove.

> Signed-off-by: Dave Jones <davej@redhat.com>

Thanks,
Acked-by: Nick Piggin <npiggin@suse.de>

> --- local-git/mm/rmap.c~	2006-10-04 16:38:06.000000000 -0400
> +++ local-git/mm/rmap.c	2006-10-04 16:38:24.000000000 -0400
> @@ -576,15 +576,14 @@ void page_add_file_rmap(struct page *pag
>  void page_remove_rmap(struct page *page)
>  {
>  	if (atomic_add_negative(-1, &page->_mapcount)) {
> -#ifdef CONFIG_DEBUG_VM
>  		if (unlikely(page_mapcount(page) < 0)) {
>  			printk (KERN_EMERG "Eeek! page_mapcount(page) went negative! (%d)\n", page_mapcount(page));
>  			printk (KERN_EMERG "  page->flags = %lx\n", page->flags);
>  			printk (KERN_EMERG "  page->count = %x\n", page_count(page));
>  			printk (KERN_EMERG "  page->mapping = %p\n", page->mapping);
> +			BUG();
>  		}
> -#endif
> -		BUG_ON(page_mapcount(page) < 0);
> +
>  		/*
>  		 * It would be tidy to reset the PageAnon mapping here,
>  		 * but that might overwrite a racing page_add_anon_rmap


--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/