Messages in this thread | | | Date | Wed, 25 Sep 2013 19:50:55 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH] hotplug: Optimize {get,put}_online_cpus() |
| |
On 09/25, Peter Zijlstra wrote: > > On Wed, Sep 25, 2013 at 05:55:15PM +0200, Oleg Nesterov wrote: > > > > +static inline void get_online_cpus(void) > > > +{ > > > + might_sleep(); > > > + > > > + /* Support reader-in-reader recursion */ > > > + if (current->cpuhp_ref++) { > > > + barrier(); > > > + return; > > > + } > > > + > > > + preempt_disable(); > > > + if (likely(!__cpuhp_writer)) > > > + __this_cpu_inc(__cpuhp_refcount); > > > > mb() to ensure the reader can't miss, say, a STORE done inside > > the cpu_hotplug_begin/end section. > > > > put_online_cpus() needs mb() as well. > > OK, I'm not getting this; why isn't the sync_sched sufficient to get out > of this fast path without barriers?
Aah, sorry, I didn't notice this version has another synchronize_sched() in cpu_hotplug_done().
Then I need to recheck again...
No. Too tired too ;) damn LSB test failures...
> > > + if (atomic_dec_and_test(&cpuhp_waitcount)) > > > + wake_up_all(&cpuhp_writer); > > > > Same problem as in previous version. __get_online_cpus() succeeds > > without incrementing __cpuhp_refcount. "goto start" can't help > > afaics. > > I added a goto into the cond-block, not before the cond; but see the > version below.
"into the cond-block" doesn't look right too, at first glance. This always succeeds, but by this time another writer can already hold the lock.
Oleg.
| |