linux-kernel - Re: [RFC PATCH] timer: Improve itimers scalability

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1438885310.4833.82.camel@j-VirtualBox>
Date:	Thu, 06 Aug 2015 11:21:50 -0700
From:	Jason Low <jason.low2@...com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
	Frederic Weisbecker <fweisbec@...il.com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Davidlohr Bueso <dave@...olabs.net>,
	Mike Galbraith <umgwanakikbuti@...il.com>, terry.rudd@...com,
	Rik van Riel <riel@...hat.com>,
	Waiman Long <Waiman.Long@...com>,
	Scott J Norton <scott.norton@...com>, jason.low2@...com
Subject: Re: [RFC PATCH] timer: Improve itimers scalability

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/