Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752568Ab3JAQrR (ORCPT ); Tue, 1 Oct 2013 12:47:17 -0400 Received: from mail-we0-f176.google.com ([74.125.82.176]:59717 "EHLO mail-we0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751239Ab3JAQrO (ORCPT ); Tue, 1 Oct 2013 12:47:14 -0400 Date: Tue, 1 Oct 2013 18:47:10 +0200 From: Frederic Weisbecker To: Peter Zijlstra Cc: Oleg Nesterov , Arjan van de Ven , Fernando Luis =?iso-8859-1?Q?V=E1zquez?= Cao , Ingo Molnar , Thomas Gleixner , LKML , Tetsuo Handa , Andrew Morton Subject: Re: [PATCH 2/4] nohz: Synchronize sleep time stats with seqlock Message-ID: <20131001164708.GG24825@localhost.localdomain> References: <20130820163312.GA17957@redhat.com> <20130820175429.GI3258@twins.programming.kicks-ass.net> <20130820182553.GB22287@redhat.com> <20130821083130.GM3258@twins.programming.kicks-ass.net> <20130821113551.GA1472@redhat.com> <20130821123311.GA31370@twins.programming.kicks-ass.net> <20130821142356.GC31370@twins.programming.kicks-ass.net> <20130821164146.GA15194@redhat.com> <20131001140525.GE24825@localhost.localdomain> <20131001155633.GR3657@laptop.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20131001155633.GR3657@laptop.programming.kicks-ass.net> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1614 Lines: 56 On Tue, Oct 01, 2013 at 05:56:33PM +0200, Peter Zijlstra wrote: > > So what's wrong with something like: > > struct cpu_idletime { > seqlock_t seqlock; > unsigned long nr_iowait; > u64 start; > u64 idle_time, > u64 iowait_time, > } __cacheline_aligned_in_smp; > > DEFINE_PER_CPU(struct cpu_idletime, cpu_idletime); > > void io_schedule(void) > { > struct cpu_idletime *it = __raw_get_cpu_var(cpu_idletime); > > write_seqlock(&it->seqlock); > if (!it->nr_iowait++) > it->start = local_clock(); > write_sequnlock(&it->seqlock); > > current->in_iowait = 1; > schedule(); > current->in_iowait = 0; > > write_seqlock(&it->seqlock); > if (!--it->nr_iowait) > it->iowait_time += local_clock() - it->start; > write_sequnlock(&it->seqlock); > } > > Afaict you don't need the preempt disable and atomic muck at all. Yeah thinking more about it, the preempt disable was probably not necessary. Now that's trading 2 atomics + 1 Lock/Unlock with 2 Lock/Unlock. OTOH it computes iowait time seperately from idle time, so we probably don't need to lock the idle time anymore. Plus this solution is much much more simple. So if nobody sees a flaw there, I'll try this. Thanks. > > It will all get a little more complicated to deal with overlapping idle > and iowait times, but the idea is the same. Probably no big deal. -- 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/