Messages in this thread | | | Date | Tue, 29 Sep 2015 18:51:27 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 02/11] x86/mm/hotplug: Remove pgd_list use from the memory hotplug code |
| |
On 09/29, Ingo Molnar wrote: > > * Oleg Nesterov <oleg@redhat.com> wrote: > > > struct task_struct *next_task_with_mm(struct task_struct *p) > > { > > struct task_struct *t; > > > > p = p->group_leader; > > while ((p = next_task(p)) != &init_task) { > > if (p->flags & PF_KTHREAD) > > continue; > > > > t = find_lock_task_mm(p); > > if (t) > > return t; > > } > > > > return NULL; > > } > > > > #define for_each_task_lock_mm(p) > > for (p = &init_task; (p = next_task_with_mm(p)); task_unlock(p)) > > > > > > So that you can do > > > > for_each_task_lock_mm(p) { > > do_something_with(p->mm); > > > > if (some_condition()) { > > // UNFORTUNATELY you can't just do "break" > > task_unlock(p); > > break; > > } > > } > > > > do you think it makes sense? > > Sure, I'm inclined to use the above code from you. > > > In fact it can't be simpler, we can move task_unlock() into next_task_with_mm(), > > it can check ->mm != NULL or p != init_task. > > s/can't/can ?
yes, sorry,
> But even with that I'm not sure I can parse your suggestion. Got some (pseudo) code > perhaps?
I meant
struct task_struct *next_task_lock_mm(struct task_struct *p) { struct task_struct *t;
if (p) { task_unlock(p); p = p->group_leader; } else { p = &init_task; }
while ((p = next_task(p)) != &init_task) { if (p->flags & PF_KTHREAD) continue;
t = find_lock_task_mm(p); if (t) return t; }
return NULL; }
#define for_each_task_lock_mm(p) for (p = NULL; (p = next_task_lock_mm(p)); )
Oleg.
| |