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.
