Messages in this thread | | | Date | Tue, 24 Feb 2009 16:49:36 +0100 | From | Oleg Nesterov <> | Subject | Re: broken do_each_pid_{thread,task} |
| |
On 02/24, Jiri Slaby wrote: > > On 15.12.2008 18:09, Oleg Nesterov wrote: >> On 12/15, Oleg Nesterov wrote: >>> On 12/14, Eric W. Biederman wrote: >>>> Although seeing the unexpected corner case it gets us into I think it would >>>> be good to reconsider this test. >> >> So. I can't decide whether this patch is cleanup or the further >> uglification, but if anyone likes it I will be happy to send it. > > FWIW I don't like the patch :)
Well, I agree, it is not very nice ;)
But why do you dislike it? Yes, the implementation of pid_for_each_task() is not simple. Partly because hlist_for_each_entry_rcu() is ugly and imho should be fixed (see btw http://marc.info/?t=120879441200004).
But with this patch the callers become simpler, we can just do
pid_for_each_task(pid, type, task) do_something(task);
instead of
do_each_pid_task(pid, type, task) { do_something(task); } while_each_pid_task(pid, type, task);
and we can use break/continue safely.
> Otherwise I'll add at least a big warning about using break/cont > statements inside the loop.
Agreed, this would be nice.
>> +#define pid_for_each_task(pid, type, p) \ >> + for (p = (pid) ? (void*)(pid)->tasks[type].first : NULL; \ >> + rcu_dereference(p)&& ({ \ >> + prefetch(((struct hlist_node*)p)->next); \ >> + p = hlist_entry((void*)p, typeof(*p), pids[type].node); \ >> + 1; }); \ >> + p = ((type) != PIDTYPE_PID) ? \ >> + (void*)(p)->pids[type].node.next : NULL) >> +
Really, is this too bad?
Oleg.
| |