linux-kernel - Re: [PATCH] x86/fault: speed up uffd-unit-test by 10x: rate-limit "MCE: Killing" logs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJHvVcgYsZJ3Hm1Hpc1pifH49uVniAedL-YxUpS8q7=Y8veZ5g@mail.gmail.com>
Date: Tue, 7 May 2024 11:08:07 -0700
From: Axel Rasmussen <axelrasmussen@...gle.com>
To: David Hildenbrand <david@...hat.com>
Cc: John Hubbard <jhubbard@...dia.com>, Andy Lutomirski <luto@...nel.org>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, Peter Zijlstra <peterz@...radead.org>, 
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, x86@...nel.org, 
	"H . Peter Anvin" <hpa@...or.com>, LKML <linux-kernel@...r.kernel.org>, linux-mm@...ck.org, 
	Peter Xu <peterx@...hat.com>
Subject: Re: [PATCH] x86/fault: speed up uffd-unit-test by 10x: rate-limit
 "MCE: Killing" logs

On Tue, May 7, 2024 at 9:43 AM David Hildenbrand <david@...hat.com> wrote:
>
> On 07.05.24 18:28, John Hubbard wrote:
> > On 5/7/24 1:13 AM, David Hildenbrand wrote:
> >> The patch subject is misleading. This should be "don't flood the system
> >
> > I went back and forth on that subject line. :)
> >
> >> log". Nobody cares about the speed of a unittest ;)
> >
> > Yes they do. People should actually run the selftests, which in turn have
> > enshrined their guidelines in kernel doc. See dev-tools/kselftest.rst,
> > "Contributing new tests", which says, as you would hope, "Don't take
> > too long".
> >
> > It's important. Tests need to be quick, and having one out of 50 that
> > blows it up is worth fixing.
>
> I'm pretty sure you got my point: it's much more important to not let
> unprivileged users flood the log (possibly harming the system?) than
> making a test run faster :)
>
> >
> >>
> >> On 07.05.24 04:29, John Hubbard wrote:
> >>> If a system experiences a lot of memory failures, then any associated
> >>> printk() output really needs to be rate-limited. I noticed this while
> >>> running selftests/mm/uffd-unit-tests, which logs 12,305 lines of output,
> >>> adding (on my system) an extra 97 seconds of runtime due to printk time.
> >>
> >> Recently discussed:
> >>
> >> https://lkml.kernel.org/r/a9e3120d-8b79-4435-b113-ceb20aa45ee2@alu.unizg.hr
> >>
> >> See the pros/cons of using ratelimiting, and what an alternative for
> >> uffd is that Axel is working on.
> >>
> >> (CCing Peter and Axel)
> >>
> >
> > That thread seems to have stalled.
>
> Yes, there was no follow-up.

Apologies, I had completely forgotten about this. I blame the weekend. :)

No objections from me to the simple rate limiting proposed here, if
useful you can take:

Acked-by: Axel Rasmussen <axelrasmussen@...gle.com>

But, it seems to me the earlier proposal may still be useful.
Specifically, don't print at all for "synthetic" poisons from
UFFDIO_POISON or similar mechanisms. This way, "real" errors aren't
gobbled up by the ratelimit due to spam from "synthetic" errors. If
folks agree, I can *actually* send a patch this time. :)

>
> > I *do* have MCE experience (writing a
> > handler,
> > dealing with MCEs and related bugs), and what they wrote so far is exactly
> > correct: if you were going to flood the log, then no, we don't need to see
> > every single line printed. The first 10 or so, plus the fact that rate
> > limiting
> > kicked in, is sufficient to proceed with debugging and/or hardware
> > replacement.
> >
> > I'd like to just do this patch almost as-is, just with a fixed up subject,
> > perhaps:
> >
> >       x86/fault: rate-limit to avoid flooding dmesg with "MCE: Killing"
> > reports
> >
> > Yes?
>
>
> Makes sense to me (and thanks for confirming that we don't need
> complexity elsewhere).
>
> I think we at least need "Fixes:" (not sure if stable is warranted as
> well, 1b0a151c10a6d823f033023b9fdd9af72a89591b didn't CC stable).
>
> Consider adding a link to the report in that thread.
>
> Acked-by: David Hildenbrand <david@...hat.com>
>
> --
> Cheers,
>
> David / dhildenb
>