On Tue, Feb 24, 2015 at 3:58 PM, David Howells [off-list ref] wrote:

I have a situation where I want to do something that boils down to:

        if (test_bit(BIT_A, &p->flags) {
                ...
        } else if (test_bit(BIT_B, &p->flags) {
                ...
        } else if (test_bit(BIT_C, &p->flags) {
                ...
        } else if (test_bit(BIT_D, &p->flags) {
                ...
        } else if (test_bit(BIT_E, &p->flags) {
        }

Note that all bits are in the same unsigned long and I don't necessarily
expect the contents of p->flags to change whilst I'm looking at it.

Since the address parameter for test_bit() contains a 'volatile' keyword, the
compiler will emit a separate load (or bit test) for each condition when in
fact the most efficient way is almost certainly one load and a bunch of
bitwise-and or bit-test-in-register instructions.

Is it worth adding a __test_bit() that doesn't have the volatile?  Or should I
just rewrite it manually as?

        unsigned long flags = p->flags;
        if (test_bit(BIT_A, &flags) {
                ...
        } else if (test_bit(BIT_B, &flags) {
        ...

Or even:

        unsigned long flags = p->flags;
        if ((1 << BIT_A) & flags) {
                ...
        } else if ((1 << BIT_B) & flags) {
        ...

How about using READ_ONCE() and BIT()?

        unsigned long flags = READ_ONCE(p->flags_;
        if (BIT(BIT_A) & flags) {
                ...
        } else ...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds