Messages in this thread | | | Date | Sat, 4 Mar 2006 00:18:34 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem) |
| |
Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote: > > akpm> I'm not sure how to resolve this, really. Worried. Have you > akpm> socialised those changes with architecture maintainers? If so, > akpm> what was the feedback? > > For a short term fix, barrier() might be a safe option. > > > Add an optimization barrier to prevent prefetching jiffies before > incrementing jiffies_64. > > Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp> > > diff --git a/kernel/timer.c b/kernel/timer.c > index fc6646f..fdd12a5 100644 > --- a/kernel/timer.c > +++ b/kernel/timer.c > @@ -925,6 +925,7 @@ static inline void update_times(void) > void do_timer(struct pt_regs *regs) > { > jiffies_64++; > + barrier(); > update_times(); > softlockup_tick(regs); > }
a) Please never ever add any form of barrier without adding a comment explaining why it's there. Unless it's extremely obvious (and it rarely is), it's hard for the reader to work out what on earth it's doing there.
Example? Take a look at the smp_rmb() in ext3_get_inode_block(), work out why it's there. Time yourself.
b) On 64-bit machines jiffies and jiffies_64 always have the same address (don't they?) Is the compiler really going to move a read of an absolute address ahead of a modification of the same address?
<looks>
The address of jiffies isn't known until link time, so yup, the risk is there.
c) jiffies is declared volatile. In practice, if I know my gcc, it's just not going to play these reordering games with a volatile.
If that's true, and if some standard (presumably c99) says that it must be true then I don't think we need the patch.
- 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/
| |