All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Debabrata Banerjee <dbanerje@akamai.com>,
	Kay Sievers <kay.sievers@vrfy.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Jeff Mahoney <jeffm@suse.com>,
	dbavatar@gmail.com, johunt@akamai.com,
	stable <stable@vger.kernel.org>
Subject: Re: [PATCH] printk: Fix discarding of records
Date: Sun, 16 Feb 2014 12:47:54 -0800	[thread overview]
Message-ID: <20140216204754.GA16757@kroah.com> (raw)
In-Reply-To: <CA+55aFy7yrNuJaFLcLh5-DVFTzuFYeZ_K9s6CNGtemQDCO90mQ@mail.gmail.com>

On Sun, Feb 16, 2014 at 11:28:36AM -0800, Linus Torvalds wrote:
> Adding Kay and Greg, since the original code is from commit
> 7ff9554bb578 ("printk: convert byte-buffer to variable-length record
> buffer") and all the "prev" flag tweaks end up building on top of
> that.
> 
> The whole "prev flags" is messed up, and LOG_CONT is done very confusingly.
> 
> Why are *those* particular two "prev = msg->flags" incorrect, when
> every other case where we walk the messages they are required?
> 
> The code/logic makes no sense. You remove the "prev = msg->flags" at
> line 1070, when the *identical* loop just above it has it. So now the
> two loops count the number of characters differently. That makes no
> sense.
> 
> So I don't think this fixes the fundamental problem. I'm more inclined
> to believe that LOG_CONT is wrongly set somewhere, for example because
> a continuation wasn't actually originally printed due to coming from
> different users or something like that.
> 
> Or at the very least I want a coherent explanation why one loop would
> do this and the other would not, and why counting up *different*
> numbers could possibly make sense.
> 
> Because as it is, there clearly is some problem, but the patch does
> not look sensible to me.

Yeah, it doesn't make much sense to me either.

Kay had a printk() test module that would stress these types of paths
out a bunch, Kay, is that module around somewhere that we could maybe
add it to the kernel tree so it could be used to test changes like this?

thanks,

greg k-h

  reply	other threads:[~2014-02-16 20:46 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-14  4:42 [PATCH] printk: Fix discarding of records Debabrata Banerjee
2014-02-16 19:28 ` Linus Torvalds
2014-02-16 20:47   ` Greg Kroah-Hartman [this message]
2014-02-16 23:05     ` Kay Sievers
2014-02-16 23:23   ` Banerjee, Debabrata
2014-02-16 23:59     ` Linus Torvalds
2014-02-17  0:19       ` Banerjee, Debabrata
2014-02-17  0:41         ` Linus Torvalds
2014-02-17  0:50           ` Kay Sievers
2014-02-17  0:57             ` Linus Torvalds
2014-02-17  1:19               ` Kay Sievers
2014-02-17  1:41                 ` Kay Sievers
2014-02-17  2:38           ` Banerjee, Debabrata
2014-03-21 15:18             ` Josh Hunt
2014-03-21 15:33               ` Greg Kroah-Hartman
2014-02-16 23:24   ` [PATCH v2] " Debabrata Banerjee
2014-02-17  0:42   ` [PATCH v3] " Debabrata Banerjee

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=20140216204754.GA16757@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=dbanerje@akamai.com \
    --cc=dbavatar@gmail.com \
    --cc=jeffm@suse.com \
    --cc=johunt@akamai.com \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.