Re: [RFC] New kernel-message logging API (take 2)

From: Vegard Nossum
Date: Fri Sep 28 2007 - 07:46:42 EST


> If-blocks spanning macros are really dangerous!
>
> E.g. an Ethernet driver may want to do:
>
> kprint_block(&block, "MAC ");
> for (i = 0; i < 6; i++) {
> card->mac[i] = obtain_mac_byte_from_hw(i);
> kprint_block(&block, "%02x", card->mac[i]);
> }
>
> This looks (and should be) innocent, but the actual MAC addres retrieval
> would never be executed if loglevel <= CONFIG_KPRINT_LOGLEVEL_MAX.

Yup. Okay, so it's definitely NOT an option.

> Can't you store the loglevel in the kprint_block and check it in all
> successive kprint_*() macros? If gcc knows it's constant, it can optimize
> the non-wanted code away. As other fields in struct kprint_block cannot be
> constant (they store internal state), you have to split it like:
>
> struct kprint_block {
> int loglevel;
> struct real_kprint_block real; /* internal state */
> }
>
> and pass &block.real() instead of &block to all successive internal functions.
> I haven't tried this, so let's hope gcc is actually smart enough...

It isn't, apparently. Or not with my test, anyway. Either way, it's
probably better not to make those assumptions about or rely too much
on the smartness of the compiler (we don't have *any* guarantees).

The best solution for now is probably to pass the log-level into each
line, as Dick Streefland suggested, though it would lead to a hairier
syntax, or just skip the whole interface for now, as Jan Engelhardt
suggested. Thanks.

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