On 11/21, Davidlohr Bueso wrote: > > On Mon, 21 Nov 2016, Oleg Nesterov wrote: > >> On 11/21, Oleg Nesterov wrote: >>> >>> No, no, I meant that afaics both readers can see per_cpu_sum() != 0 and >>> thus the writer won't be woken up. Till the next down_read/up_read. >>> >>> Suppose that we have 2 CPU's, both counters == 1, both readers decrement. >>> its counter at the same time. >>> >>> READER_ON_CPU_0 READER_ON_CPU_1 >>> >>> --ctr_0; --ctr_1; >>> >>> if (ctr_0 + ctr_1) if (ctr_0 + ctr_1) >>> wakeup(); wakeup(); >>> >>> Why we can't miss a wakeup? > > But the patch is really: if (!(ctr_0 + ctr_1)).
Of course, I meant if (ctr_0 + ctr_1 == 0). >> And in fact I am not sure this optimization makes sense... But it would be >> nice to avoid wake_up() when the writer sleeps in rcu_sync_enter(). Or this >> is the "slow mode" sem (cgroup_threadgroup_rwsem). > > Why do you think using per_cpu_sum() does not make sense? As mentioned in the > changelog it optimizes for incoming readers while the writer is doing > sync_enter > and getting the regular rwsem. What am I missing? And this does make sense, but see below, >> I need to re-check, but what do you think about the change below? > > While optimizing for multiple writers (rcu_sync_enter) is certainly valid > (at least considering the cgroups rwsem you mention), No, it is not for multiple writers. rcu_sync_enter() is slow, the new readers can come and acquire/release this lock. And if it is a "slow mode" sem then every up() does wakeup which we want to eliminate. But after sem->readers_block is already true, I am not sure the additional per_cpu_sum() is a win (even if it was correct), the new readers can't come. Except __percpu_down_read()->__percpu_up_read() which we want to optimize too, but in this case we do not need per_cpu_sum() too. I'll try to make a patch this week... I had this optimization in mind from the very beginning, I event mentioned it during the last discussion, but never had time. Basically we should not inc if readers_block == T. Oleg.