Re: [PATCH] use a stable clock reference in vdso vgetns

From: Glauber Costa
Date: Wed Oct 13 2010 - 22:45:25 EST


On Wed, Oct 13, 2010 at 06:36:55PM -0700, john stultz wrote:
> On Wed, 2010-10-13 at 11:07 -0300, Glauber Costa wrote:
> > On Fri, Oct 01, 2010 at 10:49:53AM -0700, john stultz wrote:
> > > On Fri, Oct 1, 2010 at 6:09 AM, Glauber Costa <glommer@xxxxxxxxxx> wrote:
> > > > When using vdso services for clock_gettime, we test for the ability
> > > > of a fine-grained measurement through the existance of a vread() function.
> > > >
> > > > However, from the time we test it, to the time we use it, vread() reference
> > > > may not be valid anymore. It happens, for example, when we change the current
> > > > clocksource from one that provides vread (say tsc) to one that lacks it
> > > > (say acpi_pm), in the middle of clock_gettime routine.
> > > >
> > > > seqlock does not really protect us, since readers here won't stop the writers
> > > > to change references. The proposed solution is to grab a copy of the clock
> > > > structure early, and use it as a stable reference onwards.
> > >
> > > Ah. Good find! The fix looks reasonable to me. However, its likely the
> > > similar code in arch/x86/kernel/vsyscall_64.c will need a similar fix.
> > >
> > > Awhile back there was some motivation to merge the two vdso/vsyscall
> > > implementations to avoid the duplication, but my memory is failing on
> > > why that didn't happen. I feel like it had to do with complication
> > > with the way the two implementations are mapped out to userland. Even
> > > so, it seems a shared forced inline method would resolve the issue, so
> > > maybe it just fell off the todo list?
> > News here?
>
> Errr.. Sorry, are you waiting for me to implement this? Or did you want
> a comment on your earlier mail? (Apologies, I've been a bit scattered
> here).
>
np. Comments on the earlier e-mail.
I tried a bit more with that, and sent two other e-mails arguing why I thought
that was the right approach. (well, the second best, just after unification)

> I think trying to unify the two implementations would be nice if you're
> up to trying, but if not, go ahead and push your patch and that will fix
> the bug until I can get around to looking at the unification.
Unfortunately, I have other things under my radar now, so if people
don't see a problem, I'd like to have this particular problem solved.
The patch seems right anyway.

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