All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Down <chris@chrisdown.name>
To: Borislav Petkov <bp@alien8.de>
Cc: linux-kernel@vger.kernel.org, sean.j.christopherson@intel.com,
	tony.luck@intel.com, torvalds@linux-foundation.org,
	x86@kernel.org, kernel-team@fb.com
Subject: Re: [PATCH -v2.1] x86/msr: Filter MSR writes
Date: Tue, 14 Jul 2020 13:19:55 +0100	[thread overview]
Message-ID: <20200714121955.GA2080@chrisdown.name> (raw)
In-Reply-To: <20200615063837.GA14668@zn.tnic>

Hi Borislav,

This is certainly a good idea, but I wonder whether we should be more pragmatic 
about the printk ratelimiting while we give userspace time to react and update 
their methodologies.

As one example, there is a common MSR hack which is verging on essential if 
you're doing thermally intensive work on some recent ThinkPads[0][1], and this 
drastically reduces the signal-to-noise ratio in kmsg (and this is only about 
five minutes after boot):

     % dmesg | wc -l
     2963
     % dmesg | grep -c 'unrecognized MSR'
     2411

That is, even with pr_err_ratelimited, we still end up logging on basically 
every single write, even though it's from the same TGID writing to the same 
MSRs, and end up becoming >80% of kmsg.

Of course, one can boot with `allow_writes=1` to avoid these messages at all, 
but that then has the downfall that one doesn't get _any_ notification at all 
about these problems in the first place, and so is much less likely to forget 
to fix it. One might rather it was less binary: it was still logged, just less 
often, so that application developers _do_ have the incentive to improve their 
current methods, without us having to push other useful stuff out of the kmsg 
buffer.

This one example isn't the point, of course: I'm sure there are plenty of other 
non-ideal-but-pragmatic cases where people are writing to MSRs from userspace 
right now, and it will take time for those people to find other solutions.

I completely agree with you that there should be a better solution for these 
cases, and that writing to MSRs from userspace is really not a good idea.  
However, going from zero to over 80% of dmesg in cases where these MSRs are 
repeatedly used seems too fast to me.

Have you considered perhaps making the ramping up of error logging more gradual 
by having this printk have its own, more conservative `struct ratelimit_state`, 
as we do in some other places with similar noise concerns? Then we could 
gradually make the warnings more aggressive as time goes on, up until the point 
where we make allow_writes=0 the default.

Thanks,

Chris

0: Lenovo is supposedly fixing this since last year, but no news yet.
1: https://github.com/erpalma/throttled

  parent reply	other threads:[~2020-07-14 12:20 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-12 10:50 [RFC PATCH] x86/msr: Filter MSR writes Borislav Petkov
2020-06-12 16:34 ` Sean Christopherson
2020-06-12 16:46   ` Borislav Petkov
2020-06-12 16:57     ` Sean Christopherson
2020-06-12 17:03       ` Borislav Petkov
2020-06-12 17:43         ` Sean Christopherson
2020-06-12 17:52           ` Borislav Petkov
2020-06-12 17:20 ` Linus Torvalds
2020-06-12 17:48   ` Borislav Petkov
2020-06-12 19:47     ` Borislav Petkov
2020-06-12 20:39     ` Peter Zijlstra
2020-06-13  5:40       ` Tony Luck
2020-06-13  9:39       ` Borislav Petkov
2020-06-13 15:48 ` [PATCH -v2] " Borislav Petkov
2020-06-15  6:38   ` [PATCH -v2.1] " Borislav Petkov
2020-06-25  5:51     ` Sean Christopherson
2020-06-25  8:37       ` Borislav Petkov
2020-07-14 12:19     ` Chris Down [this message]
2020-07-14 15:47       ` Borislav Petkov
2020-07-14 16:04         ` Chris Down
2020-07-14 16:46           ` Luck, Tony
2020-07-14 16:58             ` Borislav Petkov
2020-07-14 17:02             ` Chris Down
2020-07-14 16:56           ` Borislav Petkov
2020-07-14 17:04             ` Chris Down
2020-07-14 18:52             ` Srinivas Pandruvada
2020-07-15  4:26               ` Borislav Petkov
2020-07-14 19:17           ` Matthew Garrett
2020-11-17 21:00             ` Mathieu Chouquet-Stringer
2020-11-17 21:20               ` Borislav Petkov
2020-11-18  8:58                 ` Mathieu Chouquet-Stringer
2020-11-18  9:09                 ` Mathieu Chouquet-Stringer
2020-11-18 11:50                   ` Borislav Petkov
2020-11-18 14:04                     ` [PATCH] " Mathieu Chouquet-Stringer
2020-11-18 17:50                       ` Borislav Petkov
2020-11-19 10:53                         ` Mathieu Chouquet-Stringer
2020-11-25 21:41                           ` Mathieu Chouquet-Stringer
2020-11-26 10:03                           ` Borislav Petkov
2020-11-17 21:21               ` [PATCH -v2.1] " Matthew Garrett
2020-11-17 21:22                 ` Matthew Garrett
2020-11-18  9:02                   ` Mathieu Chouquet-Stringer
2020-06-17 15:06 ` [tip: x86/misc] " tip-bot2 for Borislav Petkov
2020-06-25  8:45 ` tip-bot2 for Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200714121955.GA2080@chrisdown.name \
    --to=chris@chrisdown.name \
    --cc=bp@alien8.de \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sean.j.christopherson@intel.com \
    --cc=tony.luck@intel.com \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.