[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110907091145.GB7725@aftab>
Date: Wed, 7 Sep 2011 11:11:45 +0200
From: Borislav Petkov <bp@...64.org>
To: "Luck, Tony" <tony.luck@...el.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...e.hu>, Borislav Petkov <bp@...64.org>,
Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>
Subject: Re: [PATCH 1/5] x86, mce: rework use of TIF_MCE_NOTIFY
On Wed, Aug 31, 2011 at 06:25:34PM -0400, Luck, Tony wrote:
> From: Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>
Looks ok, just minor nitpicks and spelling fixes below.
> The basic flow of MCE handler is summarized as follows:
> 1) from NMI context:
> check hardware error registers, determine error severity,
> and then panic or request non-NMI context by irq_work() to
> continue the system.
> 2) from (irq) context:
> call non-NMI safe functions,
> wake up loggers and schedule work if required
> 3) from worker thread:
from process context:
> process some time-consuming works like memory poisoning.
Scrub paragraph style:
> TIF_MCE_NOTIFY flag is relatively legacy and have used to do tasks of
TIF_MCE_NOTIFY is a legacy flag and has been used to do 2) and 3) in
> 2) and 3) on the thread context that interrupted by MCE. However now
the context of the process which got interrupted by an MCE.
> use of irq_work() and work-queue is enough for these tasks, so this
> patch removes duplicated tasks in mce_notify_process().
Now that irq_work()/workqueues are used for those tasks, remove
duplicated work from mce_notify_process().
> As the result there is no task to be done in the interrupted context,
, there is no work to be done in the interrupted task's context
> but soon if SRAR is supported there would be some thread-specific thing
handling of Action Required errors.
> for action required. So keep the flag for such possible future use,
> until better mechanism is introduced.
>
> Signed-off-by: Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
>
> This is the combination of two patches by Seto-san - with Boris'
> suggestion to not create a 2-line function mce_memory_failure_process()
> but to leave those inline.
> Message-ID: <4DFB1476.40804@...fujitsu.com>
> [PATCH 6/8] x86, mce: introduce mce_memory_failure_process()
> Message-ID: <4DFB1509.7020402@...fujitsu.com>
> [PATCH 7/8] x86, mce: rework use of TIF_MCE_NOTIFY
You need a double "Link: http://lkml.kernel.org/r/..." tag here.
>
> arch/x86/kernel/cpu/mcheck/mce.c | 35 ++++++++++++++++-------------------
> 1 files changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
> index 08363b0..91bb983 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce.c
> @@ -1037,8 +1037,9 @@ void do_machine_check(struct pt_regs *regs, long error_code)
> if (kill_it && tolerant < 3)
> force_sig(SIGBUS, current);
>
> - /* notify userspace ASAP */
> - set_thread_flag(TIF_MCE_NOTIFY);
> + /* Trap this thread before returning to user, for action required */
> + if (worst == MCE_AR_SEVERITY)
> + set_thread_flag(TIF_MCE_NOTIFY);
>
> if (worst > 0)
> mce_report_event(regs);
> @@ -1052,31 +1053,29 @@ EXPORT_SYMBOL_GPL(do_machine_check);
> /* dummy to break dependency. actual code is in mm/memory-failure.c */
> void __attribute__((weak)) memory_failure(unsigned long pfn, int vector)
> {
> - printk(KERN_ERR "Action optional memory failure at %lx ignored\n", pfn);
> + pr_err("Action optional memory failure at %lx ignored\n", pfn);
> }
>
> /*
> - * Called after mce notification in process context. This code
> - * is allowed to sleep. Call the high level VM handler to process
> - * any corrupted pages.
> - * Assume that the work queue code only calls this one at a time
> - * per CPU.
> - * Note we don't disable preemption, so this code might run on the wrong
> - * CPU. In this case the event is picked up by the scheduled work queue.
> - * This is merely a fast path to expedite processing in some common
> - * cases.
> + * Called in process context that interrupted by MCE and marked with
> + * TIF_MCE_NOTFY, just before returning to errorneous userland.
Polish expression:
"Called in process context which got interrupted by an MCE and marked
with TIF_MCE_NOTIFY, just before..."
> + * This code is allowed to sleep.
> + * Attempt possible recovery such as calling the high level VM handler to
> + * process any corrupted pages, and kill/signal current process if required.
This last sentence needs to go over mce_process_work() below.
> */
> void mce_notify_process(void)
> {
> - unsigned long pfn;
> - mce_notify_irq();
> - while (mce_ring_get(&pfn))
> - memory_failure(pfn, MCE_VECTOR);
> + clear_thread_flag(TIF_MCE_NOTIFY);
> +
> + /* TBD: do recovery for action required event */
> }
>
> static void mce_process_work(struct work_struct *dummy)
> {
> - mce_notify_process();
> + unsigned long pfn;
> +
> + while (mce_ring_get(&pfn))
> + memory_failure(pfn, MCE_VECTOR);
> }
>
> #ifdef CONFIG_X86_MCE_INTEL
> @@ -1157,8 +1156,6 @@ int mce_notify_irq(void)
> /* Not more than two messages every minute */
> static DEFINE_RATELIMIT_STATE(ratelimit, 60*HZ, 2);
>
> - clear_thread_flag(TIF_MCE_NOTIFY);
> -
> if (test_and_clear_bit(0, &mce_need_notify)) {
> /* wake processes polling /dev/mcelog */
> wake_up_interruptible(&mce_chrdev_wait);
Thanks.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists