Messages in this thread | | | Date | Mon, 28 Apr 2008 01:57:23 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] sched: missing locking in sched_domains code |
| |
On Mon, 28 Apr 2008 10:49:04 +0200 Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> On Mon, Apr 28, 2008 at 10:32:22AM +0200, Ingo Molnar wrote: > > > > * Heiko Carstens <heiko.carstens@de.ibm.com> wrote: > > > > > /* doms_cur_mutex serializes access to doms_cur[] array */ > > > static DEFINE_MUTEX(doms_cur_mutex); > > > > > > +static inline void lock_doms_cur(void) > > > +{ > > > + mutex_lock(&doms_cur_mutex); > > > +} > > > > > @@ -7813,8 +7811,10 @@ int arch_reinit_sched_domains(void) > > > int err; > > > > > > get_online_cpus(); > > > + lock_doms_cur(); > > > > thanks, that looks a lot more clean already. May i ask for another > > thing, if you are hacking on this anyway? Please get rid of the > > lock_doms_cur() complication now that it's not conditional - an open > > coded mutex_lock(&sched_doms_mutex) looks more readable - it gives a > > clear idea about what's happening. Also, please rename sched_doms_mutex > > to something less tongue-twisting - such as sched_domains_mutex. Hm? > > Your wish is my order:
heh, let's all boss Heiko around.
> /* doms_cur_mutex serializes access to doms_cur[] array */ > -static DEFINE_MUTEX(doms_cur_mutex); > +static DEFINE_MUTEX(sched_domains_mutex);
The comment refers to a no-longer-existing lock, and no longer correctly describes the lock's usage.
| |