Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1495185 > unrolled thread

Re: BUG_ON() in workingset_node_shadows_dec() triggers

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: BUG_ON() in workingset_node_shadows_dec() triggers Andrew Morton <akpm@linux-foundation.org> - 2016-10-04 06:10 +0200
    Re: BUG_ON() in workingset_node_shadows_dec() triggers Linus Torvalds <torvalds@linux-foundation.org> - 2016-10-04 06:20 +0200
      Re: BUG_ON() in workingset_node_shadows_dec() triggers Raymond Jennings <shentino@gmail.com> - 2016-10-04 09:10 +0200
        Re: BUG_ON() in workingset_node_shadows_dec() triggers Linus Torvalds <torvalds@linux-foundation.org> - 2016-10-04 18:10 +0200

#1495185 — Re: BUG_ON() in workingset_node_shadows_dec() triggers

On Mon, 3 Oct 2016 21:00:55 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote:

> In particular, I just got this
> 
>     kernel BUG at ./include/linux/swap.h:276

Well, it's a VM_BUG_ON and few people run with CONFIG_DEBUG_VM.

But a) something's clearly wrong and b) points taken.

[toc] | [next] | [standalone]

#1495190

On Mon, Oct 3, 2016 at 9:07 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
>
> Well, it's a VM_BUG_ON and few people run with CONFIG_DEBUG_VM.

Ehh. If by "few people" you mean "pretty much everybody", you'd be
right, but your choice of wording would be somewhat misleading,
wouldn't you say?

Hint: here's a line from the standard Fedora kernel config:

    CONFIG_DEBUG_VM=y

so *no*. VM_BUG_ON() is no less deadly than a regular BUG_ON(). It
just allows some people to build smaller kernels, but apparently
distro people would rather have debugging than save a few kB of RAM.

The VM debvugging code has VM_WARN_ON() and VM_WARN_ON_ONCE() for
people who want to get a "oops, my assumptions were wrong"

Killing machines because somebody made an assumption that was wrong is not ok.

Killing the machine is ok if we have a situation where there literally
is no other choice.

                  Linus

[toc] | [prev] | [next] | [standalone]

#1495234

On Mon, Oct 3, 2016 at 9:12 PM, Linus Torvalds 
<torvalds@linux-foundation.org> wrote:
> On Mon, Oct 3, 2016 at 9:07 PM, Andrew Morton 
> <akpm@linux-foundation.org> wrote:
>> 
>>  Well, it's a VM_BUG_ON and few people run with CONFIG_DEBUG_VM.
> 
> Ehh. If by "few people" you mean "pretty much everybody", you'd be
> right, but your choice of wording would be somewhat misleading,
> wouldn't you say?
> 
> Hint: here's a line from the standard Fedora kernel config:
> 
>     CONFIG_DEBUG_VM=y
> 
> so *no*. VM_BUG_ON() is no less deadly than a regular BUG_ON(). It
> just allows some people to build smaller kernels, but apparently
> distro people would rather have debugging than save a few kB of RAM.
> 
> The VM debvugging code has VM_WARN_ON() and VM_WARN_ON_ONCE() for
> people who want to get a "oops, my assumptions were wrong"
> 
> Killing machines because somebody made an assumption that was wrong 
> is not ok.
> 
> Killing the machine is ok if we have a situation where there literally
> is no other choice.

For the curious:

This would include situations like

1.  The kernel is confused and further processing would result in 
undefined behavior (like bluesmoke detecting PCC for example)

2.  Security hazards where we'd leak stuff if we don't shut down.

?

>                   Linus

[toc] | [prev] | [next] | [standalone]

#1495496

On Tue, Oct 4, 2016 at 12:03 AM, Raymond Jennings <shentino@gmail.com> wrote:
> On Mon, Oct 3, 2016 at 9:12 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>>
>>
>> Killing the machine is ok if we have a situation where there literally
>> is no other choice.
>
> For the curious:
>
> This would include situations like
>
> 1.  The kernel is confused and further processing would result in undefined
> behavior (like bluesmoke detecting PCC for example)

Yes. Mainly situations where you cannot even have sane error handling,
so you can't just do "warn and return without doing anything". It
might be some "I noticed that the CPU stack is corrupt, I can't even
return, I will just have to terminate".

> 2.  Security hazards where we'd leak stuff if we don't shut down.

Honestly, I can't think of a situation where that has actually happened.

Now, sometimes BUG_ON() is less onerous than other time: if you know
that you are in a regular process context where you're not holding
core locks, BUG_ON() is actually fairly benign: it will print a big
scary message, and then it will kill the current process. It's not
going to kill the machine, unless the admin has explicitly asked for
"reboot if you have issues", which is mainly a situation for the
googles of the world - if you have millions of machines and you don't
actually *care*, then rebooting is fine.

So realistically, the main places you should use BUG_ON() variants is

 (a) development code where it replaces error handling that you just
haven't written yet, and you haven't really thought through all the
possibilities, so you're saying "this can't happen, I'll fix it
later".

  It sounds like Andrew thought that that is what VM_BUG_ON() is, and
that it wouldn't be enabled unless you're a developer. But no, this is
a "RFC patch" kind of situation.

  This kind of BUG_ON() often ends up escaping into the wild, but it
should be after *huge* amounts of testing, and by definition it should
never have been accepted during anythign but the merge window. So in a
very real sense it's really my bad for not reacting to the BUG_ON()
being added during rc8.

 (b) very core code that actually verifies some very core assumptions
that are *so* important that if they are broken the code is by
definition not really able to function.

  This is the actual intended case. It's not a "let's check that
everybody did things right", it's a "this is a major design rule in
this core code".

The example in workingset_node_shadows_dec() _could_ actually have
been that kind of (b) situation, except for the timing and lack of
deep testing. But a reasonable example of (b) would be something like
the

        BUG_ON(!PageLocked(page));

kind of code in fs/buffer.c - it's core infrastructure that has been
tested with core code, and the BUG_ON() is meant to catch bad _new_
users quickly. And it's *such* a core requirement that error handling
doesn't even make sense.

Again, workingset_node_shadows_dec() could have a BUG_ON() in theory.
But the BUG_ON() is _wrong_ when we had a situation of "oh, we just
recently noticed a bug in this area, so lets' just verify that it's
really gone".

Notice? Just the timing and intent can make the difference between
"good BUG_ON() in solid code that has been around forever" and "bad
BUG_ON() checking something that we know we might be getting wrong".

                                Linus

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web