lkml.org 
[lkml]   [2009]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] x86: Fix hwpoison code related build failure on 32-bit NUMAQ

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Sat, 26 Sep 2009, Ingo Molnar wrote:
> > >
> > > +config X86_SUPPORTS_MEMORY_FAILURE
> > > + bool
> > > + depends on !X86_NUMAQ
> > > + select ARCH_SUPPORTS_MEMORY_FAILURE
> > > + default y
> >
> > Thanks Linus, this patch fixed the NUMAQ build problem.
>
> I think it's slightly buggy still.
>
> I think the X86_SUPPORTS_MEMORY_FAILURE thing should also have a
>
> depends on X86_MCE
>
> line, because we still depend on MCE.

Yeah.

( Note, it should not necessarily depend on it: while the only hw
mechanism that calls memory_failure() is indeed MCE, the act of having
a memory-failures subsystem does not depend on the presence of an x86
MCE subsystem. There's for example the injection debug-code which
allows the injection of memory_failure() calls. That should work fine
without having MCE build in as well.

But that is a separate change. )

> And as you found out, there's also the sparsemem thing.
>
> Don't make it one huge ugly thing, just split out the requirements like
>
> depends on X86_MCE
> depends on !X86_NUMAQ
> depends on X86_64 || !SPARSEMEM
>
> because I think the requirements are fairly independent, and it makes
> it easier to read (you could even comment each line on why _that_
> particular issue needs to disable X86_SUPPORTS_MEMORY_FAILURE)

Good idea, have done that too.

> But yeah, with that, and some testing, please add my sign-off (or
> acked-by, if you end up changing the patch so much that it has little
> to do with my original one)

It's still mostly your patch so i've added your SOB, thanks. Below is
the updated patch.

btw., i think mm/memory-failure.c needs similar cleanups to
ARCH_SUPPORTS_MEMORY_FAILURE.

Right now it is full of x86 details, not sure that is right. 'MCE' for
example is an x86 expression and goes way beyond just memory errors - it
stands for 'Machine Check Exception' and covers IO/bus errors, etc.

We even put 'MCE' into new ABI details in include/asm-generic/siginfo.h:

/* hardware memory error consumed on a machine check: action required */
#define BUS_MCEERR_AR (__SI_FAULT|4)
/* hardware memory error detected in process but not consumed: action optional*/
#define BUS_MCEERR_AO (__SI_FAULT|5)
#define NSIGBUS 5

That should be fixed to be something like:

BUS_MEMERR_MANDATORY
BUS_MEMERR_OPTIONAL

before such a kernel is released, IMHO.

In mm/memory-failure.c i find the 'ao' / 'ar' abbreviations rather
unreadable as well - they are totally meaningless (to me at least),
even if i know the code.

Same goes for the me_*() prefixes.

Also, 'struct to_kill' ....

Ingo


\
 
 \ /
  Last update: 2009-09-26 20:13    [W:0.049 / U:4.408 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site