[lkml]   [2012]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>
    On Mon, 2012-03-19 at 18:28 -0700, wrote:
    > On Mon, 19 Mar 2012, Ted Ts'o wrote:
    > > On Mon, Mar 19, 2012 at 10:44:10AM -0700, Joe Perches wrote:
    > >> This stuff ain't whitespace.
    > >>> I'll have to respectfully disagree with you. For ext4, as far as I am
    > > concerned, changing printk(KERN_INFO, ...) to pr_info(...) is *purely*
    > > a whitespace-level change.
    > Joe, if everything did change, what difference would it make?

    Hi David.

    > just being a consistant style doesn't matter much,

    Perhaps we disagree on the value of consistency.
    I believe it's a small but measurable effect and it
    can reduce overall ongoing defect rates.

    Is it DoublePlus_important_? No, definitely not.

    To me using pr_<level> is a bit like using const
    or marking sections devinitconst.

    It hardly matters, but it's good form and it can
    free up some working memory in ram limited systems.

    > but if there is some
    > functionality that would be possible with pr_info(...) that would not be
    > possible with printk(KERN_INFO, ...), there may be more reason to change.

    Right now, it's just macros over printk so it's pretty
    trivial. I do intend to convert pr_<level> macros to
    functions eventually to reduce code size ~.5% overall.
    That reduction does depend on quantity of CONFIG_<FOO>
    options enabled of course. Enable everything, I think
    it's ~.01%. I haven't done it in quite awhile though
    so that's a guess.

    It matters a tiny bit more for flash or ram limited

    Some driver optimizations like the rtlwifi reduction
    in -next commit 481b9606ec might have more of an
    impact though for those systems.

    So, it depends...

    cheers, Joe

     \ /
      Last update: 2012-03-20 02:53    [W:0.020 / U:9.084 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site