All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Ashok Raj <ashok.raj@intel.com>, Borislav Petkov <bp@alien8.de>
Cc: X86-kernel <x86@kernel.org>,
	LKML Mailing List <linux-kernel@vger.kernel.org>,
	Ashok Raj <ashok.raj@intel.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Tony Luck <tony.luck@intel.com>,
	alison.schofield@intel.com, reinette.chatre@intel.com
Subject: Re: [Patch V1 1/7] x86/microcode/intel: Remove redundant microcode rev pr_info()s
Date: Fri, 02 Dec 2022 19:58:42 +0100	[thread overview]
Message-ID: <87cz91pr8d.ffs@tglx> (raw)
In-Reply-To: <20221129210832.107850-2-ashok.raj@intel.com>

Ashok!

On Tue, Nov 29 2022 at 13:08, Ashok Raj wrote:
> There is a pr_info() to dump information about newly loaded microcode.

There... Somewhere, right?

> The code intends this pr_info() to be just once, but the check to ensure
> is racy. Unfortunately this happens quite often in with this new change
> resulting in multiple redundant prints on the console.

-ENOPARSE. Can you try to express that in coherent sentences please?

> microcode_init()->schedule_on_each_cpu(setup_online_cpu)->collect_cpu_info
>
> [   33.688639] microcode: sig=0x50654, pf=0x80, revision=0x2006e05
> [   33.688659] microcode: sig=0x50654, pf=0x80, revision=0x2006e05
> [   33.688660] microcode: sig=0x50654, pf=0x80, revision=0x2006e05
>
> There is already a pr_info() in microcode/core.c as shown below:
>
> microcode: Reload completed, microcode revision: 0x2b000041 -> 0x2b000070

There are quite some pr_info()'s in microcode/core.c...

$function_name() prints the new and the previous microcode revision once
when the load has completed:

  microcode: Reload completed, microcode revision: 0x2b000041 -> 0x2b000070

Hmm?

> The sig and pf aren't that useful to end user, they are available via

The sig and pf ?!? Come on, you really can do better.

> /proc/cpuinfo and this never changes between microcode loads.
>
> Remove the redundant pr_info() and the racy single print checks. This
> removes the race entirely, zap the duplicated pr_info() spam and
> simplify the code.

The last sentence does not qualify as coherent either.

Other than that. Nice cleanup.

Thanks,

        tglx

  reply	other threads:[~2022-12-02 18:59 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-29 21:08 [Patch V1 0/7] x86/microcode: Some cleanups and fixes for microcode Ashok Raj
2022-11-29 21:08 ` [Patch V1 1/7] x86/microcode/intel: Remove redundant microcode rev pr_info()s Ashok Raj
2022-12-02 18:58   ` Thomas Gleixner [this message]
2022-12-03  0:26     ` Ashok Raj
2022-12-03 13:42       ` Borislav Petkov
2022-11-29 21:08 ` [Patch V1 2/7] x86/microcode/intel: Remove retries on early microcode load Ashok Raj
2022-12-02 19:01   ` Thomas Gleixner
2022-12-03  1:48     ` Ashok Raj
2022-12-02 23:53   ` Sohil Mehta
2022-12-03  1:47     ` Ashok Raj
2022-12-05 12:18   ` Borislav Petkov
2022-12-05 16:42     ` Ashok Raj
2022-12-05 20:53   ` [tip: x86/microcode] x86/microcode/intel: Do not retry microcode reloading on the APs tip-bot2 for Ashok Raj
2022-11-29 21:08 ` [Patch V1 3/7] x86/microcode/core: Move microcode_check() to cpu/microcode/core.c Ashok Raj
2022-12-02 19:02   ` Thomas Gleixner
2022-12-02 20:57   ` Sohil Mehta
2022-12-03  0:21     ` Ashok Raj
2022-12-05 16:25   ` Borislav Petkov
2022-12-05 17:05     ` Ashok Raj
2022-12-05 21:27       ` Borislav Petkov
2022-11-29 21:08 ` [Patch V1 4/7] x86/microcode/core: Take a snapshot before and after applying microcode Ashok Raj
2022-12-02 19:09   ` Thomas Gleixner
2022-12-03  1:57     ` Ashok Raj
2022-12-07 20:25   ` Borislav Petkov
2022-12-08  0:05     ` Ashok Raj
2022-11-29 21:08 ` [Patch V1 5/7] x86/microcode/intel: Prepare the print_ucode_rev to simply take a rev to print Ashok Raj
2022-12-02 19:23   ` Thomas Gleixner
2022-11-29 21:08 ` [Patch V1 6/7] x86/microcode/intel: Print old and new rev during early boot Ashok Raj
2022-12-02 19:29   ` Thomas Gleixner
2022-11-29 21:08 ` [Patch V1 7/7] x86/microcode/intel: Print when early microcode loading fails Ashok Raj
2022-12-02 19:30   ` Thomas Gleixner
2022-12-05 18:19     ` Ashok Raj

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=87cz91pr8d.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=alison.schofield@intel.com \
    --cc=ashok.raj@intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=reinette.chatre@intel.com \
    --cc=tony.luck@intel.com \
    --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.