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