Messages in this thread | | | Date | Mon, 18 Aug 2014 01:14:45 +0400 | From | Kirill Tkhai <> | Subject | Re: [PATCH 0/2] sched: tasklist_lock cleanups (Was: don't use while_each_thread()) |
| |
On 17.08.2014 19:25, Oleg Nesterov wrote: > On 08/13, Oleg Nesterov wrote: >> >> Peter, could you take these simple patches ? >> >> Better later than never... per-file, but please feel free to join >> them in a single patch. >> >> read_lock_irq*(tasklist_lock) in kernel/sched/ files looks strange. >> Why? I'll recheck, but this looks unneeded. > > Yes, please consider these minor cleanups on top of for_each_thread > conversions. > > read_lock_irq(tasklist) in normalize_rt_tasks() doesn't really hurt, > but it looks confusing. If we really have a reason to disable irqs > this (subtle) reason should be documented. > > And I can't understand tg_has_rt_tasks(). Don't we need something > like the patch below? If not, please do not ask me why I think so, > I don't understand this black magic ;) But the usage of the global > "runqueues" array looks suspicious.
This function searches RT task which is related to this tg. It's opaquely because it looks that there is an error.
task_rq(p)->rt.tg is a task group of a top rt_rq, while the task may be queued on a child rt_rq instead of this. So, your patch is a BUGFIX, not a cleanup.
> --- x/kernel/sched/core.c > +++ x/kernel/sched/core.c > @@ -7354,7 +7354,7 @@ static inline int tg_has_rt_tasks(struct > struct task_struct *g, *p; > > for_each_process_thread(g, p) { > - if (rt_task(p) && task_rq(p)->rt.tg == tg) > + if (rt_task(p) && task_group(p) == tg) > return 1; > }
| |