[GIT PULL] trivial for 4.9

[GIT PULL] trivial for 4.9

[Jiri Kosina]

Linus,

please pull from

  git://git.kernel.org/pub/scm/linux/kernel/git/jikos/trivial.git for-linus

to get the usual rocket science from trivial tree.

Thanks.

----------------------------------------------------------------
Andreas Gruenbacher (1):
      doc: vfs: fix fadvise() sycall name

Colin Ian King (5):
      tracing/syscalls: fix multiline in error message text
      netfilter: Add missing \n to pr_err() message
      agp/intel: add missing \n to end of dev_emerg message
      lightnvm: add missing \n to end of dev_err message
      nvme: add missing \n to end of dev_warn message

Laurent Georget (1):
      securityfs: fix securityfs_create_dir comment

Masanari Iida (1):
      irq: Fix typo in tracepoint.xml

Nicolas Iooss (1):
      x86/entry: spell EBX register correctly in documentation

Uwe Kleine-König (1):
      lib/Kconfig.debug: fix DEBUG_SECTION_MISMATCH description

 Documentation/filesystems/vfs.txt | 2 +-
 arch/x86/entry/entry_64.S         | 2 +-
 drivers/char/agp/intel-agp.c      | 2 +-
 drivers/nvme/host/lightnvm.c      | 2 +-
 drivers/nvme/host/pci.c           | 2 +-
 include/trace/events/irq.h        | 2 +-
 kernel/trace/trace_syscalls.c     | 6 ++----
 lib/Kconfig.debug                 | 2 +-
 net/netfilter/core.c              | 2 +-
 security/inode.c                  | 7 +++----
 10 files changed, 13 insertions(+), 16 deletions(-)

-- 
Jiri Kosina
SUSE Labs

Re: [GIT PULL] trivial for 4.9

[Linus Torvalds]

On Fri, Oct 7, 2016 at 1:51 AM, Jiri Kosina <jikos@kernel.org> wrote:
> Colin Ian King (5):
>       netfilter: Add missing \n to pr_err() message
>       agp/intel: add missing \n to end of dev_emerg message
>       lightnvm: add missing \n to end of dev_err message
>       nvme: add missing \n to end of dev_warn message

We really shouldn't be needing these final '\n' characters any more, afaik.

If the next printk isn't done by the same process, and doesn't have a
KERN_CONT, the printk machinery should add the newline on its own.

I realize that people have been adding these '\n' characters for a
while, but is there actually any point to it? They make the code less
legible, imho. And we actually have a number of logging functions that
explicitly don't want the newline (eg ext4_warning/error()), so it's
actually more consistent to *not* have a newline than it is to have
one.

And if those '\n' characters actually make a difference, that should
be noted. Because that would imply that the printk logic isn't working
right.

Hmm?

                   Linus

Re: [GIT PULL] trivial for 4.9

[Joe Perches]

On Fri, 2016-10-07 at 12:22 -0700, Linus Torvalds wrote:
> On Fri, Oct 7, 2016 at 1:51 AM, Jiri Kosina <jikos@kernel.org> wrote:
> > Colin Ian King (5):
> >       netfilter: Add missing \n to pr_err() message
> >       agp/intel: add missing \n to end of dev_emerg message
> >       lightnvm: add missing \n to end of dev_err message
> >       nvme: add missing \n to end of dev_warn message
> 
> 
> We really shouldn't be needing these final '\n' characters any more, afaik.
> 
> If the next printk isn't done by the same process, and doesn't have a
> KERN_CONT, the printk machinery should add the newline on its own.
> 
> I realize that people have been adding these '\n' characters for a
> while, but is there actually any point to it? They make the code less
> legible, imho. And we actually have a number of logging functions that
> explicitly don't want the newline (eg ext4_warning/error()), so it's
> actually more consistent to *not* have a newline than it is to have
> one.

Not remotely true.

It's _way_ more consistent to use a newline termination.
Macros without the newline are _far_ less common than those
with newlines.

Any printk without a KERN_<level> prefix, and there
are still many of those, can cause random interleaving.

> And if those '\n' characters actually make a difference, that should
> be noted. Because that would imply that the printk logic isn't working
> right.

