Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756238AbbGGIAK (ORCPT ); Tue, 7 Jul 2015 04:00:10 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:50785 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753339AbbGGIAA (ORCPT ); Tue, 7 Jul 2015 04:00:00 -0400 Date: Tue, 7 Jul 2015 09:59:54 +0200 From: Peter Zijlstra To: Frederic Weisbecker Cc: Fredrik =?iso-8859-1?Q?Markstr=F6m?= , mingo@redhat.com, linux-kernel@vger.kernel.org, Rik van Riel , Jason Low Subject: Re: [PATCH 1/1] cputime: Make the reported utime+stime correspond to the actual runtime. Message-ID: <20150707075954.GN3644@twins.programming.kicks-ass.net> References: <20150629145837.GE3644@twins.programming.kicks-ass.net> <20150630093054.GK19282@twins.programming.kicks-ass.net> <20150630121812.GG3644@twins.programming.kicks-ass.net> <20150702121126.GP3644@twins.programming.kicks-ass.net> <20150702130701.GP18673@twins.programming.kicks-ass.net> <20150707005135.GH4981@lerouge> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150707005135.GH4981@lerouge> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2447 Lines: 71 On Tue, Jul 07, 2015 at 02:51:36AM +0200, Frederic Weisbecker wrote: > On Thu, Jul 02, 2015 at 03:07:01PM +0200, Peter Zijlstra wrote: > > + /* > > + * Make sure stime doesn't go backwards; this preserves monotonicity > > + * for utime because rtime is monotonic. > > + * > > + * utime_i+1 = rtime_i+1 - stime_i > > I'm not sure what is meant by _i+1. Since we have a discrete set of elements, we can enumerate them and _i is the i-th element in the (ordered) set. _i+1 is the i+1-th element, and so on. > I guess stime_i means prev->stime. stime_i+1 the new update of prev->stime > But then what is rtime_i and rtime_i+1 since we have no scaled rtime? still the previous and the next value. rtime_i+1 >= rtime_i just means that every next rtime value must be equal or greater than the last, IOW. rtime must be monotonic. > > + * = rtime_i+1 - (rtime_i - stime_i) > > + * = (rtime_i+1 - rtime_i) + stime_i > > + * >= stime_i > > + */ > > + if (stime < prev->stime) > > + stime = prev->stime; > > + utime = rtime - stime; > > + > > + /* > > + * Make sure utime doesn't go backwards; this still preserves > > + * monotonicity for stime, analogous argument to above. > > + */ > > + if (utime < prev->utime) { > > + utime = prev->utime; > > + stime = rtime - utime; > > I see, so we are guaranteed that this final stime won't get below > prev->stime because older prev->stime + prev->utime <= newest rtime. I > guess that's more or less what's in the comments above :-) Indeed. > > + } > > > > +update: > > + prev->stime = stime; > > + prev->utime = utime; > > out: > > *ut = prev->utime; > > *st = prev->stime; > > + raw_spin_unlock(&prev->lock); > > } > > > > void task_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime_t *st) > > Ok I scratched my head a lot on this patch and the issues behind and it looks > good to me. I worried about introducing a spinlock but we had two cmpxchg before > that. The overhead is close. Its slightly worse, I had to change the raw_spin_lock, to raw_spin_lock_irqsave() because Ingo managed to trigger a lockdep splat with sighand lock taking this lock, and sighand lock is IRQ-safe. -- 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/