Received: by 2002:a05:6a10:5bc5:0:0:0:0 with SMTP id os5csp6095521pxb; Mon, 8 Nov 2021 03:08:14 -0800 (PST) X-Google-Smtp-Source: ABdhPJz7162ZcjxgoK0HxLLU+8K6vlqtu76zfrDp5cmTB3Mg+fZ11qZWi1Ozvs1NqTBDr03waKec X-Received: by 2002:a05:6402:289f:: with SMTP id eg31mr76411684edb.192.1636369693795; Mon, 08 Nov 2021 03:08:13 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1636369693; cv=none; d=google.com; s=arc-20160816; b=zttkhb/mT6vole7dvX5JaszkyodZRTdTH0rGCbWemgB3W7iJQkvzTVM2gGhgOLBx3r DYkZnhYFEP8ursx58TdtaDiaaVe82qNUXq8i68ZbBrXrEX6LNv/R+4A65IXjfDKKa3hn fmmeskE2Qa4G59n2QgOjyuc7igGgeJTA4qSFqpDdyIgG7AqjCm4WbkCnefbg3j3kKpcb YLo/6GFFhIVtGXOy37AXmSOaZRNitavl2jhaF/EgzzTGQL5k4PitTm7fg61Obm9om/2F c4iNNjuCrQpAhVK/0Fpfv8MGD+3X4Ej029SLnbTNa/BqJ/u/pYIuS2tqMkkSD+HuX8gp NiDw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:dkim-signature:date; bh=xtDYLmAkngfEINMYcDuGTVquk3Snzoag54C0UVglbe8=; b=QWi6pB+Mw8TyTnYRwSf32H2/wLJHModCeLbzkdP/1BBNuDfSXIDrr2ln2sKGmY6c5k jfHIuZctVtqW97eH4sC8EmpNm8d2eznSOUPxDFZTSpboLWxmajFgKHgQRhuTCa2G4SI6 UkgCTMI1Rbr7AgDA3+FiNWbJyUbjKTdTg2kGYz3klZ9t+OCwwhhGuphv1EpEXdqhX80C +ytp+rEV1ncAw3N27nFizGjgo1ryWwBFcc79djL6KvSM5ZUGe44K3odYMj/6t0bYyJcK OSks4RnfNvzBHZHbOtmO8FsoTl3y3uHxQ36HIJMIJTJVbgDl248IdQ7XObuTigwpGA7R DjFw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.dev header.s=key1 header.b=UYrvWVdr; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.dev Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id j15si29163000edw.325.2021.11.08.03.07.49; Mon, 08 Nov 2021 03:08:13 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.dev header.s=key1 header.b=UYrvWVdr; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.dev Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237250AbhKHHOS (ORCPT + 99 others); Mon, 8 Nov 2021 02:14:18 -0500 Received: from out0.migadu.com ([94.23.1.103]:64755 "EHLO out0.migadu.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236151AbhKHHON (ORCPT ); Mon, 8 Nov 2021 02:14:13 -0500 Date: Mon, 8 Nov 2021 15:12:07 +0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1636355487; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=xtDYLmAkngfEINMYcDuGTVquk3Snzoag54C0UVglbe8=; b=UYrvWVdrKmC1WALCunaBtO4e9zlipfuBQ2YaIEI6AUI7EBB6LUJzctbLvDi+RndBrOBZZm rB4rw3H95xIkEDuszfyURNweKMW9cAf4+tA/QOmKZu8oFL3gie/+E94t/bcKbopOrKMLje DxYXU4HStsMqwkt51JHAT+Mr/KhaHBY= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Tao Zhou To: Peter Oskolkov Cc: Peter Zijlstra , Ingo Molnar , Thomas Gleixner , Andrew Morton , Dave Hansen , Andy Lutomirski , linux-mm@kvack.org, Linux Kernel Mailing List , linux-api@vger.kernel.org, Paul Turner , Ben Segall , Peter Oskolkov , Andrei Vagin , Jann Horn , Thierry Delisle , Tao Zhou Subject: Re: [PATCH v0.8 4/6] sched/umcg, lib/umcg: implement libumcg Message-ID: References: <20211104195804.83240-1-posk@google.com> <20211104195804.83240-5-posk@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Migadu-Flow: FLOW_OUT X-Migadu-Auth-User: tao.zhou@linux.dev Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Nov 07, 2021 at 10:27:02AM -0800, Peter Oskolkov wrote: > On Sun, Nov 7, 2021 at 8:33 AM Tao Zhou 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). This one is different because the task state flag is also masked by UMCG_TASK_STATE_MASK_FULL. But no effect I think. Document says that ' PREEMPTED: the userspace indicates it wants the worker to be preempted; there are no situations when both LOCKED and PREEMPTED flags are set at the same time. ' The task state flag this time only have UMCG_TF_LOCKED be set even if we cleared the task state flag. > > > > > > 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); > > > +}