On Tue, Oct 22, 2024 at 07:36:27PM +0000, Avadhut Naik wrote:
> Currently, exporting new additional machine check error information
> involves adding new fields for the same at the end of the struct mce.
> This additional information can then be consumed through mcelog or
> tracepoint.
>
> However, as new MSRs are being added (and will be added in the future)
> by CPU vendors on their newer CPUs with additional machine check error
> information to be exported, the size of struct mce will balloon on some
> CPUs, unnecessarily, since those fields are vendor-specific. Moreover,
> different CPU vendors may export the additional information in varying
> sizes.
>
> The problem particularly intensifies since struct mce is exposed to
> userspace as part of UAPI. It's bloating through vendor-specific data
> should be avoided to limit the information being sent out to userspace.
>
> Add a new structure mce_hw_err to wrap the existing struct mce. The same
> will prevent its ballooning since vendor-specifc data, if any, can now be
Unknown word [vendor-specifc] in commit message.
Please introduce a spellchecker into your patch creation workflow.
Also:
The tip-tree preferred ordering of variable declarations at the
beginning of a function is reverse fir tree order::
struct long_struct_name *descriptive_name;
unsigned long foo, bar;
unsigned int tmp;
int ret;
The above is faster to parse than the reverse ordering::
int ret;
unsigned int tmp;
unsigned long foo, bar;
struct long_struct_name *descriptive_name;
And even more so than random ordering::
unsigned long foo, bar;
int ret;
struct long_struct_name *descriptive_name;
unsigned int tmp;
diff ontop of yours:
---
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 3611366d56b7..28e28b69d84d 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1030,11 +1030,11 @@ static noinstr int mce_timed_out(u64 *t, const char
*msg)
*/
static void mce_reign(void)
{
- int cpu;
struct mce_hw_err *err = NULL;
struct mce *m = NULL;
int global_worst = 0;
char *msg = NULL;
+ int cpu;
/*
* This CPU is the Monarch and the other CPUs have run
@@ -1291,8 +1291,8 @@ __mc_scan_banks(struct mce_hw_err *err, struct pt_regs
*regs,
{
struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array);
struct mca_config *cfg = &mca_cfg;
- struct mce *m = &err->m;
int severity, i, taint = 0;
+ struct mce *m = &err->m;
for (i = 0; i < this_cpu_read(mce_num_banks); i++) {
arch___clear_bit(i, toclear);
@@ -1419,8 +1419,8 @@ static void kill_me_never(struct callback_head *cb)
static void queue_task_work(struct mce_hw_err *err, char *msg, void
(*func)(struct callback_head *))
{
- struct mce *m = &err->m;
int count = ++current->mce_count;
+ struct mce *m = &err->m;
/* First call, save all the details */
if (count == 1) {
diff --git a/arch/x86/kernel/cpu/mce/genpool.c
b/arch/x86/kernel/cpu/mce/genpool.c
index 504d89724ecd..d0be6dda0c14 100644
--- a/arch/x86/kernel/cpu/mce/genpool.c
+++ b/arch/x86/kernel/cpu/mce/genpool.c
@@ -73,9 +73,9 @@ struct llist_node *mce_gen_pool_prepare_records(void)
void mce_gen_pool_process(struct work_struct *__unused)
{
- struct mce *mce;
- struct llist_node *head;
struct mce_evt_llist *node, *tmp;
+ struct llist_node *head;
+ struct mce *mce;
head = llist_del_all(&mce_event_llist);
if (!head)
diff --git a/arch/x86/kernel/cpu/mce/inject.c b/arch/x86/kernel/cpu/mce/inject.c
index c65a5c4e2f22..313fe682db33 100644
--- a/arch/x86/kernel/cpu/mce/inject.c
+++ b/arch/x86/kernel/cpu/mce/inject.c
@@ -502,9 +502,9 @@ static void prepare_msrs(void *info)
static void do_inject(void)
{
+ unsigned int cpu = i_mce.extcpu;
struct mce_hw_err err;
u64 mcg_status = 0;
- unsigned int cpu = i_mce.extcpu;
u8 b = i_mce.bank;
i_mce.tsc = rdtsc_ordered();
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
