Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
From:   Stephane Eranian <eranian@google.com>
Date:   Fri, 17 Jun 2022 14:08:52 +0300
Message-ID: <CABPqkBRVF9qmxKFgmjZpzN3tx=U+_8udECMLHs7BrtzfPwmuhQ@mail.gmail.com>
Subject: [RFC] pr_warn_once() issue in x86 MSR extable code
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Eric Dumazet <edumazet@google.com>, jpoimboe@redhat.com,
        Steven Rostedt <rostedt@goodmis.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi,

Some changes to the way invalid MSR accesses are reported by the kernel is
causing some problems with messages printed on the console.

We have seen several cases of ex_handler_msr() printing invalid MSR
accesses once but
the callstack multiple times causing confusion on the console.

The last time the exception MSR code was modified (5.16) by PeterZ was:

  d52a7344bdfa x86/msr: Remove .fixup usage:

  if (!safe && wrmsr &&  pr_warn_once("unchecked MSR access error: ..."))
               show_stack_regs(regs);

Note that this code pattern was also present, though in a different
form, before this commit.

The problem here is that another earlier commit (5.13):

a358f40600b3 once: implement DO_ONCE_LITE for non-fast-path "do once"
functionality

Modifies all the pr_*_once() calls to always return true claiming that
no caller is ever
checking the return value of the functions.

This is why we are seeing the callstack printed without the associated
printk() msg.

I believe that having the pr_*_once() functions return true the first
time they are called
is useful especially when extra information, such as callstack, must
be printed to help
track the origin of the problem.

The exception handling code seems to be the only place where the
return value is checked
for pr_warn_once(). A minimal change would be to create another
version of that function
that calls DO_ONCE() instead of DO_ONCE_LITE(), e.g., pr_warn_once_return().

I can post a patch to that effect if we all agree on the approach.

Thanks.