From patchwork Mon Nov 30 10:23:31 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: David Woodhouse X-Patchwork-Id: 39792 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [18.85.46.34]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id CBD4CB6EE8 for ; Mon, 30 Nov 2009 21:25:21 +1100 (EST) Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1NF3Px-0006SQ-SJ; Mon, 30 Nov 2009 10:23:41 +0000 Received: from casper.infradead.org ([2001:770:15f::2]) by bombadil.infradead.org with esmtps (Exim 4.69 #1 (Red Hat Linux)) id 1NF3Pv-0006Kf-Fi for linux-mtd@bombadil.infradead.org; Mon, 30 Nov 2009 10:23:39 +0000 Received: from macbook.infradead.org ([2001:8b0:10b:1:216:eaff:fe05:bbb8]) by casper.infradead.org with esmtpsa (Exim 4.69 #1 (Red Hat Linux)) id 1NF3Pp-0003Ai-LA; Mon, 30 Nov 2009 10:23:34 +0000 Subject: Re: [PATCH/RFC v5 4/5]: core: Add dump device to call on oopses and panics From: David Woodhouse To: dedekind1@gmail.com In-Reply-To: <1259571118.7518.56.camel@localhost> References: <20091012131528.GC25464@elte.hu> <20091012153937.0dcd73e5@marrow.netinsight.se> <20091012110954.67d7d8d8.akpm@linux-foundation.org> <20091012182346.GH17138@elte.hu> <20091013151751.59e217a7@marrow.netinsight.se> <20091013152235.188059d2@marrow.netinsight.se> <20091126093657.GA25430@logfs.org> <1259566071.7518.48.camel@localhost> <20091130074603.GA30911@logfs.org> <1259571118.7518.56.camel@localhost> Date: Mon, 30 Nov 2009 10:23:31 +0000 Message-ID: <1259576611.19465.374.camel@macbook.infradead.org> Mime-Version: 1.0 X-Mailer: Evolution 2.28.1 (2.28.1-2.fc12) X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org See http://www.infradead.org/rpr.html Cc: =?ISO-8859-1?Q?J=F6rn?= Engel , LKML , "Koskinen Aaro \(Nokia-D/Helsinki\)" , linux-mtd , Simon Kagstrom , Ingo Molnar , Linus Torvalds , Andrew Morton , Alan Cox X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-mtd-bounces@lists.infradead.org Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org On Mon, 2009-11-30 at 10:51 +0200, Artem Bityutskiy wrote: > On Mon, 2009-11-30 at 08:46 +0100, Jörn Engel wrote: > > On Mon, 30 November 2009 09:27:51 +0200, Artem Bityutskiy wrote: > > > > > > To me it looks like 'log_end' is not supposed to wrap. What makes you > > > think it can? In which cases it can? > > > > It is a 32bit variable. Would do you expect happens once you reach > > 0xffffffff and add 1? > > Yes, now I see log_end is an ever increasing variable. > > How about this patch on top of the existing one (untested): I suspect you should be modelling this more closely on the code in do_syslog() for case 3. The end is at (log_end & LOG_BUF_MASK), and you have (logged_chars) to write. This mean that you won't write messages to the log which were 'cleared' by 'dmesg -c', but that's acceptable, I think. Why are you setting s1 to "" in the case where l1 is zero, btw? Can't it be NULL? diff --git a/kernel/printk.c b/kernel/printk.c index f711b99..d150c57 100644 --- a/kernel/printk.c +++ b/kernel/printk.c @@ -1486,26 +1486,33 @@ static const char *kmsg_to_str(enum kmsg_dump_reason reason) */ void kmsg_dump(enum kmsg_dump_reason reason) { - unsigned long len = ACCESS_ONCE(log_end); + unsigned long end; + unsigned chars; struct kmsg_dumper *dumper; const char *s1, *s2; unsigned long l1, l2; unsigned long flags; - s1 = ""; - l1 = 0; - s2 = log_buf; - l2 = len; - - /* Have we rotated around the circular buffer? */ - if (len > log_buf_len) { - unsigned long pos = len & LOG_BUF_MASK; + /* Theoretically, the log could move on after we do this, but + there's not a log we can do about that. The new messages + will overwrite the start of what we dump. */ + spin_lock_irqsave(&logbuf_lock, flags); + end = log_end & LOG_BUF_MASK; + chars = logged_chars; + spin_unlock_irqrestore(&logbuf_lock, flags); - s1 = log_buf + pos; - l1 = log_buf_len - pos; + if (logged_chars > end) { + s1 = log_buf + log_buf_len - logged_chars + end; + l1 = logged_chars - end; s2 = log_buf; - l2 = pos; + l2 = end; + } else { + s1 = ""; + l1 = 0; + + s2 = log_buf + end - logged_chars; + l2 = logged_chars; } if (!spin_trylock_irqsave(&dump_list_lock, flags)) {