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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 32436CFC293 for ; Tue, 15 Oct 2024 12:06:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=0AqxCXCBiuDdx6p0geyzzVBppgNGKEdtVzoIUeKEDwg=; b=WvO/Un9dv0vcCglTxVKz8fhnZK Wz2AmTx5aX2B9bvaru5IU6uIqTEKp8aQXTPgSXRknUq/Ta8RRTsXhkhiAkNua/3S3lpu9PiQw73Hh yJ7ULcw3aXOJORX1rzNlyySYBkz0FCLghrNaYxjEWWvLuKy2sdGywp6EZJsa/FRffCpT9NXJbwnXg /26PJlHs5XZGqMYYlmgGcZ+zKlQnKKEzTRkuwGot1BKvnww8EtWSrypRJ2g10Xw5mn5Jt/ufKY+vW sqHl8c99iIYNNyrrucJtkCTCjUAWdu+HxQGYUsAmBdYDG5HBT3hY+7q7rf7T42DR0GbpywFvlM07Q 2SaegLlw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t0gJA-000000086Vx-44Du; Tue, 15 Oct 2024 12:05:48 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1t0gHl-000000086MP-1F49 for linux-arm-kernel@lists.infradead.org; Tue, 15 Oct 2024 12:04:23 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 0325E5C5C89; Tue, 15 Oct 2024 12:04:16 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 258F0C4CEC6; Tue, 15 Oct 2024 12:04:14 +0000 (UTC) Date: Tue, 15 Oct 2024 13:04:12 +0100 From: Catalin Marinas To: Ankur Arora Cc: linux-pm@vger.kernel.org, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, will@kernel.org, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, pbonzini@redhat.com, wanpengli@tencent.com, vkuznets@redhat.com, rafael@kernel.org, daniel.lezcano@linaro.org, peterz@infradead.org, arnd@arndb.de, lenb@kernel.org, mark.rutland@arm.com, harisokn@amazon.com, mtosatti@redhat.com, sudeep.holla@arm.com, cl@gentwo.org, misono.tomohiro@fujitsu.com, maobibo@loongson.cn, joao.m.martins@oracle.com, boris.ostrovsky@oracle.com, konrad.wilk@oracle.com Subject: Re: [PATCH v8 01/11] cpuidle/poll_state: poll via smp_cond_load_relaxed() Message-ID: References: <20240925232425.2763385-1-ankur.a.arora@oracle.com> <20240925232425.2763385-2-ankur.a.arora@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240925232425.2763385-2-ankur.a.arora@oracle.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241015_050421_401631_EB358E3C X-CRM114-Status: GOOD ( 20.11 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Sep 25, 2024 at 04:24:15PM -0700, Ankur Arora wrote: > diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c > index 9b6d90a72601..fc1204426158 100644 > --- a/drivers/cpuidle/poll_state.c > +++ b/drivers/cpuidle/poll_state.c > @@ -21,21 +21,20 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev, > > raw_local_irq_enable(); > if (!current_set_polling_and_test()) { > - unsigned int loop_count = 0; > u64 limit; > > limit = cpuidle_poll_time(drv, dev); > > while (!need_resched()) { > - cpu_relax(); > - if (loop_count++ < POLL_IDLE_RELAX_COUNT) > - continue; > - > - loop_count = 0; > + unsigned int loop_count = 0; > if (local_clock_noinstr() - time_start > limit) { > dev->poll_time_limit = true; > break; > } > + > + smp_cond_load_relaxed(¤t_thread_info()->flags, > + VAL & _TIF_NEED_RESCHED || > + loop_count++ >= POLL_IDLE_RELAX_COUNT); The above is not guaranteed to make progress if _TIF_NEED_RESCHED is never set. With the event stream enabled on arm64, the WFE will eventually be woken up, loop_count incremented and the condition would become true. However, the smp_cond_load_relaxed() semantics require that a different agent updates the variable being waited on, not the waiting CPU updating it itself. Also note that the event stream can be disabled on arm64 on the kernel command line. Does the code above break any other architecture? I'd say if you want something like this, better introduce a new smp_cond_load_timeout() API. The above looks like a hack that may only work on arm64 when the event stream is enabled. A generic option is udelay() (on arm64 it would use WFE/WFET by default). Not sure how important it is for poll_idle() but the downside of udelay() that it won't be able to also poll need_resched() while waiting for the timeout. If this matters, you could instead make smaller udelay() calls. Yet another problem, I don't know how energy efficient udelay() is on x86 vs cpu_relax(). So maybe an smp_cond_load_timeout() would be better, implemented with cpu_relax() generically and the arm64 would use LDXR, WFE and rely on the event stream (or fall back to cpu_relax() if the event stream is disabled). -- Catalin