From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932853AbcHCUhV (ORCPT ); Wed, 3 Aug 2016 16:37:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37142 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932352AbcHCUhS (ORCPT ); Wed, 3 Aug 2016 16:37:18 -0400 Date: Wed, 3 Aug 2016 22:26:09 +0200 From: Oleg Nesterov To: Peter Zijlstra Cc: Andrew Morton , Dave Anderson , Ingo Molnar , "Paul E. McKenney" , Wang Shu , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] introduce for_each_process_thread_{break,continue}() helpers Message-ID: <20160803202609.GA11419@redhat.com> References: <20160725162332.GA23935@redhat.com> <20160725162348.GA23947@redhat.com> <20160802115850.GI6862@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160802115850.GI6862@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.24 (2015-08-30) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 03 Aug 2016 20:26:12 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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.