lkml.org 
[lkml]   [2016]   [Aug]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.

\
 
 \ /
  Last update: 2016-08-03 23:21    [W:2.211 / U:0.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site