lkml.org 
[lkml]   [2011]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@jp.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@jp.fujitsu.com>
> Signed-off-by: Tony Luck <tony.luck@intel.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@jp.fujitsu.com>
> [PATCH 6/8] x86, mce: introduce mce_memory_failure_process()
> Message-ID: <4DFB1509.7020402@jp.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


\
 
 \ /
  Last update: 2011-09-07 18:21    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site