Re: [PATCH v7 1/5] x86/mce: Add wrapper for struct mce to export vendor specific info
From: Naik, Avadhut <hidden>
Date: 2024-10-30 16:35:23
Also in:
linux-edac, lkml
On 10/30/2024 08:32, Borislav Petkov wrote:
On Tue, Oct 22, 2024 at 07:36:27PM +0000, Avadhut Naik wrote:quoted
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 beUnknown word [vendor-specifc] in commit message. Please introduce a spellchecker into your patch creation workflow.
Will fix this.
quoted hunk ↗ jump to hunk
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();
Ack for the suggestions. -- Thanks, Avadhut Naik