| Message ID | 1237863308.620048.912148989400.1.gpush@pingu (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | 82631f5dd114e52239fb3d1e270a49d37c088b46 |
| Delegated to: | Paul Mackerras |
| Headers | show |
On Tue, 2009-03-24 at 13:55 +1100, Jeremy Kerr wrote: > Currently, we don't enforce any ordering for updates to the lppaca > when enabling dtl logging, so we may end up enabling logging before the > index fields have been established. > > This change adds a smp_wmb() before doing the actual enable. > > Signed-off-by: Jeremy Kerr <jk@ozlabs.org> > > --- > arch/powerpc/platforms/pseries/dtl.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/powerpc/platforms/pseries/dtl.c b/arch/powerpc/platforms/pseries/dtl.c > index dc9b0f8..fafcaa0 100644 > --- a/arch/powerpc/platforms/pseries/dtl.c > +++ b/arch/powerpc/platforms/pseries/dtl.c > @@ -107,6 +107,10 @@ static int dtl_enable(struct dtl *dtl) > /* set our initial buffer indices */ > dtl->last_idx = lppaca[dtl->cpu].dtl_idx = 0; > > + /* ensure that our updates to the lppaca fields have occurred before > + * we actually enable the logging */ > + smp_wmb(); Wouldn't this still be a problem on a UP kernel? cheers
Michael,
> Wouldn't this still be a problem on a UP kernel?
I don't believe so - stores should be ordered with respect to the
current CPU, and in the UP case we still get a barrier().
However, perhaps there are other considerations with the HV that I'm not
aware of. Anyone?
Cheers,
Jeremy
On Tue, 2009-03-24 at 16:59 +1100, Jeremy Kerr wrote: > Michael, > > > Wouldn't this still be a problem on a UP kernel? > > I don't believe so - stores should be ordered with respect to the > current CPU, and in the UP case we still get a barrier(). But what if the CPU decides to do the store to the enable_mask before the stores to the other fields? cheers
diff --git a/arch/powerpc/platforms/pseries/dtl.c b/arch/powerpc/platforms/pseries/dtl.c index dc9b0f8..fafcaa0 100644 --- a/arch/powerpc/platforms/pseries/dtl.c +++ b/arch/powerpc/platforms/pseries/dtl.c @@ -107,6 +107,10 @@ static int dtl_enable(struct dtl *dtl) /* set our initial buffer indices */ dtl->last_idx = lppaca[dtl->cpu].dtl_idx = 0; + /* ensure that our updates to the lppaca fields have occurred before + * we actually enable the logging */ + smp_wmb(); + /* enable event logging */ lppaca[dtl->cpu].dtl_enable_mask = dtl_event_mask;
Currently, we don't enforce any ordering for updates to the lppaca when enabling dtl logging, so we may end up enabling logging before the index fields have been established. This change adds a smp_wmb() before doing the actual enable. Signed-off-by: Jeremy Kerr <jk@ozlabs.org> --- arch/powerpc/platforms/pseries/dtl.c | 4 ++++ 1 file changed, 4 insertions(+)