linux-kernel - Re: [PATCH 5/9] HWPoison: add memory_failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 17 May 2011 10:46:22 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Huang Ying <ying.huang@...el.com>
Cc:	Len Brown <lenb@...nel.org>, linux-kernel@...r.kernel.org,
	Andi Kleen <andi@...stfloor.org>,
	Tony Luck <tony.luck@...el.com>, 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()

* 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...

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?

Thanks,

	Ingo
--
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/