From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752750AbbJUKqi (ORCPT ); Wed, 21 Oct 2015 06:46:38 -0400 Received: from mail-pa0-f54.google.com ([209.85.220.54]:36495 "EHLO mail-pa0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751651AbbJUKqh (ORCPT ); Wed, 21 Oct 2015 06:46:37 -0400 Date: Wed, 21 Oct 2015 16:16:31 +0530 From: Viresh Kumar To: Yunhong Jiang , Frederic Weisbecker Cc: Thomas Gleixner , linux-kernel@vger.kernel.org Subject: Re: [PATCH] timer: Lazily wakup nohz CPU when adding new timer. Message-ID: <20151021104631.GB7784@ubuntu> References: <1443466096-31252-1-git-send-email-yunhong.jiang@linux.intel.com> <20151020224751.GB31289@jnakajim-build> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151020224751.GB31289@jnakajim-build> 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 Cc'ing Frederic. On 20-10-15, 15:47, Yunhong Jiang wrote: > On Sun, Oct 11, 2015 at 08:12:39PM +0200, Thomas Gleixner wrote: > > On Mon, 28 Sep 2015, Yunhong Jiang wrote: > > > static void internal_add_timer(struct tvec_base *base, struct timer_list *timer) > > > { > > > + bool kick_nohz = false; > > > + > > > /* Advance base->jiffies, if the base is empty */ > > > if (!base->all_timers++) > > > base->timer_jiffies = jiffies; > > > @@ -424,9 +426,17 @@ static void internal_add_timer(struct tvec_base *base, struct timer_list *timer) > > > */ > > > if (!(timer->flags & TIMER_DEFERRABLE)) { > > > if (!base->active_timers++ || > > > - time_before(timer->expires, base->next_timer)) > > > + time_before(timer->expires, base->next_timer)) { > > > base->next_timer = timer->expires; > > > - } > > > + /* > > > + * CPU in dynticks need reevaluate the timer wheel > > > + * if newer timer added with next_timer updated. > > > + */ > > > + if (base->nohz_active) > > > + kick_nohz = true; > > > + } > > > + } else if (base->nohz_active && tick_nohz_full_cpu(base->cpu)) > > > + kick_nohz = true; > > > > Why do you want to kick the other cpu when a deferrable timer got added? > > This is what happens in current implementation and this patch does not > change the logic. According to the comments, it's to avoid race with > idle_cpu(). Frankly speaking, I didn't get the idea of the race. > > Viresh, do you have any hints? I haven't looked at the core since few months now and looks like I don't remember anything :) This thread is where we discussed it initially: http://marc.info/?l=linux-kernel&m=139039035809125 AFAIU, this is why we kick the other CPU for a deferrable timer: - The other CPU is a full-dynticks capable CPU and may be running tickless and we should serve the timer in time (even if it is deferrable) if the CPU isn't idle. - We could have saved the kick for a full-dynticks idle CPU, but a race can happen where we thought the CPU is idle, but it has just started serving userspace tick-lessly. And the timer wouldn't be served for long time, even when the cpu was busy. Ofcourse, Frederic will kick me if I forgot the lessons he gave me earlier :) -- viresh