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

[Axel Rasmussen <axelrasmussen@google.com>]

On Tue, May 7, 2024 at 9:43 AM David Hildenbrand <david@redhat.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@google.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@redhat.com>
>
> --
> Cheers,
>
> David / dhildenb
>