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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxx

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.

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@xxxxxxxxxx>

--
Cheers,

David / dhildenb





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux