From: Peter Oskolkov <posk@posk.io>
To: Tao Zhou <tao.zhou@linux.dev>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
Andrew Morton <akpm@linux-foundation.org>,
Dave Hansen <dave.hansen@linux.intel.com>,
Andy Lutomirski <luto@kernel.org>,
linux-mm@kvack.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-api@vger.kernel.org, Paul Turner <pjt@google.com>,
Ben Segall <bsegall@google.com>, Peter Oskolkov <posk@google.com>,
Andrei Vagin <avagin@google.com>, Jann Horn <jannh@google.com>,
Thierry Delisle <tdelisle@uwaterloo.ca>
Subject: Re: [PATCH v0.8 4/6] sched/umcg, lib/umcg: implement libumcg
Date: Sun, 7 Nov 2021 10:27:02 -0800 [thread overview]
Message-ID: <CAFTs51X3s1zMVczUk_yHzLmLj5O+KaGmBy9go0EaVhphdNH_zg@mail.gmail.com> (raw)
In-Reply-To: <YYf/+LC77VUduuxD@geo.homenetwork>
On Sun, Nov 7, 2021 at 8:33 AM Tao Zhou <tao.zhou@linux.dev> wrote:
>
> On Thu, Nov 04, 2021 at 12:58:02PM -0700, Peter Oskolkov wrote:
>
> > +/* Update the state variable, set new timestamp. */
> > +static bool umcg_update_state(uint64_t *state, uint64_t *prev, uint64_t next)
> > +{
> > + uint64_t prev_ts = (*prev) >> (64 - UMCG_STATE_TIMESTAMP_BITS);
> > + struct timespec now;
> > + uint64_t next_ts;
> > + int res;
> > +
> > + /*
> > + * clock_gettime(CLOCK_MONOTONIC, ...) takes less than 20ns on a
> > + * typical Intel processor on average, even when run concurrently,
> > + * so the overhead is low enough for most applications.
> > + *
> > + * If this is still too high, `next_ts = prev_ts + 1` should work
> > + * as well. The only real requirement is that the "timestamps" are
> > + * uniqueue per thread within a reasonable time frame.
> > + */
> > + res = clock_gettime(CLOCK_MONOTONIC, &now);
> > + assert(!res);
> > + next_ts = (now.tv_sec * NSEC_PER_SEC + now.tv_nsec) >>
> > + UMCG_STATE_TIMESTAMP_GRANULARITY;
> > +
> > + /* Cut higher order bits. */
> > + next_ts &= ((1ULL << UMCG_STATE_TIMESTAMP_BITS) - 1);
>
> This is the right cut.. The same to the kernel side.
Yes, thanks!
>
> > +
> > + if (next_ts == prev_ts)
> > + ++next_ts;
> > +
> > +#ifndef NDEBUG
> > + if (prev_ts > next_ts) {
> > + fprintf(stderr, "%s: time goes back: prev_ts: %lu "
> > + "next_ts: %lu diff: %lu\n", __func__,
> > + prev_ts, next_ts, prev_ts - next_ts);
> > + }
> > +#endif
> > +
> > + /* Remove old timestamp, if any. */
> > + next &= ((1ULL << (64 - UMCG_STATE_TIMESTAMP_BITS)) - 1);
> > +
> > + /* Set the new timestamp. */
> > + next |= (next_ts << (64 - UMCG_STATE_TIMESTAMP_BITS));
> > +
> > + /*
> > + * TODO: review whether memory order below can be weakened to
> > + * memory_order_acq_rel for success and memory_order_acquire for
> > + * failure.
> > + */
> > + return atomic_compare_exchange_strong_explicit(state, prev, next,
> > + memory_order_seq_cst, memory_order_seq_cst);
> > +}
> > +
>
> > +static void task_unlock(struct umcg_task_tls *task, uint64_t expected_state,
> > + uint64_t new_state)
> > +{
> > + bool ok;
> > + uint64_t next;
> > + uint64_t prev = atomic_load_explicit(&task->umcg_task.state_ts,
> > + memory_order_acquire);
> > +
> > + next = ((prev & ~UMCG_TASK_STATE_MASK_FULL) | new_state) & ~UMCG_TF_LOCKED;
>
> Use UMCG_TASK_STATE_MASK instead and the other state flag can be checked.
Why? We want to clear the TF_LOCKED flag and keep every other bit of
state, including other state flags (but excluding timestamp).
>
> All others places that use UMCG_TASK_STATE_MASK_FULL to mask to check
> the task state may seems reasonable if the state flag not allowed to
> be set when we check that task state, otherwise use UMCG_TASK_STATE_MASK
> will be enough.
>
> Not sure.
>
>
> Thanks,
> Tao
> > + assert(next != prev);
> > + assert((prev & UMCG_TASK_STATE_MASK_FULL & ~UMCG_TF_LOCKED) == expected_state);
> > +
> > + ok = umcg_update_state(&task->umcg_task.state_ts, &prev, next);
> > + assert(ok);
> > +}
next prev parent reply other threads:[~2021-11-07 18:27 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-04 19:57 [PATCH v0.8 0/6] sched,mm,x86/uaccess: implement User Managed Concurrency Groups Peter Oskolkov
2021-11-04 19:57 ` [PATCH v0.8 1/6] sched/umcg: add WF_CURRENT_CPU and externise ttwu Peter Oskolkov
2021-11-04 19:58 ` [PATCH v0.8 2/6] mm, x86/uaccess: add userspace atomic helpers Peter Oskolkov
2021-11-05 6:10 ` kernel test robot
2021-11-09 4:18 ` Tao Zhou
2021-11-04 19:58 ` [PATCH v0.8 3/6] sched/umcg: implement UMCG syscalls Peter Oskolkov
2021-11-05 12:55 ` kernel test robot
2021-11-05 23:48 ` Thierry Delisle
2021-11-08 4:09 ` Peter Oskolkov
2021-11-06 17:20 ` Tao Zhou
2021-11-07 18:26 ` Peter Oskolkov
2021-11-08 6:57 ` Tao Zhou
2021-11-15 20:11 ` kernel test robot
2021-11-21 21:08 ` Peter Oskolkov
2021-11-04 19:58 ` [PATCH v0.8 4/6] sched/umcg, lib/umcg: implement libumcg Peter Oskolkov
2021-11-07 16:34 ` Tao Zhou
2021-11-07 18:27 ` Peter Oskolkov [this message]
2021-11-08 7:12 ` Tao Zhou
2021-11-04 19:58 ` [PATCH v0.8 5/6] sched/umcg: add Documentation/userspace-api/umcg.txt Peter Oskolkov
2021-11-04 19:58 ` [PATCH v0.8 6/6] sched/umcg, lib/umcg: add tools/lib/umcg/libumcg.txt Peter Oskolkov
2021-11-09 8:55 ` [PATCH v0.8 0/6] sched,mm,x86/uaccess: implement User Managed Concurrency Groups Barry Song
2021-11-09 16:31 ` 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=CAFTs51X3s1zMVczUk_yHzLmLj5O+KaGmBy9go0EaVhphdNH_zg@mail.gmail.com \
--to=posk@posk.io \
--cc=akpm@linux-foundation.org \
--cc=avagin@google.com \
--cc=bsegall@google.com \
--cc=dave.hansen@linux.intel.com \
--cc=jannh@google.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=pjt@google.com \
--cc=posk@google.com \
--cc=tao.zhou@linux.dev \
--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).