From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1424613AbcFMPUa (ORCPT ); Mon, 13 Jun 2016 11:20:30 -0400 Received: from www.linutronix.de ([62.245.132.108]:57681 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1424276AbcFMPU0 (ORCPT ); Mon, 13 Jun 2016 11:20:26 -0400 Date: Mon, 13 Jun 2016 17:18:17 +0200 (CEST) From: Thomas Gleixner To: Richard Cochran cc: LKML , Ingo Molnar , Peter Zijlstra , "Paul E. McKenney" , Eric Dumazet , Frederic Weisbecker , Chris Mason , Arjan van de Ven , rt@linutronix.de Subject: Re: [patch 18/20] timer: Forward wheel clock whenever possible In-Reply-To: <20160613151458.GA18490@localhost.localdomain> Message-ID: References: <20160613070440.950649741@linutronix.de> <20160613075929.693947132@linutronix.de> <20160613151458.GA18490@localhost.localdomain> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 13 Jun 2016, Richard Cochran wrote: > On Mon, Jun 13, 2016 at 08:41:04AM -0000, Thomas Gleixner wrote: > > @@ -498,23 +500,27 @@ static void internal_add_timer(struct ti > > __internal_add_timer(base, timer); > > > > /* > > - * Check whether the other CPU is in dynticks mode and needs > > - * to be triggered to reevaluate the timer wheel. We are > > - * protected against the other CPU fiddling with the timer by > > - * holding the timer base lock. This also makes sure that a > > - * CPU on the way to stop its tick can not evaluate the timer > > - * wheel. > > - * > > - * Spare the IPI for deferrable timers on idle targets though. > > - * The next busy ticks will take care of it. Except full dynticks > > - * require special care against races with idle_cpu(), lets deal > > - * with that later. > > - */ > > - if (IS_ENABLED(CONFIG_NO_HZ_COMMON) && base->nohz_active) { > > - if (!(timer->flags & TIMER_DEFERRABLE) || > > - tick_nohz_full_cpu(base->cpu)) > > - wake_up_nohz_cpu(base->cpu); > > - } > > + * We might have to IPI the remote CPU if the base is idle and the > > + * timer is not deferrable. If the other cpu is on the way to idle > > + * then it can't set base->is_idle as we hold base lock. > > + */ > > + if (!IS_ENABLED(CONFIG_NO_HZ_COMMON) || !base->is_idle || > > + (timer->flags & TIMER_DEFERRABLE)) > > + return; > > The tests for is_idle and TIMER_DEFERRABLE are actually checking the > same thing. I was looking at the usage of base.is_idle, and it is > only ever set for BASE_STD. > > So the TIMER_DEFERRABLE test is redundant, but maybe this is on > purpose? Anyhow, it did leave me scratching my head. You're right. is_idle makes the deferrable check obsolete. We can remove it, but it needs a big fat comment .... Thanks tglx