Messages in this thread | | | Date | Mon, 3 Aug 2015 15:49:32 +0200 | From | Frederic Weisbecker <> | Subject | Re: [PATCH 05/10] nohz: New tick dependency mask |
| |
On Mon, Aug 03, 2015 at 03:24:07PM +0200, Peter Zijlstra wrote: > On Mon, Aug 03, 2015 at 03:05:37PM +0200, Frederic Weisbecker wrote: > > On Mon, Aug 03, 2015 at 02:43:50PM +0200, Peter Zijlstra wrote: > > > On Thu, Jul 23, 2015 at 06:42:10PM +0200, Frederic Weisbecker wrote: > > > > +unsigned long __tick_nohz_set_tick_dependency(enum tick_dependency_bit bit, > > > > + unsigned long *dep) > > > > +{ > > > > + unsigned long prev; > > > > + unsigned long old = *dep; > > > > + unsigned long mask = BIT_MASK(bit); > > > > + > > > > + while ((prev = cmpxchg(dep, old, old | mask)) != old) { > > > > + old = prev; > > > > + cpu_relax(); > > > > + } > > > > + > > > > + return prev; > > > > +} > > > > > > That's typically called a fetch_or(). The function name, which is > > > entirely too long, also doesn't suggest a return value. > > > > Nice, I can export fetch_or() from sched code to atomic_fetch_or() like > > you just suggested and bring it to asm_generic if you like. > > asm_generic means you _have_ to go implement it for all archs that do > not use asm_generic. > > The version I have is also unsuited for the atomic name, as it deals > with all integer types that cmpxchg() can consume (and I need it that > way, because thread_info::flags does not have a consistent type across > archs (unsigned long vs unsigned int is the most common variation).
Indeed, and I need clear_bit() to be the symetric call and that doesn't seem to operate on atomic_t.
> > You can go add it to linux/atomic.h I suppose, with the regular #ifdef > guards we have there. Then add: > > atomic{,long,64}_fetch_{add,sub,inc,dec,or,and,xor}() > > I suppose, using cmpxchg() loops and then let archs implement as we go.
Right.
> > > So perhaps fetch_and_or() ? Of course having "and" then "or" in the same function name > > might be confusing but after all we have functions names starting with "test" then "and". > > No fetch_$op is a well known name for these things, don't go change it. > That just means people will _not_ find it.
Ok.
> > > Concerning the nohz function name, tick_nohz is a long prefix that is very likely > > to produce too long names. Perhaps I should trim tick_nohz to just nohz. > > No, then we'll get confused with the regular nohz stuff. But yes a 10 > letter prefix sucks, but at least you can try and not then add another > 30.
Well regular nohz stuff (that is, idle nohz right?) and nohz full have the same prefix. We just have tick_nohz_full_* for very specific things but that's still compatible with a common reduced prefix.
But lets keep it that way for now, if a rename is needed, we'll do it beyond the scope of that set. I'll just take your tick_nohz_set_dep() suggestion.
Thanks.
| |