MIME-Version: 1.0
In-Reply-To: <20150630121812.GG3644@twins.programming.kicks-ass.net>
References: <1434099316-29749-1-git-send-email-fredrik.markstrom@gmail.com>
 <1434099316-29749-2-git-send-email-fredrik.markstrom@gmail.com>
 <1434104217.1495.74.camel@twins> <20150612110158.GA18673@twins.programming.kicks-ass.net>
 <CAKdL+dQWGtzURUe+4cB4XtsoaEPh0p0O1ejzbgxN0AUsbvUi0g@mail.gmail.com>
 <20150629145837.GE3644@twins.programming.kicks-ass.net> <CAKdL+dRPEenCzi14SWypOv6Vv+x2X0bHQXXc-G6gqUT+VhOtUw@mail.gmail.com>
 <20150630093054.GK19282@twins.programming.kicks-ass.net> <CAKdL+dR68T7LMZ_ndwihvMo_SaUB1G++JaZ28RXiqaHs9gpP0g@mail.gmail.com>
 <20150630121812.GG3644@twins.programming.kicks-ass.net>
From: =?UTF-8?Q?Fredrik_Markstr=C3=B6m?= <fredrik.markstrom@gmail.com>
Date: Tue, 30 Jun 2015 20:30:05 +0200
Message-ID: <CAKdL+dTTgfKrskE5d6OxktxJ4SHeMvmRPka4oAQ97LJmDefn6A@mail.gmail.com>
Subject: Re: [PATCH 1/1] cputime: Make the reported utime+stime correspond to
 the actual runtime.
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@redhat.com, linux-kernel@vger.kernel.org,
        Rik van Riel <riel@redhat.com>, Jason Low <jason.low2@hp.com>,
        =?UTF-8?B?RnLDqWTDqXJpYyBXZWlzYmVja2Vy?= <fweisbec@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2275
Lines: 60

On Tue, Jun 30, 2015 at 2:18 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Tue, Jun 30, 2015 at 01:50:15PM +0200, Fredrik Markström wrote:
>> Excellent,
>
> Please do not top post.

Understood, sorry !

>
>> The reason I replaced the early bail with that last test is that I
>> believe it needs to be done within the lock and I wanted to keep that
>> region short. To be honest I'm not sure this test is needed at all
>> anymore, but I couldn't make sense of the comment above the early bail
>> so I didn't dare to remove it.
>
> Ah, there's a simple reason we should keep it, apart from the wobblies
> in calculating the division. Imagine two concurrent callers, on with an
> rtime ahead of the other. Let the latest rtime caller acquire the lock
> first and compute s/u-time. Once the second caller acquires the lock, we
> observe the last rtime was in the past and we use the latest values.

You are so right, sorry about that ! I agree the test is needed and it needs to
be done with the lock held.

But I don't think the "wobblies" in the division is, since the division doesn't
affect the sum (prev->stime + prev->rtime) anymore, so that comment should
go, right ?

>
>> Regarding the lock, have you considered how many cores you need
>> hammering at rusage to introduce some substantial congestion ?
>
> Spinlock contention across 120 cores and 4 nodes is pretty bad, even
> with hardly any hold time :-)
>
> I've not investigated where the absolute pain threshold is, but given the
> size (and growth) of machines these days, its seems like a prudent
> thing.
>

Well I guess it can be a problem on a system where 120 cores are doing nothing
but hammering on rusage... on the other hand I feel a system like that
deserves it.  :)

>> Sorry for not letting this go (I know I should) but I always feel bad
>> introducing per thread data.
>
> Yes agreed, but a global lock is just asking for trouble. Esp when its
> as easy as this to avoid it.
>

Ok, you might be right. Either or I'm letting go now :)

/Fredrik
--
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/