From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail144.messagelabs.com (mail144.messagelabs.com [216.82.254.51]) by kanga.kvack.org (Postfix) with SMTP id 99D4F900001 for ; Fri, 29 Apr 2011 05:14:59 -0400 (EDT) Message-ID: <4DBA818E.6040501@kpanic.de> Date: Fri, 29 Apr 2011 11:14:54 +0200 From: Stefan Assmann MIME-Version: 1.0 Subject: Re: [RFC PATCH 2/3] support for broken memory modules (BadRAM) References: <1303921007-1769-1-git-send-email-sassmann@kpanic.de> <1303921007-1769-3-git-send-email-sassmann@kpanic.de> <20110427211258.GQ16484@one.firstfloor.org> <4DB90A66.3020805@kpanic.de> <20110428150821.GT16484@one.firstfloor.org> <4DB98D13.1050107@kpanic.de> In-Reply-To: <4DB98D13.1050107@kpanic.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Andi Kleen Cc: linux-mm@kvack.org, tony.luck@intel.com, mingo@elte.hu, hpa@zytor.com, rick@vanrein.org, akpm@linux-foundation.org, lwoodman@redhat.com, riel@redhat.com On 28.04.2011 17:51, Stefan Assmann wrote: > On 04/28/2011 05:08 PM, Andi Kleen wrote: >>> You're right, logging every page marked would be too verbose. That's why >>> I wrapped that logging into pr_debug. >> >> pr_debug still floods the kernel log buffer. On large systems >> it often already overflows. > > That's a pain then, I understand. I took a closer look at pr_debug and it seems that pr_debug gets evaluated to a conditional branch and thus does not flood the log buffer if not explicitly enabled. I confirmed that by dumping the log buffer. So in the normal use-case things should be fine and if pr_debug really is enabled it dumps a lot of data, which I hope is acceptable for debugging purposes. > >> >>> However I kept the printk in the case of early allocated pages. The user >>> should be notified of the attempt to mark a page that's already been >>> allocated by the kernel itself. >> >> That's ok, although if you're unlucky (e.g. hit a large mem_map area) >> it can be also very nosiy. >> >> It would be better if you fixed the printks to output ranges. > > BadRAM patterns might often mark non-consecutive pages so outputting > ranges could be more verbose than what we have now. I'll try to think > of something to minimize log output. How about the following: static int __init badram_mark_pages(unsigned long addr, unsigned long mask) { unsigned long pagecount = 0, is_reserved = 0; [...] printk(KERN_INFO "BadRAM: mark 0x%lx with mask 0x%0lx\n", addr, mask); do { [...] if (memblock_is_reserved(addr)) { pr_debug("BadRAM: page %lu reserved by kernel\n", pfn); is_reserved++; continue; } [...] pr_debug("BadRAM: page %lu (addr 0x%0lx) marked bad " "[total %lu]\n", pfn, addr, pagecount); } while (next_masked_address(&addr, mask)); if (is_reserved) printk(KERN_WARNING "BadRAM: %lu page(s) already reserved and " "could not be marked bad\n", is_reserved); return pagecount; } This way everything with possibly high volume log output is guarded by pr_debug and only the summary gets printed by default. No log_buf cluttering but also a bit harder to debug for somebody who's interested in finding out which pages are already reserved. Stefan -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: email@kvack.org