Re: [patch 1/5] genirq: Prepare the handling of shared oneshot interrupts

[Linus Torvalds]

On Wed, Feb 23, 2011 at 3:52 PM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>
> +               /*
> +                * We or the mask lockless. Safe because the code
> +                * which clears the mask is serialized
> +                * vs. IRQ_INPROGRESS.
> +                */
> +               desc->threads_oneshot |= action->thread_mask;
> +               wake_up_process(action->thread);

That comment makes no sense.

What has "code which clears the mask" to do with anything?

Even if another CPU _sets_ a bit, doing so in parallel will lose bits
if it's not locked. One CPU will read the old value, set its bit, and
store the new value: with the race, only one of the bits will be set.

So maybe the code is safe, but the comment is still totally wrong. You
had better be safe not just against people clearing bits, you need to
be safe against ANY OTHER WRITER (clear or set), including very much
OTHER BITS in the same word.

So if it really is safe, comment on ALL the cases.

                     Linus
--
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/