Messages in this thread | | | Date | Wed, 3 Aug 2016 22:26:09 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/3] introduce for_each_process_thread_{break,continue}() helpers |
| |
Thanks for review and sorry for delay, I am travelling till the end of this week.
On 08/02, Peter Zijlstra wrote: > > On Mon, Jul 25, 2016 at 06:23:48PM +0200, Oleg Nesterov wrote: > > +void for_each_process_thread_continue(struct task_struct **p_leader, > > + struct task_struct **p_thread) > > +{ > > + struct task_struct *leader = *p_leader, *thread = *p_thread; > > + struct task_struct *prev, *next; > > + u64 start_time; > > + > > + if (pid_alive(thread)) { > > + /* mt exec could change the leader */ > > + *p_leader = thread->group_leader; > > + } else if (pid_alive(leader)) { > > + start_time = thread->start_time; > > + prev = leader; > > + > > + for_each_thread(leader, next) { > > + if (next->start_time > start_time) > > + break; > > + prev = next; > > + } > > This, > > > + *p_thread = prev; > > + } else { > > + start_time = leader->start_time; > > + prev = &init_task; > > + > > + for_each_process(next) { > > + if (next->start_time > start_time) > > + break; > > + prev = next; > > + } > > and this, could be 'SPEND_TOO_MUCH_TIME' all by themselves. > > Unlikely though, nor do I really have a better suggestion :/
Yeees, I don't think this can actually hurt "in practice", but I agree, compared to rcu_lock_break() this is only bounded by PID_MAX_DEFAULT in theory.
Will you agree if I just add the "int max_scan" argument and make it return a boolean for the start? The caller will need to abort the for_each_process_thread() loop if _continue() fails.
Probably this is not what we actually want for show_filter_state(), we can make it better later. We can "embed" the rcu_lock_break() logic into _continue(), or change break/continue to record the state (leader_start_time, thread_start_time) so that a "false" return from _continue() means that the caller needs another schedule(),
struct state state;
rcu_read_lock(); for_each_process_thread(p, t) { do_something_slow(p, t);
if (SPENT_TOO_MANY_TIME) { for_each_process_thread_break(p, t, &state); another_break: rcu_read_unlock(); schedule(); rcu_read_lock(); if (!for_each_process_thread_continue(&p, &t, LIMIT, &state)) goto another_break; } } rcu_read_unlock();
Not sure. I'd like to do something simple for the start. We need to make show_state_filter() "preemptible" in any case. And even killable, I think. Not only it can trivially trigger the soft-lockups (at least), it can simply never finish.
Oleg.
| |