lkml.org 
[lkml]   [2012]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2] printk: ignore recursion_bug flag in HW error handle process
    From
    Date
    On Thu, 2012-05-24 at 15:56 -0700, Andrew Morton wrote:
    > On Thu, 24 May 2012 08:11:45 +0200
    > Borislav Petkov <bp@alien8.de> wrote:
    >
    > > > +/* HW error handle status helpers */
    > > > +extern atomic_t hw_error;
    > > > +static inline void hw_error_enter(void)
    > > > +{
    > > > + atomic_inc(&hw_error);
    > > > +}
    > > > +
    > > > +static inline void hw_error_exit(void)
    > > > +{
    > > > + atomic_dec(&hw_error);
    > > > +}
    > > > +
    > > > +static inline int in_hw_error(void)
    > > > +{
    > > > + return atomic_read(&hw_error);
    > > > +}
    > >
    > > Shouldn't those be generic empty functions and each arch implement their
    > > own with the stuff they want to do on the respective architecture when
    > > they get a hardware error?
    >
    > This code needs documentation.
    >
    > Specifically, it should clearly explain (and hence define) what a
    > "hardware error" *is*, and for what purpose this code exists.
    >
    > Because as it stands, this interface is hopelessly vague. Once one
    > sees that it is *specifically* used for handling mce within a printk,
    > it all makes sense.
    >
    > And with that understanding comes the realisation that the interface is
    > poorly named. It will not be used for any purpose other than adjusting
    > printk() behavior so it should mention printk() in its name and in its
    > comments and probably it should all be moved into printk.h.
    >
    > Futhermore, this code is not really related to MCE or hardware or
    > anything else. It is simply a way in which callers can suppress
    > printk()'s recursion check. Callers are free to use it for reasons
    > other than "hardware errors".
    >
    > And once all that is done, and this interface becomes part of printk()
    > then no, there is no need to add per-arch hooks. An arch can call into
    > printk_recursion_check_disable() and printk_recursion_chack_enable() -
    > nice and simple.
    >
    >
    > IOW, the title of this patch should be
    >
    > [patch 1/2] printk: add interface for disabling recursion check
    > [patch 2/2] x86 mce: use new printk recursion disabling interface
    Andrew,

    Thanks for the detailed comment. It's more reasonable to bind it to printk.
    We would follow it to create new patches.

    Yanmin




    \
     
     \ /
      Last update: 2012-05-25 02:41    [W:0.026 / U:31.716 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site