Not at all.  Until printk KERN_<level> uses are mandated,
then these newlines are still useful.

Re: [GIT PULL] trivial for 4.9

[Linus Torvalds]

On Fri, Oct 7, 2016 at 1:04 PM, Joe Perches <joe@perches.com> wrote:
>
> Any printk without a KERN_<level> prefix, and there
> are still many of those, can cause random interleaving.

How about people actually work on *that* instead of working around it?

Because the above really should not be true.

> Not at all.  Until printk KERN_<level> uses are mandated,
> then these newlines are still useful.

The patches literally added those '\n' things to the pr_xyz() routines
that *enforce* KERN_<level>.

So really. It's a step backwards. We shouldn't need them. We should
*remove* '\n' at the end, and then if that actually causes problems,
we should fix those problems.

             Linus

Re: [GIT PULL] trivial for 4.9

[Joe Perches]

On Fri, 2016-10-07 at 13:13 -0700, Linus Torvalds wrote:
> On Fri, Oct 7, 2016 at 1:04 PM, Joe Perches <joe@perches.com> wrote:
> > Any printk without a KERN_<level> prefix, and there
> > are still many of those, can cause random interleaving.
> How about people actually work on *that* instead of working around it?
> Because the above really should not be true.
> > Not at all.  Until printk KERN_<level> uses are mandated,
> > then these newlines are still useful.
> The patches literally added those '\n' things to the pr_xyz() routines
> that *enforce* KERN_<level>.

No, because any of those can be followed by a bare printk
or a pr_cont that continues the original line.

> So really. It's a step backwards. We shouldn't need them. We should
> *remove* '\n' at the end, and then if that actually causes problems,
> we should fix those problems.

$ git grep -w printk * | grep -v "^tools" | grep -v KERN | wc -l
13176

Have at it but here are _far_ fewer missing newlines
and it'd be much lower churn to fix those than remove
all the newlines and fix the missing continuations.

Re: [GIT PULL] trivial for 4.9

[Linus Torvalds]

On Fri, Oct 7, 2016 at 1:18 PM, Joe Perches <joe@perches.com> wrote:
>
> No, because any of those can be followed by a bare printk
> or a pr_cont that continues the original line.

.. and quite frankly, if they do, we should fix *that*.

>> So really. It's a step backwards. We shouldn't need them. We should
>> *remove* '\n' at the end, and then if that actually causes problems,
>> we should fix those problems.
>
> $ git grep -w printk * | grep -v "^tools" | grep -v KERN | wc -l
> 13176
>
> Have at it but here are _far_ fewer missing newlines
> and it'd be much lower churn to fix those than remove
> all the newlines and fix the missing continuations.

None of that has any relevance to the question "why should we add
extra commits to go backwards"?

Those newlines aren't "missing". The lack of newlines is how modern
code can and does work. It's that simple.

And no, I'm not interested in some mass conversion the other way _either_.

I find this noise to add '\n' characters completely pointless. It's
bogus stupid churn that doesn't actually make the source code better,
and it also doesn't actually seem to fix any behavioral issues.

And if there are behavioral issues, they should (a) be pointed out and
(b) be fixed.

In *no* case does it make sense to randomly just add newline
characters without even having a reason for it.

End of story.

            Linus

Re: [GIT PULL] trivial for 4.9

[Joe Perches]

On Fri, 2016-10-07 at 13:25 -0700, Linus Torvalds wrote:
> I find this noise to add '\n' characters completely pointless. It's
> bogus stupid churn that doesn't actually make the source code better,
> and it also doesn't actually seem to fix any behavioral issues.
> 
> And if there are behavioral issues, they should (a) be pointed out and
> (b) be fixed.
> 
> In *no* case does it make sense to randomly just add newline
> characters without even having a reason for it.

It prevents random interleaving from those other
12000+ possible printk calls without an explicit
KERN_<LEVEL>

Re: [GIT PULL] trivial for 4.9

[Linus Torvalds]

On Fri, Oct 7, 2016 at 1:33 PM, Joe Perches <joe@perches.com> wrote:
>>
>> And if there are behavioral issues, they should (a) be pointed out and
>> (b) be fixed.
>>
>> In *no* case does it make sense to randomly just add newline
>> characters without even having a reason for it.
>
> It prevents random interleaving from those other
> 12000+ possible printk calls without an explicit
> KERN_<LEVEL>

