[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4DD23750.3030606@intel.com>
Date: Tue, 17 May 2011 16:52:32 +0800
From: Huang Ying <ying.huang@...el.com>
To: Ingo Molnar <mingo@...e.hu>
CC: Len Brown <lenb@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Andi Kleen <andi@...stfloor.org>,
"Luck, Tony" <tony.luck@...el.com>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
Andi Kleen <ak@...ux.intel.com>,
"Wu, Fengguang" <fengguang.wu@...el.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH 5/9] HWPoison: add memory_failure_queue()
On 05/17/2011 04:46 PM, Ingo Molnar wrote:
>
> * Huang Ying <ying.huang@...el.com> wrote:
>
>> memory_failure() is the entry point for HWPoison memory error
>> recovery. It must be called in process context. But commonly
>> hardware memory errors are notified via MCE or NMI, so some delayed
>> execution mechanism must be used. In MCE handler, a work queue + ring
>> buffer mechanism is used.
>>
>> In addition to MCE, now APEI (ACPI Platform Error Interface) GHES
>> (Generic Hardware Error Source) can be used to report memory errors
>> too. To add support to APEI GHES memory recovery, a mechanism similar
>> to that of MCE is implemented. memory_failure_queue() is the new
>> entry point that can be called in IRQ context. The next step is to
>> make MCE handler uses this interface too.
>>
>> Signed-off-by: Huang Ying <ying.huang@...el.com>
>> Cc: Andi Kleen <ak@...ux.intel.com>
>> Cc: Wu Fengguang <fengguang.wu@...el.com>
>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>> ---
>> include/linux/mm.h | 1
>> mm/memory-failure.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 93 insertions(+)
>
> I have to say i disagree with how this is designed and how this is exposed to
> user-space - and i pointed this out before.
>
> It's up to Len whether you muck up drivers/acpi/ but here you are patching mm/
> again ...
>
> I just had a quick look into the current affairs of mm/memory-inject.c and it
> has become an *even* nastier collection of hacks since the last time i
> commented on its uglies.
>
> Special hack upon special hack, totally disorganized code, special-purpose,
> partly ioctl driven opaque information extraction to user-space using the
> erst-dbg device interface. We have all the maintenance overhead and little of
> the gains from hw error event features...
Like the name suggested, erst-dbg is only for debugging. It is not a
user space interface. The user space interface used by APEI now is printk.
> In this patch you add:
>
> +struct memory_failure_entry {
> + unsigned long pfn;
> + int trapno;
> + int flags;
> +};
>
> Instead of exposing this event to other users who might be interested in these
> events - such as the RAS daemon under development by Boris.
>
> We have a proper framework (ring-buffer, NMI execution, etc.) for reporting
> events, why are you not using (and extending) it instead of creating this nasty
> looking, isolated, ACPI specific low level feature?
This patch has nothing to do with hardware error event reporting. It is
just about hardware error recovering.
Best Regards,
Huang Ying
--
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