KVM Archive mirror
 help / color / mirror / Atom feed
From: "Christoph Lameter (Ampere)" <cl@gentwo.org>
To: Ankur Arora <ankur.a.arora@oracle.com>
Cc: linux-pm@vger.kernel.org, kvm@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,  catalin.marinas@arm.com,
	will@kernel.org, tglx@linutronix.de,  mingo@redhat.com,
	bp@alien8.de, 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,
	 joao.m.martins@oracle.com, boris.ostrovsky@oracle.com,
	 konrad.wilk@oracle.com
Subject: Re: [PATCH 1/9] cpuidle: rename ARCH_HAS_CPU_RELAX to ARCH_HAS_OPTIMIZED_POLL
Date: Fri, 3 May 2024 10:07:16 -0700 (PDT)	[thread overview]
Message-ID: <f72de572-bf9c-7b1c-194d-6e2de9d4c9b5@gentwo.org> (raw)
In-Reply-To: <877cgba5xn.fsf@oracle.com>

On Thu, 2 May 2024, Ankur Arora wrote:

>> The intend was to make the processor aware that we are in a spin loop. Various
>> processors have different actions that they take upon encountering such a cpu
>> relax operation.
>
> Sure, though most processors don't have a nice mechanism to do that.
> x86 clearly has the REP; NOP thing. arm64 only has a YIELD which from my
> measurements is basically a NOP when executed on a system without
> hardware threads.
>
> And that's why only x86 defines ARCH_HAS_CPU_RELAX.

My impression is that the use of arm YIELD has led cpu architects to 
implement similar mechanisms to x86s PAUSE, This is not part of the spec 
but it has been there for a long time. So I would rather leave it as is.


>> These are not the same and I think we need both config options.
>
> My main concern is that poll_idle() conflates polling in idle with
> ARCH_HAS_CPU_RELAX, when they aren't really related.
>
> So, poll_idle(), and its users should depend on ARCH_HAS_OPTIMIZED_POLL
> which, if defined by some architecture, means that poll_idle() would
> be better than a spin-wait loop.
>
> Beyond that I'm okay to keep ARCH_HAS_CPU_RELAX around.
>
> That said, do you see a use for ARCH_HAS_CPU_RELAX? The only current
> user is the poll-idle path.

I would think that we need a generic cpu_poll() mechanism that can fall 
back to cpu_relax() on processors that do not offer such thing (x86?) and 
if not even that is there fall back.

We already have something like that in the smp_cond_acquire mechanism (a 
bit weird to put that in the barrier.h>).

So what if we had

void cpu_wait(unsigned flags, unsigned long timeout, void *cacheline);

With

#define CPU_POLL_INTERRUPT (1 << 0)
#define CPU_POLL_EVENT (1 <<  1)
#define CPU_POLL_CACHELINE (1 << 2)
#define CPU_POLL_TIMEOUT (1 << 3)
#define CPU_POLL_BROADCAST_EVENT (1 << 4)
#define CPU_POLL_LOCAL_EVENT (1 << 5)


The cpu_poll() function coud be generically defined in asm-generic and 
then arches could provide their own implementation optimizing the hardware 
polling mechanisms.

Any number of flags could be specified simultaneously. On ARM this would 
map then to SEVL SEV and WFI/WFE WFIT/WFET

So f.e.

cpu_wait(CPU_POLL_INTERUPT|CPU_POLL_EVENT|CPU_POLL_TIMEOUT|CPU_POLL_CACHELINE, 
timeout, &mylock);

to wait on a change in a cacheline with a timeout.

In additional we could then think about making effective use of the 
signaling mechanism provided by SEV in core logic of the kernel. Maybe 
that is more effective then waiting for a cacheline in some situations.


> With WFE, sure there's a problem in that you depend on an interrupt or
> the event-stream to get out of the wait. And, so sometimes you would
> overshoot the target poll timeout.

Right. The dependence on the event stream makes this approach a bit 
strange. Having some sort of generic cpu_wait() feature with timeout spec 
could avoid that.


  reply	other threads:[~2024-05-03 17:07 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-30 18:37 [PATCH 0/9] Enable haltpoll for arm64 Ankur Arora
2024-04-30 18:37 ` [PATCH 1/9] cpuidle: rename ARCH_HAS_CPU_RELAX to ARCH_HAS_OPTIMIZED_POLL Ankur Arora
2024-05-02  1:33   ` Christoph Lameter (Ampere)
2024-05-03  4:13     ` Ankur Arora
2024-05-03 17:07       ` Christoph Lameter (Ampere) [this message]
2024-05-06 21:27         ` Ankur Arora
2024-04-30 18:37 ` [PATCH 2/9] Kconfig: move ARCH_HAS_OPTIMIZED_POLL to arch/Kconfig Ankur Arora
2024-04-30 18:37 ` [PATCH 3/9] cpuidle-haltpoll: condition on ARCH_CPUIDLE_HALTPOLL Ankur Arora
2024-05-22 16:09   ` Joao Martins
2024-04-30 18:37 ` [PATCH 4/9] cpuidle-haltpoll: define arch_haltpoll_supported() Ankur Arora
2024-05-01 11:48   ` kernel test robot
2024-05-22 16:09   ` Joao Martins
2024-04-30 18:37 ` [PATCH 5/9] governors/haltpoll: drop kvm_para_available() check Ankur Arora
2024-04-30 18:37 ` [PATCH 6/9] cpuidle/poll_state: poll via smp_cond_load_relaxed() Ankur Arora
2024-04-30 18:37 ` [PATCH 7/9] arm64: define TIF_POLLING_NRFLAG Ankur Arora
2024-04-30 18:37 ` [PATCH 8/9] arm64: support cpuidle-haltpoll Ankur Arora
2024-05-30 23:07   ` Okanovic, Haris
2024-04-30 18:37 ` [PATCH 9/9] cpuidle/poll_state: limit POLL_IDLE_RELAX_COUNT on arm64 Ankur Arora
2024-04-30 18:56 ` [PATCH 0/9] Enable haltpoll for arm64 Ankur Arora

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f72de572-bf9c-7b1c-194d-6e2de9d4c9b5@gentwo.org \
    --to=cl@gentwo.org \
    --cc=ankur.a.arora@oracle.com \
    --cc=arnd@arndb.de \
    --cc=boris.ostrovsky@oracle.com \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=harisokn@amazon.com \
    --cc=hpa@zytor.com \
    --cc=joao.m.martins@oracle.com \
    --cc=konrad.wilk@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=lenb@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).