linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Oskolkov <posk@google.com>
To: Thierry Delisle <tdelisle@uwaterloo.ca>
Cc: avagin@google.com, bsegall@google.com, jannh@google.com,
	linux-api@vger.kernel.org, linux-kernel@vger.kernel.org,
	mingo@redhat.com, pabuhr@uwaterloo.ca, peterz@infradead.org,
	pjt@google.com, posk@posk.io, tglx@linutronix.de
Subject: Re: [PATCH 3/4 v0.4] sched/umcg: add Documentation/userspace-api/umcg.rst
Date: Fri, 6 Aug 2021 10:25:02 -0700	[thread overview]
Message-ID: <CAPNVh5cQG8WjRryYXhrApNmV-7ybJ7ePpjXqtS6RUdguXO4e=A@mail.gmail.com> (raw)
In-Reply-To: <1dc64d75-da9b-272a-a9ab-7d2ae7a85764@uwaterloo.ca>

On Fri, Aug 6, 2021 at 9:52 AM Thierry Delisle <tdelisle@uwaterloo.ca> wrote:
>
>   > All _umcg_ state changes here happen in the userspace before
>   > sys_umcg_wait() is called. So:
>   >
>   > W1: cmpxchg W1:RUNNING => W1:IDLE
>   >  - if OK, call sys_umcg_wait()
>   >  - if failed, do something else (notes below)
>   >
>   > W2: cmpxchg W1:IDLE => W1:RUNNING
>   >  - if OK, lock itself, set W2:next_tid to W1, call sys_umcg_wait()
>   > (will not block nor spin), restore next_tid and state/unlock upon
>   > syscall return
>   >  - if failed, do something else
>
> I am talking about the case where both cmpxchg() succeed and W2 sets
> *both* UMCG_WAIT_WAKE_ONLY and UMCG_WAIT_WF_CURRENT_CPU.  More
> specifically, if things are ordered like so: (ideally use monospace font)
>
>           - w1 -                     - w2 -
>
> w1:RUNNING => w1:IDLE|L   |
> S:IDLE => S:RUNNING       |
> sys_umcg_wait():          |
>       set ~UMCG_TF_LOCKED  |
>                            |   w1:IDLE => w1:RUNNING|L
>                            |   w2:RUNNING => w2:IDLE|L
>                            |   w2:next_tid := W1.tid
>                            |   w1:RUNNING|L => w1:RUNNING
>                            |   sys_umcg_wait():
>                            |       set ~UMCG_TF_LOCKED
>                            |       do_context_switch()
>       idle_loop()          |
>
> What does ttwu() do with w1 if it has not reached idle_loop yet?

If both cmpxchg() succeeded, but W1 was never put to sleep, ttwu()
will do nothing and W1 will continue running on its initial CPU, while
W2 will continue running on its own CPU. WF_CURRENT_CPU is an advisory
flag, and in this situation it will not do anything.

>
> I am not familiar with the details of kernel context-switching, but in
> user-level threading, more specifically Cforall, this would be a problem.
> Between the call to do_context_switch() and and idle_loop(), there is a
> window where 2 CPUs run the same thread at the same time. One thread is
> performing the front side of the context switch and the other threads
> wakes up on the backside of the context switch. This behaviour invariably
> corrupts the program stack of that thread. Again, I do not know if that
> applies here. I am not exactly sure how the program stack is handled when
> inside a system call.

This is a wake, not a context switch, right? I'm not sure why you are
concerned with context switching here. And even if it were a context
switch, the kernel manages thread stacks properly, there's nothing to
worry about.

Am I missing something?

  reply	other threads:[~2021-08-06 17:25 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-01 20:06 [PATCH 0/4 v0.4] sched/umcg: RFC UMCG patchset Peter Oskolkov
2021-08-01 20:06 ` [PATCH 1/4 v0.4] sched/umcg: add WF_CURRENT_CPU and externise ttwu Peter Oskolkov
2021-08-01 20:06 ` [PATCH 2/4 v0.4] sched/umcg: RFC: add userspace atomic helpers Peter Oskolkov
2021-08-01 20:06 ` [PATCH 3/4 v0.4] sched/umcg: add Documentation/userspace-api/umcg.rst Peter Oskolkov
2021-08-01 20:08   ` Peter Oskolkov
2021-08-04 19:12   ` Thierry Delisle
2021-08-04 21:48     ` Peter Oskolkov
2021-08-06 16:51       ` Thierry Delisle
2021-08-06 17:25         ` Peter Oskolkov [this message]
2021-08-09 14:15           ` Thierry Delisle
2021-08-01 20:06 ` [PATCH 4/4 v0.4] sched/umcg: RFC: implement UMCG syscalls Peter Oskolkov
2021-08-04 22:04   ` Thierry Delisle
2021-08-04 23:30     ` Peter Oskolkov

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='CAPNVh5cQG8WjRryYXhrApNmV-7ybJ7ePpjXqtS6RUdguXO4e=A@mail.gmail.com' \
    --to=posk@google.com \
    --cc=avagin@google.com \
    --cc=bsegall@google.com \
    --cc=jannh@google.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pabuhr@uwaterloo.ca \
    --cc=peterz@infradead.org \
    --cc=pjt@google.com \
    --cc=posk@posk.io \
    --cc=tdelisle@uwaterloo.ca \
    --cc=tglx@linutronix.de \
    /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 NNTP newsgroup(s).