YOU ARE NOT LISTENING.

Let me do this really clearly and slowly.

I'm not applying patches that

 (a) are random churn

 (b) make the code uglier

 (c) are about purely theoretical problems IN OTHER CODE.

How hard is that to understand? The "\n"-adding patches are ALL of the
above. They don't fix a problem, they actually *hide* problems, and
they hide problems that

 (1) do not matter

 (2) aren't in the code that the "\n"-adding patch even adds.

See?

So stop with the idiotic theoretical arguments about interleaving that
isn't even true.

Instead, start with the *actual* problems. First off, if somebody
actually reports an issue like the above, fix *that*.

And btw, even without an explicit KERN_<level>, you should still not
get any interleaving. Only an _explicit_ KERN_CONT should cause
interleaving, and dammit, if some interrupt does a KERN_CONT without
having had a non-cont printk before it, that code is buggy and should
damn well be fixed.

And if it's not an interrupt, then the "not the same task as last
time" should add the newline.

In other words, all your arguments seem entirely wrong. IF that
interleaving actually happens, we have bugs that should be fixed.

Again: we should not add stupid churn that doesn't even fix the bugs,
just makes code harder to read and adds churn.

Seriously.

If you can send me a patch to *fix* anything, go ahead. But stop this
idiotic "let's add random pointless newline characters" crap already.

               Linus

Re: [GIT PULL] trivial for 4.9

[Linus Torvalds]

On Fri, Oct 7, 2016 at 2:06 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> And btw, even without an explicit KERN_<level>, you should still not
> get any interleaving. Only an _explicit_ KERN_CONT should cause
> interleaving

Btw, note the "should" there. Because we do seem to have broken that
_again_.  It worked fine at some point, but lookie here:

commit 61e99ab8e35a88b8c4d0f80d3df9ee16df471be5
Author: Joe Perches <joe@perches.com>
Date:   Mon Jul 30 14:40:21 2012 -0700

    printk: remove the now unnecessary "C" annotation for KERN_CONT

    Now that all KERN_<LEVEL> uses are prefixed with ASCII SOH, there is no
    need for a KERN_CONT.  Keep it backward compatible by adding #define
    KERN_CONT ""

Joe, you *are* the problem here.

So you are literally the person who broke this.

Goddammit, I don't want to hear another peep from you. You broke this
because you wanted to save a few bytes in those strings, and then
*because* you broke it, you then argue for putting those bytes back in
the form of "\n" characters.

Fuck me sideways. You make this big deal about how this interleaving
is a big problem, and at no point did you actually point to the real
issue, which was your very own breakage where you made it all fragile.

Christ.

                    Linus

Re: [GIT PULL] trivial for 4.9

[Joe Perches]

On Fri, 2016-10-07 at 14:06 -0700, Linus Torvalds wrote:
> And btw, even without an explicit KERN_<level>, you should still not
> get any interleaving. Only an _explicit_ KERN_CONT should cause
> interleaving, and dammit, if some interrupt does a KERN_CONT without
> having had a non-cont printk before it, that code is buggy and should
> damn well be fixed.

That's not true.  KERN_CONT is a no-op.
Bare printks interleave.

$ git grep KERN_CONT include/linux/kern_levels.h
include/linux/kern_levels.h:#define KERN_CONT   ""

I think it was like 2007 when I first suggested _not_ having
newlines on the pr_<level> macros that were eventually added
by Emil Medve.

Re: [GIT PULL] trivial for 4.9

[Tony Luck]

What if there isn't a "next printk" call for hours, or days?

That poor little message without a "\n" will sit in the kernel buffers,
and the user who might want to see the message can't, until some
unrelated thing happens to print something.

-Tony

Re: [GIT PULL] trivial for 4.9

[Tony Luck]

On Fri, Oct 7, 2016 at 4:01 PM, Tony Luck <tony.luck@gmail.com> wrote:
> What if there isn't a "next printk" call for hours, or days?
>
> That poor little message without a "\n" will sit in the kernel buffers,
> and the user who might want to see the message can't, until some
> unrelated thing happens to print something.

