lkml.org 
[lkml]   [2015]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 08/14] hrtimer: Allow hrtimer::function() to free the timer
From
Date
В Ср, 10/06/2015 в 18:04 +0200, Oleg Nesterov пишет:
> Hi Kirill,
>
> On 06/10, Kirill Tkhai wrote:
> >
> > В Вт, 09/06/2015 в 23:33 +0200, Oleg Nesterov пишет:
> > >
> > > hrtimer_active(timer)
> > > {
> > >
> > > do {
> > > base = READ_ONCE(timer->base->cpu_base);
> > > seq = read_seqcount_begin(&cpu_base->seq);
> > >
> > > if (timer->state & ENQUEUED ||
> > > base->running == timer)
> > > return true;
> > >
> > > } while (read_seqcount_retry(&cpu_base->seq, seq) ||
> > > base != READ_ONCE(timer->base->cpu_base));
> > >
> > > return false;
> > > }
> > >
> > > And we need to avoid the races with 2 transitions in __run_hrtimer().
> > >
> > > The first race is trivial, we change __run_hrtimer() to do
> > >
> > > write_seqcount_begin(cpu_base->seq);
> > > cpu_base->running = timer;
> > > __remove_hrtimer(timer); // clears ENQUEUED
> > > write_seqcount_end(cpu_base->seq);
> >
> > We use seqcount, because we are afraid that hrtimer_active() may miss
> > timer->state or cpu_base->running, when we are clearing it.
>
> Yes,
>
> > If we use two pairs of write_seqcount_{begin,end} in __run_hrtimer(),
> > we may protect only the places where we do that:
> >
> > cpu_base->running = timer;
> > write_seqcount_begin(cpu_base->seq);
> > __remove_hrtimer(timer); // clears ENQUEUED
> > write_seqcount_end(cpu_base->seq);
> >
> > ....
> >
> > timer->state |= HRTIMER_STATE_ENQUEUED;
> > write_seqcount_begin(cpu_base->seq);
> > base->running = NULL;
> > write_seqcount_end(cpu_base->seq);
>
> Afaics, no. Afaics, the following code is correct:
>
> seqcount_t LOCK;
> bool X = true, Y = false;
>
> void read(void)
> {
> bool x, y;
>
> do {
> seq = read_seqcount_begin(&LOCK);
>
> x = X; y = Y;
>
> } while (read_seqcount_retry(&LOCK, seq));
>
> BUG_ON(!x && !y);
> }
>
> void write(void)
> {
> Y = true;
>
> write_seqcount_begin(LOCK);
> write_seqcount_end(LOCK);
>
> X = false;
> }
>
> If we rely on the "locking" semantics of seqcount_t, this doesn't really
> differ from
>
> spinlock_t LOCK;
> bool X = true, Y = false;
>
> void read(void)
> {
> bool x, y;
>
> spin_lock(LOCK);
> x = X; y = Y;
> spin_unlock(LOCK);
>
> BUG_ON(!x && !y);
> }
>
> void write(void)
> {
> Y = true;
>
> spin_lock(LOCK);
> spin_unlock(LOCK);
>
> X = false;
> }
>
> If "read" takes the lock before "write", it must see X == true.
>
> Otherwise "read" should see all memory changes done before or
> inside the "write" critical section, so it must see Y == true.
>
> No?

I'm agree with you. Thanks for the explanation.



\
 
 \ /
  Last update: 2015-06-11 18:41    [W:0.113 / U:0.636 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site