On 10/09, Chen, Hanxiao wrote: > > > From: Oleg Nesterov [mailto:oleg@xxxxxxxxxx] > > > > Hmm. We only want the tasks from our namespace, yes? Perhaps find_ge_pid() > > makes more sense? > > Only tasks from our ns is valid. > But how could find_ge_pid() do that? > > nr = 1; > while (nr < PID_MAX_LIMIT) { > find_ge_pid(nr, curr_ns); > list_add(); > nr++; > } something like this, except list_add() should obviously depend on is_child_reaper() check. This can be more optimal in sub-namespaces, you do not need to abuse the global process list. And if you change this code to use get_pid/put_pid, then you do not need to hold rcu_read_lock() throughout, you only need it around find_ge_pid + get_pid. At the same time, for_each_process() in the global namespace can be faster if there are a lot of sub-threads. > Perhaps that's not a good way. OK, I won't insist. although it would be nice to know why do you think this is bad. > > > + pid = task_pid(p); > > > > Well, in theory you need barrier() here. Or perhaps we should add > > ACCESS_ONCE() into task_pid()... > > You mean modify task_pid as: > return ACCESS_ONCE(task->pids[PIDTYPE_PID].pid;); Yes. But not now an not in this patch of course. I'd suggest to add barrier() just in case. > > And imho it would be better to declare pidns_list/pidns_tree locally > > in nslist_proc_show() and pass them to the callees. > > That's a good idea. > Will changed in the next version. Good. And I forgot to mention, in this case you do not need pidns_list_lock at all afaics. Oleg. _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/containers
