Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

2010-03-24 Thread Michael Ellerman
On Sat, 2010-03-20 at 23:05 +, Ben Hutchings wrote:
 WARN() is used in some places to report firmware or hardware bugs that
 are then worked-around.  These bugs do not affect the stability of the
 kernel and should not set the usual TAINT_WARN flag.  To allow for
 this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint
 flag as argument.
..
 The architecture-specific changes here are untested and need to be
 reviewed by architecture maintainers.

I'm not one of them, but this at least builds on powerpc FWIW.

cheers


signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

2010-03-23 Thread Paul Mundt
On Sat, Mar 20, 2010 at 11:05:40PM +, Ben Hutchings wrote:
 WARN() is used in some places to report firmware or hardware bugs that
 are then worked-around.  These bugs do not affect the stability of the
 kernel and should not set the usual TAINT_WARN flag.  To allow for
 this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint
 flag as argument.
 
 Architectures that implement warnings using trap instructions instead
 of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint)
 instead of __WARN().
 
 Signed-off-by: Ben Hutchings b...@decadent.org.uk
 ---
 The architecture-specific changes here are untested and need to be
 reviewed by architecture maintainers.
 
I'm a bit confused about how this is supposed to work, the TAINT_xxx
values are bit positions presently from 0 to 10, while BUGFLAG_xxx are
ranged from 0 up. You've set up BUGFLAG_TAINT() to that the TAINT_xxx
value is shifted up 8 bits but neglected the fact that the trap type is
16-bits on most (all?) of the platforms using trap-based BUG handling.

If the 'taint' in question is just the TAINT_xxx value by itself and will
never be a bitmap then that's fine, but there's certainly not enough room
to pass the bitmap in on top of the bugflag otherwise (I don't know if
this is your intention or not though).

Also note that some platforms (like SH) implement additional bugflags, so
we at least want to keep the lower byte available for architecture
private use.

Having said that, the current patch does work for me, although I'm a bit
nervous about someone thinking it's ok to pass in a taint bitmap here.

Tested-by: Paul Mundt let...@linux-sh.org
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

2010-03-23 Thread Ben Hutchings
On Mon, 2010-03-22 at 22:47 -0400, Andrew Morton wrote:
 On Sat, 20 Mar 2010 23:05:40 + Ben Hutchings b...@decadent.org.uk wrote:
 
  WARN() is used in some places to report firmware or hardware bugs that
  are then worked-around.  These bugs do not affect the stability of the
  kernel and should not set the usual TAINT_WARN flag.  To allow for
  this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint
  flag as argument.
  
  Architectures that implement warnings using trap instructions instead
  of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint)
  instead of __WARN().
 
 When you say they must now implement, I assume that you mean that
 they _do_ now implement, and that no additional architecture work is
 needed.

Right, I believe I fixed-up all the current architectures.  There might
be more architectures out there, unmerged as yet.

Ben.

-- 
Ben Hutchings
Any smoothly functioning technology is indistinguishable from a rigged demo.


signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

2010-03-23 Thread Ben Hutchings
On Tue, 2010-03-23 at 16:45 +0900, Paul Mundt wrote:
 On Sat, Mar 20, 2010 at 11:05:40PM +, Ben Hutchings wrote:
  WARN() is used in some places to report firmware or hardware bugs that
  are then worked-around.  These bugs do not affect the stability of the
  kernel and should not set the usual TAINT_WARN flag.  To allow for
  this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint
  flag as argument.
  
  Architectures that implement warnings using trap instructions instead
  of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint)
  instead of __WARN().
  
  Signed-off-by: Ben Hutchings b...@decadent.org.uk
  ---
  The architecture-specific changes here are untested and need to be
  reviewed by architecture maintainers.
  
 I'm a bit confused about how this is supposed to work, the TAINT_xxx
 values are bit positions presently from 0 to 10, while BUGFLAG_xxx are
 ranged from 0 up. You've set up BUGFLAG_TAINT() to that the TAINT_xxx
 value is shifted up 8 bits but neglected the fact that the trap type is
 16-bits on most (all?) of the platforms using trap-based BUG handling.
 
 If the 'taint' in question is just the TAINT_xxx value by itself and will
 never be a bitmap then that's fine, but there's certainly not enough room
 to pass the bitmap in on top of the bugflag otherwise (I don't know if
 this is your intention or not though).

Yes, the taint value must be a bit number not a flag.  Sloppy wording on
my part.

 Also note that some platforms (like SH) implement additional bugflags, so
 we at least want to keep the lower byte available for architecture
 private use.

I noticed, that's why I started at 8 not 1.

 Having said that, the current patch does work for me, although I'm a bit
 nervous about someone thinking it's ok to pass in a taint bitmap here.

We can maybe use BUILD_BUG_ON() here as the taint bit is already
required to be a compile-time constant.

Ben.

 Tested-by: Paul Mundt let...@linux-sh.org
 

-- 
Ben Hutchings
Any smoothly functioning technology is indistinguishable from a rigged demo.


signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

2010-03-22 Thread Andrew Morton
On Sat, 20 Mar 2010 23:05:40 + Ben Hutchings b...@decadent.org.uk wrote:

 WARN() is used in some places to report firmware or hardware bugs that
 are then worked-around.  These bugs do not affect the stability of the
 kernel and should not set the usual TAINT_WARN flag.  To allow for
 this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint
 flag as argument.
 
 Architectures that implement warnings using trap instructions instead
 of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint)
 instead of __WARN().

