Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1445493
| From | Oleg Nesterov <oleg@redhat.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() |
| Date | 2016-07-18 15:50 +0200 |
| Message-ID | <rWh5L-6Ru-7@gated-at.bofh.it> (permalink) |
| References | (5 earlier) <rVdnz-ap-9@gated-at.bofh.it> <rVetk-No-37@gated-at.bofh.it> <rVfIJ-1Ix-3@gated-at.bofh.it> <rVBpT-6Ag-3@gated-at.bofh.it> <rWfnj-5Fz-1@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 07/18, Peter Zijlstra wrote:
>
> I think I ended up with this:
>
> .----> GP_IDLE <--------------.
> | | |
> | | __rcu_sync_enter() | <GP> / rcu_sync_exit()
> | v |
> | GP_ENTER --------------'
> | |
> | <GP> |
> | v
> | GP_PASSED <---------.
> | | |
> | | rcu_sync_exit() | <GP> / __rcu_sync_enter()
> | v |
> `----- GP_EXIT ------------'
> ^
> <GP> | __rcu_sync_enter() + rcu_sync_exit()
> v
> GP_RETRY
Thanks! I'll include this into the changelog.
> > static void rcu_sync_call(struct rcu_sync *rsp)
> > {
> > // TODO: THIS IS SUBOPTIMAL. We want to call it directly
> > // if rcu_blocking_is_gp() == T, but it has might_sleep().
>
> Not sure I get that comment..
I meant, we actually want
static void rcu_sync_call(struct rcu_sync *rsp)
{
if (rcu_blocking_is_gp())
rcu_sync_func(rsp);
else
gp_ops[rsp->gp_type].call(&rsp->cb_head, rcu_sync_func)
}
but this needs some other simple changes. rcu_sync_func() needs
the same spinlock and rcu_blocking_is_gp() calls might_sleep().
We can simply move rcu_sync_call() outside of rsp->rss_lock, but
then we will need more comments to explain why we can't race with
enter/exit from someone else. Or introduce __rcu_blocking_is_gp()
without might_sleep(), but this needs a trivial change outside of
rcu/sync.c.
We will see. This is simple anyway.
> > spin_lock_irqsave(&rsp->rss_lock, flags);
> > if (rsp->gp_count) {
> > /*
> > * We're at least a GP after the first __rcu_sync_enter().
> > */
> > rsp->gp_state = GP_PASSED;
>
> So we can end up here in two ways afaict.
>
> The simple way: someone called __rcu_sync_enter(), we go IDLE -> ENTER
> with a raised count. Once the GP passes, we get here, observe the raised
> count and advance to PASSED.
>
> The more involved way: we were EXIT and someone calls __rcu_sync_enter()
> to raise the count again. The callback from rcu_sync_exit() was still
> pending and once we get here we observe the raised count and voila.
Yes, yes.
> Now, since state != IDLE, I suppose this is valid, but it does hurt my
> brain.
Simply put, if rsp->gp_count != 0 we do not care about the history and
GP_PASSED is always correct when rcu callback is called, this obviously
means that we passed a GP.
Except GP_IDLE -> GP_PASSED transition is wrong, but this must not be
possible because only rcu callback can set GP_IDLE and only if !gp_count,
so we must always have at least one GP in between. Note also BUG_ON()
checks at the start.
> So I think its solid, but you failed to mention one state transition,
> which seems ok in any case.
Great, thanks for review.
I'll send the actual patch on top of your changes.
Oleg.
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 2/2] locking/percpu-rwsem: Introduce bias knob Peter Zijlstra <peterz@infradead.org> - 2016-07-14 20:40 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob Oleg Nesterov <oleg@redhat.com> - 2016-07-14 20:50 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob Peter Zijlstra <peterz@infradead.org> - 2016-07-14 21:00 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob Peter Zijlstra <peterz@infradead.org> - 2016-07-14 21:30 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-07-14 21:30 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob Peter Zijlstra <peterz@infradead.org> - 2016-07-14 21:40 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-07-14 22:00 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob Oleg Nesterov <oleg@redhat.com> - 2016-07-15 15:30 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-07-15 15:40 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob Oleg Nesterov <oleg@redhat.com> - 2016-07-15 15:50 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-07-15 17:40 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob Oleg Nesterov <oleg@redhat.com> - 2016-07-15 18:50 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-07-15 20:10 +0200
[PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() Oleg Nesterov <oleg@redhat.com> - 2016-07-16 19:20 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() Oleg Nesterov <oleg@redhat.com> - 2016-07-16 20:50 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() Peter Zijlstra <peterz@infradead.org> - 2016-07-18 14:00 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() Oleg Nesterov <oleg@redhat.com> - 2016-07-18 15:50 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-07-19 23:00 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() Oleg Nesterov <oleg@redhat.com> - 2016-07-20 17:20 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-07-20 23:00 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() Oleg Nesterov <oleg@redhat.com> - 2016-07-21 19:40 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() Oleg Nesterov <oleg@redhat.com> - 2016-07-20 19:20 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-07-20 23:40 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() Oleg Nesterov <oleg@redhat.com> - 2016-07-21 19:40 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-07-22 05:30 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() John Stultz <john.stultz@linaro.org> - 2016-07-25 19:10 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() Oleg Nesterov <oleg@redhat.com> - 2016-07-25 19:30 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() Oleg Nesterov <oleg@redhat.com> - 2016-07-25 19:10 +0200
Re: [PATCH] rcu_sync: simplify the state machine, introduce __rcu_sync_enter() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-07-25 19:50 +0200
Re: [PATCH 2/2] locking/percpu-rwsem: Introduce bias knob Oleg Nesterov <oleg@redhat.com> - 2016-07-15 15:50 +0200
csiph-web