From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 512EDC761A6 for ; Tue, 4 Apr 2023 14:32:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235375AbjDDOc1 (ORCPT ); Tue, 4 Apr 2023 10:32:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35512 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234699AbjDDOcY (ORCPT ); Tue, 4 Apr 2023 10:32:24 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1F6CFEC for ; Tue, 4 Apr 2023 07:32:24 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id AC66163517 for ; Tue, 4 Apr 2023 14:32:23 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B4A28C4339E; Tue, 4 Apr 2023 14:32:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1680618743; bh=TtZ+P9hLCf/bO8SXpTpi4CGf1vSiKLdn+atcfEv8O+I=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=nUEOirkWUxwUIs+y9i5hIUi/tUTwPGkcq/aiVWz5LcLC6SmVQiYq0mmg5tSEVUCmF 5D2ydjf/SeHZrIJkmISi1uD+t7wqAAPrMTlTFrHgxnFlzn5egeF4FnYGsT4y2WQqu6 QSTqEU7nXam+7LXsIzAhkqutCxOdVyXMspMdo2lPC9Lot+q3QD7awKXMcoBTeCSSAE NS7ad/RCjRulq0VfCPggUfyRKzT+g2zLLC9/G3KEn/EVFWzei0xZ5YKzkXOrwNz28b lOGYua1Vep0cibns+gRWN4VhVaF8v9Bjt+A5KUfVQNx0fYU+gd8atcK7CaWhhz7rpQ H8VDNChGtkboA== Date: Tue, 4 Apr 2023 16:32:20 +0200 From: Frederic Weisbecker To: Anna-Maria Behnsen Cc: linux-kernel@vger.kernel.org, Peter Zijlstra , John Stultz , Thomas Gleixner , Eric Dumazet , "Rafael J . Wysocki" , Arjan van de Ven , "Paul E . McKenney" , Frederic Weisbecker , Rik van Riel Subject: Re: [PATCH v5 16/18] timer: Implement the hierarchical pull model Message-ID: References: <20230301141744.16063-1-anna-maria@linutronix.de> <20230301141744.16063-17-anna-maria@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 04, 2023 at 04:05:27PM +0200, Anna-Maria Behnsen wrote: > On Tue, 21 Mar 2023, Frederic Weisbecker wrote: > > > On Wed, Mar 01, 2023 at 03:17:42PM +0100, Anna-Maria Behnsen wrote: > > > +static u64 tmigr_handle_remote_cpu(unsigned int cpu, u64 now, > > > + unsigned long jif) > > > +{ > > > + struct timer_events tevt; > > > + struct tmigr_walk data; > > > + struct tmigr_cpu *tmc; > > > + u64 next = KTIME_MAX; > > > + unsigned long flags; > > > + > > > + tmc = per_cpu_ptr(&tmigr_cpu, cpu); > > > + > > > + raw_spin_lock_irqsave(&tmc->lock, flags); > > > + /* > > > + * Remote CPU is offline or no longer idle or other cpu handles cpu > > > + * timers already or next event was already expired - return! > > > + */ > > > + if (!tmc->online || tmc->remote || tmc->cpuevt.ignore || > > > + now < tmc->cpuevt.nextevt.expires) { > > > + raw_spin_unlock_irqrestore(&tmc->lock, flags); > > > + return next; > > > + } > > > + > > > + tmc->remote = 1; > > > + > > > + /* Drop the lock to allow the remote CPU to exit idle */ > > > + raw_spin_unlock_irqrestore(&tmc->lock, flags); > > > + > > > + if (cpu != smp_processor_id()) > > > + timer_expire_remote(cpu); > > > + > > > + /* next event of cpu */ > > > + fetch_next_timer_interrupt_remote(jif, now, &tevt, cpu); > > > > If the target CPU gets an idle interrupt right after the above call and enqueues > > a new timer (which becomes the new earliest), tmigr_cpu_deactivate() -> > > tmigr_new_timer() is going to ignore it due to tmc->remote = 1, right? > > It's worse. The newly enqueued timer is updated in the timer migration > hierarchy when CPU goes back idle and afterwards it will be overwritten by > the group walk propagating the old first timer in > tmigr_handle_remote_cpu()... Hmm then that would require the remote CPU to exit dynticks and then re-enter dynticks, right? Yes, sounds possible too. > > I will change the code after remote timer expiry: > > 1. take the remote timer bases locks > 2. take the tmc->lock > 3. get the next timer interrupt remote > 4. drop the remote timer bases locks > 5. propagate new timer changes > 6. drop the tmc->lock Right that sounds good! Thanks. > > Thanks, > > Anna-Maria