Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756089AbbHFSV4 (ORCPT ); Thu, 6 Aug 2015 14:21:56 -0400 Received: from g4t3425.houston.hp.com ([15.201.208.53]:39049 "EHLO g4t3425.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753870AbbHFSVy (ORCPT ); Thu, 6 Aug 2015 14:21:54 -0400 Message-ID: <1438885310.4833.82.camel@j-VirtualBox> Subject: Re: [RFC PATCH] timer: Improve itimers scalability From: Jason Low To: Oleg Nesterov Cc: Peter Zijlstra , Ingo Molnar , linux-kernel@vger.kernel.org, Frederic Weisbecker , "Paul E. McKenney" , Davidlohr Bueso , Mike Galbraith , terry.rudd@hp.com, Rik van Riel , Waiman Long , Scott J Norton , jason.low2@hp.com Date: Thu, 06 Aug 2015 11:21:50 -0700 In-Reply-To: <20150806141820.GA8258@redhat.com> References: <1438734584.2927.15.camel@j-VirtualBox> <20150806141820.GA8258@redhat.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2515 Lines: 66 On Thu, 2015-08-06 at 16:18 +0200, Oleg Nesterov wrote: > On 08/04, Jason Low wrote: > > > > @@ -973,13 +981,6 @@ static void check_process_timers(struct task_struct *tsk, > > virt_expires = check_timers_list(++timers, firing, utime); > > sched_expires = check_timers_list(++timers, firing, sum_sched_runtime); > > > > - /* > > - * Check for the special case process timers. > > - */ > > - check_cpu_itimer(tsk, &sig->it[CPUCLOCK_PROF], &prof_expires, ptime, > > - SIGPROF); > > - check_cpu_itimer(tsk, &sig->it[CPUCLOCK_VIRT], &virt_expires, utime, > > - SIGVTALRM); > > soft = READ_ONCE(sig->rlim[RLIMIT_CPU].rlim_cur); > > if (soft != RLIM_INFINITY) { > > unsigned long psecs = cputime_to_secs(ptime); > > @@ -1010,11 +1011,21 @@ static void check_process_timers(struct task_struct *tsk, > > } > > } > > > > + /* > > + * Check for the special case process timers. > > + */ > > + check_cpu_itimer(tsk, &sig->it[CPUCLOCK_PROF], &prof_expires, ptime, > > + SIGPROF); > > + check_cpu_itimer(tsk, &sig->it[CPUCLOCK_VIRT], &virt_expires, utime, > > + SIGVTALRM); > > + > > Not sure I understand this part... looks wrong actually, please note > that RLIMIT_CPU block above may need to update prof_expires _after_ > check_cpu_itimer(), or I am totally confused. This change isn't critical to the patch, so we can delete this from the patch. Though from my understanding, the purpose of prof_expires is to collect the earliest prof expire time for when we update "sig->cputime_expires.prof_exp". So I think it wouldn't matter which order prof_expire gets updated (as long as check_timers_list() is called first, since prof_expires gets directly assigned there). > > if (READ_ONCE(sig->cputimer.running)) { > > struct task_cputime group_sample; > > > > + /* > > + * If another thread in the group is already checking > > + * for the thread group cputimer, then we will skip that. > > + */ > > + if (READ_ONCE(sig->cputimer.is_checking_timer)) > > + return 0; > > + > > Cosmetic, I won't insist, but this is not symmetrical to ->running check, > > if (READ_ONCE(sig->cputimer.running) && > !READ_ONCE(sig->cputimer.is_checking_timer)) > > looks a littke bit better to me. I agree, I will be making that change. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/