On Thu, 18 Oct 2007, Benjamin Herrenschmidt wrote:
>
> + smp_mb();
> while (desc->status & IRQ_INPROGRESS)
> cpu_relax();
So, what exactly does it protect against? At a minimum, this needs a
comment in the changelog, and probably preferably in the source code too.
The thing is, synchronize_irq() can only protect against interrupts that
are *already*running* on another CPU, and the caller must have made sure
that no new interrupts are coming in (or at least that whatever new
interrupts that come in will not pick up a certain piece of data).
So I can imagine that the smb_mb() is there so that the caller - who has
cleared some list or done something like that - will have any preceding
writes that it did be serialized with actually checking the old state of
"desc->status".
Fair enough - I can see that a smp_mb() is useful. But I think it needs
documenting as such, and preferably with an example of how this actually
happened in the first place (do you have one?)
The synchronize_irq() users that are really fundamental (ie the irq
datastructures themselves) all should use the irq descriptor spinlock, and
should *not* be needing any of this, since they would have serialized with
whoever actually set the IRQ_INPROGRESS bit in the first place.
So in *that* sense, I think the memory barrier is useless, and I can't
make up my mind whether it's good or bad. Which is why I'd really like to
have an example scenario spelled out where it makes a difference..
Ok?
Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
