[patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments - Kernel

This is a discussion on [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments - Kernel ; Hi, This patch series introduces WARN(), which is a WARN_ON() variant that takes printk() like arguments. This was done after both akpm and I hit several cases where this would have been useful; in addition, with WARN(), we put the ...

+ Reply to Thread
Page 1 of 2 1 2 LastLast
Results 1 to 20 of 22

Thread: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

  1. [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

    Hi,

    This patch series introduces WARN(), which is a WARN_ON() variant that
    takes printk() like arguments. This was done after both akpm and I hit
    several cases where this would have been useful; in addition, with
    WARN(), we put the printk string inside the -----[ cut here ]-----
    section, making it more likely that users (and tools like kerneloops)
    pick up the message in addition to just the bare WARNING.

    The first few patches have been in -mm for a long time; the later ones
    are newer and introduce more users of WARN().

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. [patch 12/17] Use WARN() in block/

    From: Arjan van de Ven
    Subject: Use WARN instead of printk+WARN_ON in block

    Use WARN() instead of a printk+WARN_ON() pair; this way the message
    becomes part of the warning section for better reporting/collection.

    Signed-off-by: Arjan van de Ven ---
    block/as-iosched.c | 3 +--
    1 file changed, 1 insertion(+), 2 deletions(-)

    Index: linux.trees.git/block/as-iosched.c
    ================================================== =================
    --- linux.trees.git.orig/block/as-iosched.c
    +++ linux.trees.git/block/as-iosched.c
    @@ -837,8 +837,7 @@ static void as_completed_request(struct
    WARN_ON(!list_empty(&rq->queuelist));

    if (RQ_STATE(rq) != AS_RQ_REMOVED) {
    - printk("rq->state %d\n", RQ_STATE(rq));
    - WARN_ON(1);
    + WARN(1, "rq->state %d\n", RQ_STATE(rq));
    goto out;
    }

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [patch 2/17] Add a WARN() macro that acts like WARN_ON()+printk

    On Tue, 2008-07-08 at 09:40 -0700, Arjan van de Ven wrote:
    > +#ifndef WARN
    > +#define WARN(condition, format...) ({ \
    > + int __ret_warn_on = !!(condition); \
    > + if (unlikely(__ret_warn_on)) \
    > + __WARN_printf(format); \
    > + unlikely(__ret_warn_on); \
    > +})
    > +#endif
    > +


    If all current uses of WARN are going to change, perhaps
    adding an argument for KERN_ or removing the
    KERN_ prefixes and standardizing on a single
    KERN_ (KERN_WARNING?) is appropriate.

    If not standardizing on a single KERN_ prefix,
    perhaps change the conditional and using something like:

    assert_(cond, fmt, arg...)
    or
    assert_msg(cond, level, fmt, arg...)


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [patch 2/17] Add a WARN() macro that acts like WARN_ON()+printk

    On Tue, 08 Jul 2008 11:00:05 -0700
    Joe Perches wrote:

    > On Tue, 2008-07-08 at 09:40 -0700, Arjan van de Ven wrote:
    > > +#ifndef WARN
    > > +#define WARN(condition, format...)
    > > ({ \
    > > + int __ret_warn_on
    > > = !!(condition); \
    > > + if
    > > (unlikely(__ret_warn_on)) \
    > > +
    > > __WARN_printf(format); \
    > > +
    > > unlikely(__ret_warn_on); \
    > > +}) +#endif
    > > +

    >
    > If all current uses of WARN are going to change, perhaps
    > adding an argument for KERN_ or removing the
    > KERN_ prefixes and standardizing on a single
    > KERN_ (KERN_WARNING?) is appropriate.


    I looked at this and there are various levels in use today, I don't
    think we can standardize on one unfortunately.
    I don't think there's a real problem; WARN() really acts like printk...
    all the way.



    If you want to reach me at my work email, use arjan@linux.intel.com
    For development, discussion and tips for power savings,
    visit http://www.lesswatts.org
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments


    * Arjan van de Ven wrote:

    > Hi,
    >
    > This patch series introduces WARN(), which is a WARN_ON() variant that
    > takes printk() like arguments. This was done after both akpm and I hit
    > several cases where this would have been useful; in addition, with
    > WARN(), we put the printk string inside the -----[ cut here ]-----
    > section, making it more likely that users (and tools like kerneloops)
    > pick up the message in addition to just the bare WARNING.
    >
    > The first few patches have been in -mm for a long time; the later ones
    > are newer and introduce more users of WARN().


    i've created a new -git based topic branch in tip/core/warn-API and
    picked up your patches:

    Arjan van de Ven (16):
    full tree: clear the WARN() namespace
    core: add a WARN() macro that acts like WARN_ON()+printk
    kobject: introduce WARN() usage in the kobject code
    irq: use WARN() in kernel/irq/manage.c
    mm: use WARN() in mm/vmalloc.c
    lockdep: use WARN() in kernel/lockdep.c
    irq: use WARN() in kernel/irq/chip.c
    x86: use WARN() in arch/x86/mm/ioremap.c
    x86: use WARN() in arch/x86/mm/pageattr.c
    x86: use WARN() in arch/x86/kernel
    block: use WARN() in block/
    driver core: use WARN() in drivers/base/
    core libs: use WARN() in lib/
    fs: use WARN() in fs/
    sysfs: use WARN() in fs/sysfs
    procfs: use WARN() in fs/proc/

    it can be picked up via:

    git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core/warn-API

    ( Note, i added a few more tree-wide namespace preparation fixes to the
    first patch - a few more came in since you first prepared this patch i
    guess. )

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

    On Wed, 9 Jul 2008 12:13:48 +0200 Ingo Molnar wrote:

    > > Hi,
    > >
    > > This patch series introduces WARN(), which is a WARN_ON() variant that
    > > takes printk() like arguments. This was done after both akpm and I hit
    > > several cases where this would have been useful; in addition, with
    > > WARN(), we put the printk string inside the -----[ cut here ]-----
    > > section, making it more likely that users (and tools like kerneloops)
    > > pick up the message in addition to just the bare WARNING.
    > >
    > > The first few patches have been in -mm for a long time; the later ones
    > > are newer and introduce more users of WARN().

    >
    > i've created a new -git based topic branch in tip/core/warn-API and
    > picked up your patches:


    um, why?

    If you merge this into linux-next then it will trash already-merged patches
    in -mm and, more particularly, it will trash other trees which you aren't
    looking at, causing Stephen problems.

    The way to merge this code is to get the base patches into mainline and
    then trickle the dependent patches into subsystem trees, or direct into
    mainline after the subsystem trees have merged, and with suitable acks.

    You aren't set up to do that?
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments


    * Andrew Morton wrote:

    > > > The first few patches have been in -mm for a long time; the later
    > > > ones are newer and introduce more users of WARN().

    > >
    > > i've created a new -git based topic branch in tip/core/warn-API and
    > > picked up your patches:

    >
    > um, why?
    >
    > If you merge this into linux-next then it will trash already-merged
    > patches in -mm and, more particularly, it will trash other trees which
    > you aren't looking at, causing Stephen problems.


    no, i didnt plan to push this towards linux-next - given the broad
    consensus and given the wide spread of the changes.

    I wanted to wait with this until the end of the merge window and keep it
    tested and merged up nicely. I.e. zero maintenance overhead to
    subsystems.

    > The way to merge this code is to get the base patches into mainline
    > and then trickle the dependent patches into subsystem trees, or direct
    > into mainline after the subsystem trees have merged, and with suitable
    > acks.
    >
    > You aren't set up to do that?


    i think it's better to just go through the merge window i believe, and
    then do this atomically in one correct and tested step, when all
    subsystem trees are at their minimum size and there's virtually no
    collisions.

    Note that this situation is special: this is a patchset that has
    virtually no functionality side-effects, and hence can be done 100%
    correctly, i thought the atomic step was the right approach.

    For anything semantically meaningful i too would do the spread-out
    gradual approach (and i'm presently doing that for a number of topics).

    But if you'd like to do this the spread-out way then sure, and i will
    drop this tree. ( if you do that then please import the commits from
    tip/core/warn-API, i fixed a couple of of typos in the commit messages
    and did some merging and extensions as well. The tree also passed a fair
    amount of testing meanwhile as well. )

    Anyway, your call.

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

    On Wed, 9 Jul 2008 13:37:03 +0200 Ingo Molnar wrote:

    >
    > * Andrew Morton wrote:
    >
    > > > > The first few patches have been in -mm for a long time; the later
    > > > > ones are newer and introduce more users of WARN().
    > > >
    > > > i've created a new -git based topic branch in tip/core/warn-API and
    > > > picked up your patches:

    > >
    > > um, why?
    > >
    > > If you merge this into linux-next then it will trash already-merged
    > > patches in -mm and, more particularly, it will trash other trees which
    > > you aren't looking at, causing Stephen problems.

    >
    > no, i didnt plan to push this towards linux-next - given the broad
    > consensus and given the wide spread of the changes.


    Well, there was no way for me (or, I believe, Arjan or anyone else) to have
    worked this out from your reply.

    > I wanted to wait with this until the end of the merge window and keep it
    > tested and merged up nicely. I.e. zero maintenance overhead to
    > subsystems.


    That's an option. That's why I will cc the relevant subsystem maintainers
    on the commits, and will collect and maintain the acked-by's.

    > > The way to merge this code is to get the base patches into mainline
    > > and then trickle the dependent patches into subsystem trees, or direct
    > > into mainline after the subsystem trees have merged, and with suitable
    > > acks.
    > >
    > > You aren't set up to do that?

    >
    > i think it's better to just go through the merge window i believe, and
    > then do this atomically in one correct and tested step, when all
    > subsystem trees are at their minimum size and there's virtually no
    > collisions.


    Probably.

    > Note that this situation is special: this is a patchset that has
    > virtually no functionality side-effects, and hence can be done 100%
    > correctly, i thought the atomic step was the right approach.
    >
    > For anything semantically meaningful i too would do the spread-out
    > gradual approach (and i'm presently doing that for a number of topics).
    >
    > But if you'd like to do this the spread-out way then sure, and i will
    > drop this tree. ( if you do that then please import the commits from
    > tip/core/warn-API, i fixed a couple of of typos in the commit messages
    > and did some merging and extensions as well. The tree also passed a fair
    > amount of testing meanwhile as well. )
    >
    > Anyway, your call.


    Well I haven't got onto processing these patches in detail yet. An open
    questions is why the damn thing was resubmitted from scratch when I've
    already merged it and fixed various rejects and had to fix several bugs in
    it. Do those rejects need to be re-fixed? Were my bugfixes folded back?
    I haven't looked yet. I'll need to generate the incremental diff and see
    what was done.

    But if what you've merged was against mainline then it isn't terribly
    useful.

    Hopefully this sort of thing won't happen as much once I get -mm into
    linx-next. Soon...

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments


    * Andrew Morton wrote:

    > > Note that this situation is special: this is a patchset that has
    > > virtually no functionality side-effects, and hence can be done 100%
    > > correctly, i thought the atomic step was the right approach.
    > >
    > > For anything semantically meaningful i too would do the spread-out
    > > gradual approach (and i'm presently doing that for a number of
    > > topics).
    > >
    > > But if you'd like to do this the spread-out way then sure, and i
    > > will drop this tree. ( if you do that then please import the commits
    > > from tip/core/warn-API, i fixed a couple of of typos in the commit
    > > messages and did some merging and extensions as well. The tree also
    > > passed a fair amount of testing meanwhile as well. )
    > >
    > > Anyway, your call.

    >
    > Well I haven't got onto processing these patches in detail yet. An
    > open questions is why the damn thing was resubmitted from scratch when
    > I've already merged it and fixed various rejects and had to fix
    > several bugs in it. Do those rejects need to be re-fixed? Were my
    > bugfixes folded back? I haven't looked yet. I'll need to generate the
    > incremental diff and see what was done.
    >
    > But if what you've merged was against mainline then it isn't terribly
    > useful.


    i cross-ported it from the linux-next base to -git base and the conflict
    fallout was minimal, well below 5% or so. I.e. the patches change
    long-existent warnings that have not changed in linux-next either.

    About 30% of the changes in this patchset affect subsystems that i
    co-maintain, still tip/core/warn-API did a pure, conflict-free git-merge
    when it was applied ontop of all these subsystem trees:

    commit 99eb83efbe2e3c12d26be22a032495ccddb36a2c
    Merge: de67579... c2e0195...
    Author: Ingo Molnar
    Date: Wed Jul 9 12:14:48 2008 +0200

    Merge branch 'core/warn-API'

    Hence my suggestion that this should be maintained and forward ported to
    the end of the merge window, in a separate, standalone tree that is -git
    based.

    Anyway, no strong feelings, it's your call.

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

    On Wed, 9 Jul 2008 04:46:13 -0700
    Andrew Morton wrote:
    > Well I haven't got onto processing these patches in detail yet. An
    > open questions is why the damn thing was resubmitted from scratch
    > when I've already merged it and fixed various rejects and had to fix
    > several bugs in it. Do those rejects need to be re-fixed? Were my
    > bugfixes folded back? I haven't looked yet. I'll need to generate
    > the incremental diff and see what was done.


    I started with the version in -mm (and had to do one reject fix against
    -next) with folding the 2 -fix's into it.

    --
    If you want to reach me at my work email, use arjan@linux.intel.com
    For development, discussion and tips for power savings,
    visit http://www.lesswatts.org
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [patch 13/17] Use WARN() in drivers/base/

    On Tue, 8 Jul 2008 09:53:07 -0700 Arjan van de Ven wrote:

    > Signed-off-by: Arjan van de Ven Index: linux.trees.git/drivers/base/core.c


    A number of these patches had mangled signed-off-by: lines.

    Please try to be consistent in the presence and placement of the ^--- line
    at the end of the changelog.

    I verified that all three copies of "Use WARN() in fs/" were the same.

    I've decided that I don't like the whole thing This:

    #define WARN(condition, format...) ({ \
    int __ret_warn_on = !!(condition); \
    if (unlikely(__ret_warn_on)) \
    __WARN_printf(format); \
    unlikely(__ret_warn_on); \
    })

    is not a WARN(). It is a WARN_ON() function. The use of this name now leaves
    us no sensible name under which to implement

    #define WARN(format...)


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [patch 2/17] Add a WARN() macro that acts like WARN_ON()+printk

    On Tue, 8 Jul 2008 09:40:23 -0700 Arjan van de Ven wrote:

    > From: Arjan van de Ven
    >
    > Add a WARN() macro that acts like WARN_ON(), with the added feature that it
    > takes a printk like argument that is printed as part of the warning message.
    >


    Apart from a little whitespace tweak, this is identical to what I already had.

    > +#define WARN_ONCE(condition, format...) ({ \
    > + static int __warned; \
    > + int __ret_warn_once = !!(condition); \
    > + \
    > + if (unlikely(__ret_warn_once)) \
    > + if (WARN(!__warned, format)) \
    > + __warned = 1; \
    > + unlikely(__ret_warn_once); \
    > +})


    Except it adds this operation, without describing it at all in the
    changelog.

    Is this some brainfart, or am I missing something? I can see some sense in
    a WARN_ONCE(format...), but not in a WARN_ONCE() which takes a `condition'
    and should be called WARN_ON_ONCE(), which we already have.

    As it appears that you didn't add any users of WARN_ONCE(), I shall
    delicately step away from this patch.

    More care, please?
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [patch 1/17] Clear the WARN() namespace

    On Tue, 8 Jul 2008 09:39:30 -0700 Arjan van de Ven wrote:

    > From: Arjan van de Ven
    >
    > We want to use WARN() as a variant of WARN_ON(), however a few drivers are
    > using WARN() internally. This patch renames these to WARNING() to avoid the
    > namespace clash. A few cases were defining but not using the thing, for those
    > cases I just deleted the definition.
    >
    > Signed-off-by: Arjan van de Ven
    > Acked-by: Greg KH
    > Cc: Karsten Keil
    > Signed-off-by: Andrew Morton
    > ---
    >
    > drivers/isdn/hisax/st5481.h | 4
    > drivers/isdn/hisax/st5481_b.c | 4
    > drivers/isdn/hisax/st5481_d.c | 6
    > drivers/isdn/hisax/st5481_usb.c | 18 +-
    > drivers/usb/gadget/at91_udc.h | 2
    > drivers/usb/gadget/cdc2.c | 2
    > drivers/usb/gadget/ether.c | 2
    > drivers/usb/gadget/file_storage.c | 14 -
    > drivers/usb/gadget/fsl_usb2_udc.c | 2
    > drivers/usb/gadget/fsl_usb2_udc.h | 2
    > drivers/usb/gadget/gmidi.c | 2
    > drivers/usb/gadget/goku_udc.c | 2
    > drivers/usb/gadget/goku_udc.h | 2
    > drivers/usb/gadget/inode.c | 2
    > drivers/usb/gadget/net2280.c | 2
    > drivers/usb/gadget/net2280.h | 2
    > drivers/usb/gadget/omap_udc.c | 6
    > drivers/usb/gadget/omap_udc.h | 2
    > drivers/usb/gadget/printer.c | 2
    > drivers/usb/gadget/pxa25x_udc.c | 6
    > drivers/usb/gadget/pxa25x_udc.h | 2
    > drivers/usb/gadget/u_ether.c | 3
    > drivers/usb/host/isp116x-hcd.c | 2
    > drivers/usb/host/isp116x.h | 2
    > drivers/usb/host/sl811-hcd.c | 2
    > drivers/usb/host/sl811.h | 2
    > drivers/usb/misc/usbtest.c | 4
    > include/linux/usb/composite.h | 2


    After fixing rejects, this appears to be identical to what I already had.


    > usr/share/quilt/refresh | 304 --------------------------------------


    Except for this curosity, which I rather hope wasn't supposed to be there.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [patch 13/17] Use WARN() in drivers/base/

    On Fri, 11 Jul 2008 12:20:11 -0700
    Andrew Morton wrote:

    > On Tue, 8 Jul 2008 09:53:07 -0700 Arjan van de Ven
    > wrote:
    >
    > > Signed-off-by: Arjan van de Ven Index:
    > > linux.trees.git/drivers/base/core.c

    >
    > A number of these patches had mangled signed-off-by: lines.
    >
    > Please try to be consistent in the presence and placement of the ^---
    > line at the end of the changelog.
    >
    > I verified that all three copies of "Use WARN() in fs/" were the same.
    >
    > I've decided that I don't like the whole thing This:
    >
    > #define WARN(condition, format...)
    > ({ \ int __ret_warn_on
    > = !!(condition); \ if
    > (unlikely(__ret_warn_on)) \
    > __WARN_printf(format); \
    > unlikely(__ret_warn_on); \ })
    >
    > is not a WARN(). It is a WARN_ON() function. The use of this name
    > now leaves us no sensible name under which to implement
    >


    I'm totally open to a better name.
    Having a condition in there is really nice, it means we can fold the
    if() into it in many cases. Just like BUG_ON() did.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  15. Re: [patch 2/17] Add a WARN() macro that acts like WARN_ON()+printk

    On Fri, 11 Jul 2008 12:19:49 -0700
    Andrew Morton wrote:

    > On Tue, 8 Jul 2008 09:40:23 -0700 Arjan van de Ven
    > wrote:
    >
    > > From: Arjan van de Ven
    > >
    > > Add a WARN() macro that acts like WARN_ON(), with the added feature
    > > that it takes a printk like argument that is printed as part of the
    > > warning message.
    > >

    >
    > Apart from a little whitespace tweak, this is identical to what I
    > already had.
    >
    > > +#define WARN_ONCE(condition, format...)
    > > ({ \
    > > + static int
    > > __warned; \
    > > + int __ret_warn_once
    > > = !!(condition); \
    > > + \
    > > + if
    > > (unlikely(__ret_warn_once)) \
    > > + if (WARN(!__warned, format))
    > > \
    > > + __warned =
    > > 1; \
    > > + unlikely(__ret_warn_once); \
    > > +})

    >
    > Except it adds this operation, without describing it at all in the
    > changelog.
    >
    > Is this some brainfart, or am I missing something? I can see some
    > sense in a WARN_ONCE(format...), but not in a WARN_ONCE() which takes
    > a `condition' and should be called WARN_ON_ONCE(), which we already
    > have.


    WARN_ON_ONCE() doesn't take printk arguments. So WARN_ONCE() is
    WAR_ON_ONCE() with printk arguments...


    --
    If you want to reach me at my work email, use arjan@linux.intel.com
    For development, discussion and tips for power savings,
    visit http://www.lesswatts.org
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: [patch 13/17] Use WARN() in drivers/base/

    On Fri, 11 Jul 2008 13:54:09 -0700 Arjan van de Ven wrote:

    > On Fri, 11 Jul 2008 12:20:11 -0700
    > Andrew Morton wrote:
    >
    > > On Tue, 8 Jul 2008 09:53:07 -0700 Arjan van de Ven
    > > wrote:
    > >
    > > > Signed-off-by: Arjan van de Ven Index:
    > > > linux.trees.git/drivers/base/core.c

    > >
    > > A number of these patches had mangled signed-off-by: lines.
    > >
    > > Please try to be consistent in the presence and placement of the ^---
    > > line at the end of the changelog.
    > >
    > > I verified that all three copies of "Use WARN() in fs/" were the same.
    > >
    > > I've decided that I don't like the whole thing This:
    > >
    > > #define WARN(condition, format...)
    > > ({ \ int __ret_warn_on
    > > = !!(condition); \ if
    > > (unlikely(__ret_warn_on)) \
    > > __WARN_printf(format); \
    > > unlikely(__ret_warn_on); \ })
    > >
    > > is not a WARN(). It is a WARN_ON() function. The use of this name
    > > now leaves us no sensible name under which to implement
    > >

    >
    > I'm totally open to a better name.
    > Having a condition in there is really nice, it means we can fold the
    > if() into it in many cases. Just like BUG_ON() did.


    Alexey's WARN_IF() would suit, I guess. Plain old "WARN" is just wrong
    here, alas.

    I can just edit all the diffs if we're all OK with that.



    I don't suppose there's any way of tricking the preprocessor into
    supporting

    WARN_ON(foo == 42);

    as well as

    WARN_ON(foo == 42, "bite me!");


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: [patch 13/17] Use WARN() in drivers/base/

    On Fri, 11 Jul 2008 15:11:10 -0700
    Andrew Morton wrote:

    > On Fri, 11 Jul 2008 13:54:09 -0700 Arjan van de Ven
    > wrote:
    >
    > > On Fri, 11 Jul 2008 12:20:11 -0700
    > > Andrew Morton wrote:
    > >
    > > > On Tue, 8 Jul 2008 09:53:07 -0700 Arjan van de Ven
    > > > wrote:
    > > >
    > > > > Signed-off-by: Arjan van de Ven Index:
    > > > > linux.trees.git/drivers/base/core.c
    > > >
    > > > A number of these patches had mangled signed-off-by: lines.
    > > >
    > > > Please try to be consistent in the presence and placement of the
    > > > ^--- line at the end of the changelog.
    > > >
    > > > I verified that all three copies of "Use WARN() in fs/" were the
    > > > same.
    > > >
    > > > I've decided that I don't like the whole thing This:
    > > >
    > > > #define WARN(condition, format...)
    > > > ({ \ int __ret_warn_on
    > > > = !!(condition); \ if
    > > > (unlikely(__ret_warn_on)) \
    > > > __WARN_printf(format); \
    > > > unlikely(__ret_warn_on);
    > > > \ })
    > > >
    > > > is not a WARN(). It is a WARN_ON() function. The use of this
    > > > name now leaves us no sensible name under which to implement
    > > >

    > >
    > > I'm totally open to a better name.
    > > Having a condition in there is really nice, it means we can fold the
    > > if() into it in many cases. Just like BUG_ON() did.

    >
    > Alexey's WARN_IF() would suit, I guess. Plain old "WARN" is just
    > wrong here, alas.
    >
    > I can just edit all the diffs if we're all OK with that.


    I'm ok with that.

    >
    >
    >
    > I don't suppose there's any way of tricking the preprocessor into
    > supporting
    >
    > WARN_ON(foo == 42);
    >
    > as well as
    >
    > WARN_ON(foo == 42, "bite me!");


    there might be some obscure GCC extensions, but to be honest I fear the
    worst ;(


    --
    If you want to reach me at my work email, use arjan@linux.intel.com
    For development, discussion and tips for power savings,
    visit http://www.lesswatts.org
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  18. Re: [patch 13/17] Use WARN() in drivers/base/

    On Fri, 11 Jul 2008 15:11:10 -0700
    Andrew Morton wrote:

    >
    > I don't suppose there's any way of tricking the preprocessor into
    > supporting
    >
    > WARN_ON(foo == 42);
    >
    > as well as
    >
    > WARN_ON(foo == 42, "bite me!");
    >


    after reading preprocessor docs from gcc and trying some things:
    We can do this. It comes at a price: the price is a blank line in the
    WARN trace for the "no printk comments" case, and we lose the ability
    to override the printk level. (which you can argue is a feature by just
    setting it to KERN_WARNING).

    (and some interesting but otherwise non-harmful preprocessor stuff in
    headers)

    Is this is price worth paying to not have a second macro?

    --
    If you want to reach me at my work email, use arjan@linux.intel.com
    For development, discussion and tips for power savings,
    visit http://www.lesswatts.org
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  19. Re: [patch 13/17] Use WARN() in drivers/base/

    On Fri, 11 Jul 2008 15:51:05 -0700 Arjan van de Ven wrote:

    > On Fri, 11 Jul 2008 15:11:10 -0700
    > Andrew Morton wrote:
    >
    > >
    > > I don't suppose there's any way of tricking the preprocessor into
    > > supporting
    > >
    > > WARN_ON(foo == 42);
    > >
    > > as well as
    > >
    > > WARN_ON(foo == 42, "bite me!");
    > >

    >
    > after reading preprocessor docs from gcc and trying some things:
    > We can do this. It comes at a price: the price is a blank line in the
    > WARN trace for the "no printk comments" case, and we lose the ability
    > to override the printk level. (which you can argue is a feature by just
    > setting it to KERN_WARNING).
    >
    > (and some interesting but otherwise non-harmful preprocessor stuff in
    > headers)


    the blank line: might be avoidable by doing some extra work at runtime
    to recognise its presence?

    overriding facility level: doesn't sound very useful, as WARN()'s
    stack-trace's facility level is not controllable.

    > Is this is price worth paying to not have a second macro?


    Dunno, how ugly is the patch?

    It would be rather nice to not go and fatten the interface. Would
    there be additional text or data size costs?
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  20. Re: [patch 13/17] Use WARN() in drivers/base/


    > I don't suppose there's any way of tricking the preprocessor into
    > supporting
    >
    > WARN_ON(foo == 42);
    >
    > as well as
    >
    > WARN_ON(foo == 42, "bite me!");


    Here's one that abuses variadic macros and limits the number of
    arguments to 19.

    #include
    #include

    #define cnt(y...) _cnt( , ## y)
    #define _cnt(y...) __cnt(y,19,18,17,16,15,14,13,12,11,10,9,8,7,6,5,4, 3,2,1,0)
    #define __cnt(x0,x1,x2,x3,x4,x5,x6,x7,x8,x9,x10,x11,x12,x1 3,x14,x15,x16,x17,x18,x19,n,ys...) n

    static void warn_on_slowpath(int argcount, ...)
    {
    va_list args;
    char *fmt;

    // print beginning of warning

    if (argcount) {
    va_start(args, count);
    fmt = va_arg(args, char *);
    vprintf(fmt, args);
    va_end(args);
    }

    // print rest of warning
    }

    #define WARN_ON(test, fmt...) \
    do { \
    if (test) { \
    warn_on_slowpath(cnt(fmt) , ## fmt); \
    printf("WARN\n"); \
    } \
    } while (0)

    int main(void)
    {
    WARN_ON(1);
    WARN_ON(1, "asdf %d\n", 7);

    return 0;
    }


    johannes

    -----BEGIN PGP SIGNATURE-----
    Comment: Johannes Berg (powerbook)

    iQIcBAABAgAGBQJId+hhAAoJEKVg1VMiehFYvl8QAKiW6Ay6ny k6IzSfHTSYAi+g
    JiB2cxW9O8qYNWKyZ2hMuHP5woyoFis13ZPaYc45QiugHslo2O PfEFueOdqTG2Kp
    TRAMoDPbGe+a2NLShBuR/DFFbzyHOLnunoKbOYBVLTi3025gfxmc5F27QZMM9YNJ
    6HUiTHQJANDZQ5utBscALrSuabF6tsfoby9STQlzYiLkzCDj97 6I+Iz3GyfT2iPG
    2UHLWqcdsalo2QaLsmqTk17ao9fCoR1Xwj8ZMrmttngJH28rYr +838N6ab7zEnY2
    n0RmB1Gfs29C5tjXPrc788wc1lKowXm2lvAvM/0aST5azY1aBv5mu9rvvJk0uMAB
    18LVXt2/4yih+C8xo0VbloDuVS6FalPmjdpSLeKtUNu4mRv4DJ/xgk+NSBy4iL9w
    K4SiayfY4YL0GpUL6miqRNgVi8Eignqphon1KLYRauJHjixuwi W+HpNPQnIh0aGT
    xnifxe9HK6v/us83cTDQJcTBOZcbiwkkTP6ATfYbLnfBJ4+4NA6jTLRSKXVZv9 99
    tlEZNDQ+oDciMDv1cRg6aoQ3ig/b6S+6SMY/R3DURSSclMnRX/zX3vvhqXufJECV
    iD/gn6LAosTmiozT9fhVm9uEVhLvjvuYIg51J+8L+zE+imuHkrjpG/h5k79GpYF4
    a3UNdZ54JYGDNDj9TMHv
    =lv5Y
    -----END PGP SIGNATURE-----


+ Reply to Thread
Page 1 of 2 1 2 LastLast