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    [W:0.036 / U:0.096 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site