On Mon, May 16, 2016 at 10:46:38AM +0100, Matt Fleming wrote:
> > >  
> > > - rq->clock_skip_update = 0;
> > > + /* Clear ACT, preserve everything else */
> > > + rq->clock_update_flags ^= RQCF_ACT_SKIP;
> > 
> > The comment says "Clear ACT", but this is really xor, and I am not sure
> > this is even what you want.
>  
> Urgh, you're right. I'm not sure what I was thinking when I wrote
> that.

It happens, ;)
 
> > In addition, would it be simpler to do this?
> > 
> > update_rq_clock()
> >     if (flags & RQCF_ACT_SKIP)
> >             flags <<= 1; /* effective skip is an update */
> >             return;
> > 
> >     flags = RQCF_UPDATED;
> 
> No because if someone calls rq_clock() immediately after __schedule(),
> or even immediately after we clear RQCF_ACT_SKIP in __schedule(), we
> should trigger a warning since the clock has not actually been
> updated.

Well, I don't know how concurrent it can be, but aren't both update
and read synchronized by rq->lock? So I don't understand the latter
case, and the former should be addressed (missing its own update?).

Reply via email to