Retracted ... I'm sure that at some point in the past it happened like
that ... but
I just retested on 4.8 and the first message (with no "\n") showed up on the
serial port just fine without some other message to push it out. When the
next message came along, a "\n" was auto-inserted.

-Tony

Re: [GIT PULL] trivial for 4.9

[Linus Torvalds]

On Fri, Oct 7, 2016 at 4:09 PM, Tony Luck <tony.luck@gmail.com> wrote:
> On Fri, Oct 7, 2016 at 4:01 PM, Tony Luck <tony.luck@gmail.com> wrote:
>> What if there isn't a "next printk" call for hours, or days?
>>
>> That poor little message without a "\n" will sit in the kernel buffers,
>> and the user who might want to see the message can't, until some
>> unrelated thing happens to print something.
>
> Retracted ...  I'm sure that at some point in the past it happened
> like that ...  but I just retested on 4.8 and the first message (with
> no "\n") showed up on the serial port just fine without some other
> message to push it out.  When the next message came along, a "\n" was
> auto-inserted.

Yeah, that immediate printout has actually always worked fine - the
newline was never really a buffering thing.

The buffering actually came fairly late, with the "newfangled"
record-oriented logging facility (4+ years old by now).

Our kernel message log was historically just a plain buffer, and not
record-oriented at all. You'd just read and write it as a stream. In
that historical context, it made tons of sense to just write something
without a newline, and then continue writing on the same line: that's
how <stderr> always works.

But back in 2012, Kay Sievers wanted to make it record-based, because
reasons. That *really* doesn't play well with the whole "oh, you might
not get the whole thing in one go" model, and things were broken a few
times. It also made our printk implementation a lot more complex. But
there was some argument for a full-featured syslog facility.  Line
continuations suddenly weren't very natural any more, because now a
continuation printk was very much a "broken record".

Anyway, the complexity has had upsides too, and the "let's not require
'\n' at the end" actually came in through that (in fact, I think the
record-based internal format removes the newlines at the end of
characters). And there are some real advantages, both with timestamps
and with having per-record log levels. So it's definitely not all bad,
but there's a fair amount of complexity in there. The old model was
rather broken in other ways, though, so on the whole I think we're
doing fairly ok.

But yes, the line continuation that *used* to be very natural (but
always had problems with concurrent output from multiple contexts)
definitely makes for extra complexity in the record-based model. But
exactly *because* the record-based thing needs to be more careful
about those newlines, it actually ended up being why the explicit "\n"
thing at the end of a printk shouldn't really matter any more.

And not having it does make for nicer printk strings. "Just the facts".

             Linus

Re: [GIT PULL] trivial for 4.9

[Joe Perches]

On Fri, 2016-10-07 at 14:37 -0700, Linus Torvalds wrote:
> On Fri, Oct 7, 2016 at 2:06 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > 
> > And btw, even without an explicit KERN_<level>, you should still not
> > get any interleaving. Only an _explicit_ KERN_CONT should cause
> > interleaving
> 
> 
> Btw, note the "should" there. Because we do seem to have broken that
> _again_.  It worked fine at some point, but lookie here:
> 
> commit 61e99ab8e35a88b8c4d0f80d3df9ee16df471be5
> Author: Joe Perches <joe@perches.com>
> Date:   Mon Jul 30 14:40:21 2012 -0700
> 
>     printk: remove the now unnecessary "C" annotation for KERN_CONT
> 
>     Now that all KERN_<LEVEL> uses are prefixed with ASCII SOH, there is no
>     need for a KERN_CONT.  Keep it backward compatible by adding #define
>     KERN_CONT ""
> 
> Joe, you *are* the problem here.
> 
> So you are literally the person who broke this.
> 
> Goddammit, I don't want to hear another peep from you. You broke this
> because you wanted to save a few bytes in those strings, and then
> *because* you broke it, you then argue for putting those bytes back in
> the form of "\n" characters.
> 
> Fuck me sideways. You make this big deal about how this interleaving
> is a big problem, and at no point did you actually point to the real
> issue, which was your very own breakage where you made it all fragile.

The _only_ person that's making a "big deal"
out of it is _you_. 

And I believe you are wrong here.  Both about the
original behavior and the effect of the change I made.

