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
| ||
|
Date: Tue, 28 Apr 2015 15:57:33 +0200 From: Peter Zijlstra <peterz@...radead.org> To: Rik van Riel <riel@...riel.com> Cc: Heiko Carstens <heiko.carstens@...ibm.com>, linux-kernel@...r.kernel.org, Andy Lutomirsky <amluto@...capital.com>, Frederic Weisbecker <fweisbec@...hat.com>, williams@...hat.com Subject: Re: [PATCH v2] context_tracking: remove local_irq_save from __acct_update_integrals On Tue, Apr 28, 2015 at 08:53:18AM -0400, Rik van Riel wrote: > So what can I do to move forward with this patch? > > It speeds up syscall entry / exit by 7% when nohz_full > is enabled on a CPU... > > Should I have the irq block compiled in only when > sizeof(cputime_t) > sizeof(long) ? So I'm not sure how the IRQ disable even works today (as you write in the Changelog), it appears racy. At which point you already have the problem of load/store tearing. So either you (the pural) need to go fix that, or we should collectively say sod it and just do your patch. That said, your patch would probably want a WRITE_ONCE() around the assignment to acct_timexpd; ensuring the read happens in a single load doesn't really help if then the store is done in two :-) Also, we should probably reduce indent in that function, something like the below. --- kernel/tsacct.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/kernel/tsacct.c b/kernel/tsacct.c index 975cb49e32bf..9e225425bc3a 100644 --- a/kernel/tsacct.c +++ b/kernel/tsacct.c @@ -123,27 +123,27 @@ void xacct_add_tsk(struct taskstats *stats, struct task_struct *p) static void __acct_update_integrals(struct task_struct *tsk, cputime_t utime, cputime_t stime) { - if (likely(tsk->mm)) { - cputime_t time, dtime; - struct timeval value; - unsigned long flags; - u64 delta; - - local_irq_save(flags); - time = stime + utime; - dtime = time - tsk->acct_timexpd; - jiffies_to_timeval(cputime_to_jiffies(dtime), &value); - delta = value.tv_sec; - delta = delta * USEC_PER_SEC + value.tv_usec; - - if (delta == 0) - goto out; + cputime_t time, dtime; + struct timeval value; + unsigned long flags; + u64 delta; + + if (unlikely(!tsk->mm)) + return; + + local_irq_save(flags); + time = stime + utime; + dtime = time - tsk->acct_timexpd; + jiffies_to_timeval(cputime_to_jiffies(dtime), &value); + delta = value.tv_sec; + delta = delta * USEC_PER_SEC + value.tv_usec; + + if (delta) { tsk->acct_timexpd = time; tsk->acct_rss_mem1 += delta * get_mm_rss(tsk->mm); tsk->acct_vm_mem1 += delta * tsk->mm->total_vm; - out: - local_irq_restore(flags); } + local_irq_restore(flags); } /** -- 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/
Powered by blists - more mailing lists