lkml.org 
[lkml]   [2011]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 07/10] MCE: replace mce.c use of TIF_MCE_NOTIFY with user_return_notifier
    (2011/06/10 6:35), Luck, Tony wrote:
    > From: Tony Luck <tony.luck@intel.com>
    >
    > Ingo wrote:
    >> We already have a generic facility to do such things at
    >> return-to-userspace: _TIF_USER_RETURN_NOTIFY.
    >
    > This just a proof of concept patch ... before this can become
    > real the user-return-notifier code would have to be made NMI
    > safe (currently it uses hlist_add_head/hlist_del, which would
    > need to be changed to Ying's NMI-safe single threaded lists).
    >
    > Reviewed-by: Avi Kivity <avi@redhat.com>
    > NOT-Signed-off-by: Tony Luck <tony.luck@intel.com>
    > ---
    > arch/x86/Kconfig | 1 +
    > arch/x86/kernel/cpu/mcheck/mce.c | 47 +++++++++++++++++++++++++++----------
    > arch/x86/kernel/signal.c | 6 -----
    > 3 files changed, 35 insertions(+), 19 deletions(-)
    >
    > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
    > index cc6c53a..054e127 100644
    > --- a/arch/x86/Kconfig
    > +++ b/arch/x86/Kconfig
    > @@ -845,6 +845,7 @@ config X86_REROUTE_FOR_BROKEN_BOOT_IRQS
    >
    > config X86_MCE
    > bool "Machine Check / overheating reporting"
    > + select USER_RETURN_NOTIFIER
    > ---help---
    > Machine Check support allows the processor to notify the
    > kernel if it detects a problem (e.g. overheating, data corruption).
    > diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
    > index ffc8d11..28d223e 100644
    > --- a/arch/x86/kernel/cpu/mcheck/mce.c
    > +++ b/arch/x86/kernel/cpu/mcheck/mce.c
    > @@ -38,6 +38,7 @@
    > #include <linux/mm.h>
    > #include <linux/debugfs.h>
    > #include <linux/edac_mce.h>
    > +#include <linux/user-return-notifier.h>
    >
    > #include <asm/processor.h>
    > #include <asm/hw_irq.h>
    > @@ -69,6 +70,15 @@ atomic_t mce_entry;
    >
    > DEFINE_PER_CPU(unsigned, mce_exception_count);
    >
    > +struct mce_notify {
    > + struct user_return_notifier urn;
    > + bool registered;
    > +};
    > +
    > +static void mce_do_notify(struct user_return_notifier *urn);
    > +
    > +static DEFINE_PER_CPU(struct mce_notify, mce_notify);
    > +
    > /*
    > * Tolerant levels:
    > * 0: always panic on uncorrected errors, log corrected errors
    > @@ -947,6 +957,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
    > int i;
    > int worst = 0;
    > int severity;
    > + struct mce_notify *np;
    > /*
    > * Establish sequential order between the CPUs entering the machine
    > * check handler.
    > @@ -1099,7 +1110,12 @@ void do_machine_check(struct pt_regs *regs, long error_code)
    > force_sig(SIGBUS, current);
    >
    > /* notify userspace ASAP */
    > - set_thread_flag(TIF_MCE_NOTIFY);
    > + np = &__get_cpu_var(mce_notify);
    > + if (np->registered == 0) {
    > + np->urn.on_user_return = mce_do_notify;
    > + user_return_notifier_register(&np->urn);
    > + np->registered = 1;
    > + }
    >
    > if (worst > 0)
    > mce_report_event(regs);
    > @@ -1116,28 +1132,35 @@ void __attribute__((weak)) memory_failure(unsigned long pfn, int vector)
    > printk(KERN_ERR "Action optional memory failure at %lx ignored\n", pfn);
    > }
    >
    > +static void mce_process_ring(void)
    > +{
    > + unsigned long pfn;
    > +
    > + mce_notify_irq();
    > + while (mce_ring_get(&pfn))
    > + memory_failure(pfn, MCE_VECTOR);
    > +}
    > +
    > /*
    > * 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.
    > */
    > -void mce_notify_process(void)
    > +static void mce_do_notify(struct user_return_notifier *urn)
    > {
    > - unsigned long pfn;
    > - mce_notify_irq();
    > - while (mce_ring_get(&pfn))
    > - memory_failure(pfn, MCE_VECTOR);
    > + struct mce_notify *np = container_of(urn, struct mce_notify, urn);
    > +
    > + user_return_notifier_unregister(urn);
    > + np->registered = 0;
    > +
    > + mce_process_ring();
    > }

    Now I'm reconsidering the MCE event notification mechanism.
    One of something nervous is whether it is really required to process
    "_AO" memory poisoning (i.e. mce_process_ring()) here in a process
    context that unfortunately interrupted by MCE (or preempted after that).
    I'm uncertain how long walking though the task_list for unmap will takes,
    and not sure it is acceptable if the unlucky thread is a kind of latency
    sensitive...

    If we can move mce_process_ring() to worker thread completely, what
    we have to do will be:
    1) from NMI context, request non-NMI context by irq_work()
    2) from (irq) context, wake up loggers and schedule work if required
    3) from worker thread, process "_AO" memory poisoning etc.

    So now question is why user_return_notifier is needed here.
    Is it just an alternative of irq_work() for !LOCAL_APIC ?


    Thanks,
    H.Seto

    >
    > static void mce_process_work(struct work_struct *dummy)
    > {
    > - mce_notify_process();
    > + mce_process_ring();
    > }
    >
    > #ifdef CONFIG_X86_MCE_INTEL
    > @@ -1218,8 +1241,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_up_interruptible(&mce_wait);
    >
    > diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
    > index 4fd173c..44efc22 100644
    > --- a/arch/x86/kernel/signal.c
    > +++ b/arch/x86/kernel/signal.c
    > @@ -838,12 +838,6 @@ static void do_signal(struct pt_regs *regs)
    > void
    > do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags)
    > {
    > -#ifdef CONFIG_X86_MCE
    > - /* notify userspace of pending MCEs */
    > - if (thread_info_flags & _TIF_MCE_NOTIFY)
    > - mce_notify_process();
    > -#endif /* CONFIG_X86_64 && CONFIG_X86_MCE */
    > -
    > /* deal with pending signal delivery */
    > if (thread_info_flags & _TIF_SIGPENDING)
    > do_signal(regs);



    \
     
     \ /
      Last update: 2011-06-10 10:11    [W:0.035 / U:0.264 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site