I'm not swearing, I'm just commenting.
In no case is a comment anything like a "big deal".

And the only thing that commit did was avoid
skipping forward in the format.  It didn't have
anything to do with interleaving.

The logging code for v3.5 and v3.6, (pre and post
this patch) both of which I just tested, emit these
on a single line

	printk(KERN_INFO "format");
	printk(" continued on the same line\n");

Same as

	pr_info("format");
	pr_cont(" continued on the same line\n");

neither dmesg nor /dev/kmsg show any difference.

Re: [GIT PULL] trivial for 4.9

[Steven Rostedt]

On Fri, Oct 07, 2016 at 04:09:51PM -0700, Tony Luck wrote:
> On Fri, Oct 7, 2016 at 4:01 PM, Tony Luck <tony.luck@gmail.com> wrote:
> > What if there isn't a "next printk" call for hours, or days?
> >
> > That poor little message without a "\n" will sit in the kernel buffers,
> > and the user who might want to see the message can't, until some
> > unrelated thing happens to print something.
> 
> Retracted ... I'm sure that at some point in the past it happened like
> that ... but
> I just retested on 4.8 and the first message (with no "\n") showed up on the
> serial port just fine without some other message to push it out. When the
> next message came along, a "\n" was auto-inserted.
> 

No there was a small window in time that it did just that. And it caused me a
day of debugging. My ftrace self tests at boot up do something like:

printk("testing X....");

if (test(X))
	printk(" PASSED\n");
else
	printk(" FAILED\n");


And with this new change, the "testing X..." never appeared, but it caused the
system to crash, It confused me because it crashed right after a:

testing Y.... PASSED

came out.

Thus I was debugging why Y failed caused the system crash even though it
reported a passing statement. The sad part was that my bug came in after this
updated to printk. So in bisecting I didn't notice. But what I did notice in
bisecting, was test X, and I tried that, and sure enough it crashed. Then
that's where I started looking at changes in printk.

I made a minor stink about it, and we converted the behavior back to where
kernel writes to printk would always go to the console.

-- Steve

Re: [GIT PULL] trivial for 4.9

[Joe Perches]

On Fri, 2016-10-07 at 14:37 -0700, Linus Torvalds wrote:
> On Fri, Oct 7, 2016 at 2:06 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > 
> > And btw, even without an explicit KERN_<level>, you should still not
> > get any interleaving. Only an _explicit_ KERN_CONT should cause
> > interleaving
> 
> 
> Btw, note the "should" there. Because we do seem to have broken that
> _again_.  It worked fine at some point, but lookie here:

So, I've gone back to each release from 3.6 to 3.3 looking for a case
where interleaving printks inserted newlines automatically for different
processes.

I can't find one.  Neither in dmesg nor in /dev/kmsg.

What version of the kernel do _you_ think had this behavior?

> commit 61e99ab8e35a88b8c4d0f80d3df9ee16df471be5
> Author: Joe Perches <joe@perches.com>
> Date:   Mon Jul 30 14:40:21 2012 -0700
> 
>     printk: remove the now unnecessary "C" annotation for KERN_CONT
> 
>     Now that all KERN_<LEVEL> uses are prefixed with ASCII SOH, there is no
>     need for a KERN_CONT.  Keep it backward compatible by adding #define
>     KERN_CONT ""
> 
> Joe, you *are* the problem here.

It's not any release version from 3.3 to 3.6 that I can find.
Perhaps you have faster processes than I have to isolate this.

> So you are literally the person who broke this.

No, I don't think so.  Not with the patch above.
Nor with any patch I wrote from 3.3 to 3.6.

> Goddammit, I don't want to hear another peep from you.

Try again, this time with feeling.

> You broke this because you wanted to save a few bytes in those strings,

Saving code size is generally good.
I didn't break any existing behavior.

> Fuck me sideways.

I trust and hope you prefer to be the big spoon.

Newlines added to formats today _do_ help prevent
interleaving from other processes.

Is that optimal?  No.
Would I prefer no newlines for these formats?  Sure.
Does your proposal to eliminate newlines work today?  No.
Is it an easy problem to fix?  No.

For today, it's better to add newlines to formats to help
avoid random interleaving.