When you say they must now implement, I assume that you mean that
they _do_ now implement, and that no additional architecture work is
needed.

 The architecture-specific changes here are untested and need to be
 reviewed by architecture maintainers.

That would be nice.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

2010-03-21 Thread Ben Hutchings
On Sun, 2010-03-21 at 20:10 +0100, Andi Kleen wrote:
 Ben Hutchings b...@decadent.org.uk writes:
 
  WARN() is used in some places to report firmware or hardware bugs that
  are then worked-around.  These bugs do not affect the stability of the
  kernel and should not set the usual TAINT_WARN flag.  To allow for
  this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint
  flag as argument.
 
  Architectures that implement warnings using trap instructions instead
  of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint)
  instead of __WARN().
 
 I guess this should enforce that at least some taint flag is set?
 (e.g. with a BUILD_BUG_ON)

I'm being a bit sloppy with the wording here.  The TAINT_* macros are
actually bit numbers, not flags.  I could define a TAINT_MAX and add:

BUILD_BUG_ON(taint  0 || taint  TAINT_MAX);

Not sure that that's really worth doing though.

Ben.

-- 
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.


signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

2010-03-21 Thread Andi Kleen
Ben Hutchings b...@decadent.org.uk writes:

 WARN() is used in some places to report firmware or hardware bugs that
 are then worked-around.  These bugs do not affect the stability of the
 kernel and should not set the usual TAINT_WARN flag.  To allow for
 this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint
 flag as argument.

 Architectures that implement warnings using trap instructions instead
 of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint)
 instead of __WARN().

I guess this should enforce that at least some taint flag is set?
(e.g. with a BUILD_BUG_ON)

-Andi

-- 
a...@linux.intel.com -- Speaking for myself only.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

2010-03-20 Thread Ben Hutchings
WARN() is used in some places to report firmware or hardware bugs that
are then worked-around.  These bugs do not affect the stability of the
kernel and should not set the usual TAINT_WARN flag.  To allow for
this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint
flag as argument.

Architectures that implement warnings using trap instructions instead
of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint)
instead of __WARN().

Signed-off-by: Ben Hutchings b...@decadent.org.uk
---
The architecture-specific changes here are untested and need to be
reviewed by architecture maintainers.

Ben.

 arch/parisc/include/asm/bug.h  |8 
 arch/powerpc/include/asm/bug.h |6 +++---
 arch/s390/include/asm/bug.h|8 
 arch/sh/include/asm/bug.h  |4 ++--
 include/asm-generic/bug.h  |   34 --
 kernel/panic.c |   24 
 lib/bug.c  |2 +-
 7 files changed, 66 insertions(+), 20 deletions(-)

diff --git a/arch/parisc/include/asm/bug.h b/arch/parisc/include/asm/bug.h
index 75e46c5..72cfdb0 100644
--- a/arch/parisc/include/asm/bug.h
+++ b/arch/parisc/include/asm/bug.h
@@ -44,7 +44,7 @@
 #endif
 
 #ifdef CONFIG_DEBUG_BUGVERBOSE
-#define __WARN()   \
+#define __WARN_TAINT(taint)\
do {\
asm volatile(\n   \
 1:\t PARISC_BUG_BREAK_ASM \n   \
@@ -54,11 +54,11 @@
 \t.org 2b+%c3\n  \
 \t.popsection\
 : : i (__FILE__), i (__LINE__),\
-i (BUGFLAG_WARNING), \
+i (BUGFLAG_TAINT(taint)),\
 i (sizeof(struct bug_entry)) );  \
} while(0)
 #else
-#define __WARN()   \
+#define __WARN_TAINT(taint)\
do {\
asm volatile(\n   \
 1:\t PARISC_BUG_BREAK_ASM \n   \
@@ -67,7 +67,7 @@
 \t.short %c0\n   \
 \t.org 2b+%c1\n  \
 \t.popsection\
-: : i (BUGFLAG_WARNING), \
+: : i (BUGFLAG_TAINT(taint)),\
 i (sizeof(struct bug_entry)) );  \
} while(0)
 #endif
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 2c15212..065c590 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -85,12 +85,12 @@
}   \
 } while (0)
 
-#define __WARN() do {  \
+#define __WARN_TAINT(taint) do {   \
__asm__ __volatile__(   \
1: twi 31,0,0\n   \
_EMIT_BUG_ENTRY \
: : i (__FILE__), i (__LINE__), \
- i (BUGFLAG_WARNING),\
+ i (BUGFLAG_TAINT(taint)),   \
  i (sizeof(struct bug_entry)));  \
 } while (0)
 
@@ -104,7 +104,7 @@
1: PPC_TLNEI %4,0\n \
_EMIT_BUG_ENTRY \
: : i (__FILE__), i (__LINE__), \
- i (BUGFLAG_WARNING),\
+ i (BUGFLAG_TAINT(TAINT_WARN)),  \
  i (sizeof(struct bug_entry)),   \
  r (__ret_warn_on)); \
}   \
diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index 9beeb9d..bf90d1f 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -46,18 +46,18 @@
unreachable();  \
 } while (0)
 
-#define __WARN() do {  \
-   __EMIT_BUG(BUGFLAG_WARNING);\
+#define __WARN_TAINT(taint) do {   \
+   __EMIT_BUG(BUGFLAG_TAINT(taint));   \
 } while (0)
 
 #define WARN_ON(x) ({  \
int __ret_warn_on = !!(x);  \