From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965707AbbJ2Dpd (ORCPT ); Wed, 28 Oct 2015 23:45:33 -0400 Received: from mail-wm0-f43.google.com ([74.125.82.43]:36408 "EHLO mail-wm0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751299AbbJ2Dpc (ORCPT ); Wed, 28 Oct 2015 23:45:32 -0400 X-Greylist: delayed 470 seconds by postgrey-1.27 at vger.kernel.org; Wed, 28 Oct 2015 23:45:32 EDT Date: Thu, 29 Oct 2015 04:37:37 +0100 From: Frederic Weisbecker To: Hiroshi Shimamoto Cc: Peter Zijlstra , Ingo Molnar , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v2] cputime: fix invalid gtime Message-ID: <20151029033735.GA30367@lerouge> References: <7F861DC0615E0C47A872E6F3C5FCDDBD05F543FE@BPXM14GP.gisp.nec.co.jp> <20151028161145.GA25958@lerouge> <7F861DC0615E0C47A872E6F3C5FCDDBD05F54FC5@BPXM14GP.gisp.nec.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05F54FC5@BPXM14GP.gisp.nec.co.jp> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 29, 2015 at 01:10:01AM +0000, Hiroshi Shimamoto wrote: > > Obviously I completely messed up there. And task_cputime() has a similar issue > > but it happens to work due to vtime_snap_whence set to VTIME_SLEEPING when vtime > > doesn't run. Still it works at the cost of a seqcount read operation. > > > > Do you think you could fix it too (along with task_cputime_scaled())? I think those > > patches will also need a stable tag. > > Do you mean that task_cputime() and task_cputime_scaled() don't hit invalid behavior > but have some extra operation cost which could be removed? Exactly. > > Will look into it, and send patches with stable tag. Thanks a lot! Oh and another detail: vtime_accounting_enabled() checks if vtime accounting is done precisely on the current CPU. That's what we want to check when we account the time but not when we want to read the cputime of a task. For example, CPU 0 never has vtime_accounting_enabled() because it plays the role of timekeeper and as such it keeps the tick periodic. So if task A runs on CPU 1 that has vtime accounting on, and we read the cputime of task A from CPU 0, vtime_accounting_enabled() will be false whereas we need to compute the delta. So vtime_accounting_enabled() isn't suitable to check if vtime is running on _some_ CPU such that we can't return utime/stime with a raw read. Ideally we shoud rename vtime_accounting_enabled() to vtime_accounting_cpu_enabled() and have vtime_accounting_enabled() to check if vtime runs somewhere. But that would be too much an invasive change for a stable patch. So lets just use context_tracking_is_enabled() for now instead. I think task_gtime() is also buggy when the target task runs in a CPU that doesn't do vtime accounting (whereas another CPU does vtime accounting). We should fix that with using a new VTIME_GUEST value instead of (or along with) PF_VCPU. But that's another story, your fix is much more important for now. > > thanks, > Hiroshi > > > > > Thanks! > > > > > > > > > do { > > > seq = read_seqbegin(&t->vtime_seqlock); > > > > > > -- > > > 1.8.3.1 > > >