Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Sender: Tejun Heo <htejun@gmail.com>
Date:   Thu, 7 Dec 2023 14:16:30 -1000
From:   Tejun Heo <tj@kernel.org>
To:     Changwoo Min <multics69@gmail.com>
Cc:     kernel-dev@igalia.com, andrea.righi@canonical.com,
        andrii@kernel.org, ast@kernel.org, bpf@vger.kernel.org,
        brho@google.com, bristot@redhat.com, bsegall@google.com,
        changwoo@igalia.com, daniel@iogearbox.net, derkling@google.com,
        dietmar.eggemann@arm.com, dschatzberg@meta.com,
        dskarlat@cs.cmu.edu, dvernet@meta.com, haoluo@google.com,
        himadrics@inria.fr, joshdon@google.com, juri.lelli@redhat.com,
        kernel-team@meta.com, linux-kernel@vger.kernel.org,
        martin.lau@kernel.org, memxor@gmail.com, mgorman@suse.de,
        mingo@redhat.com, peterz@infradead.org, pjt@google.com,
        riel@surriel.com, rostedt@goodmis.org,
        torvalds@linux-foundation.org, vincent.guittot@linaro.org,
        vschneid@redhat.com
Subject: Re: [PATCH] scx: set p->scx.ops_state using atomic_long_set_release
Message-ID: <ZXJgXqQlHc1mgd1m@slm.duckdns.org>
References: <20231111024835.2164816-13-tj@kernel.org>
 <20231207020459.117365-1-changwoo@igalia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231207020459.117365-1-changwoo@igalia.com>
Precedence: bulk

Hello,

On Thu, Dec 07, 2023 at 11:04:59AM +0900, Changwoo Min wrote:
> p->scx.ops_state should be updated using the release semantics,
> atomic_long_set_release(), because it is read using
> atomic_long_read_acquire() at ops_dequeue() and wait_ops_state().
> ---
>  kernel/sched/ext.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 53ee906aa2b6..3a40ca2007b6 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -881,7 +881,7 @@ static void do_enqueue_task(struct rq *rq, struct task_struct *p, u64 enq_flags,
>  	qseq = rq->scx.ops_qseq++ << SCX_OPSS_QSEQ_SHIFT;
>  
>  	WARN_ON_ONCE(atomic_long_read(&p->scx.ops_state) != SCX_OPSS_NONE);
> -	atomic_long_set(&p->scx.ops_state, SCX_OPSS_QUEUEING | qseq);
> +	atomic_long_set_release(&p->scx.ops_state, SCX_OPSS_QUEUEING | qseq);

atomic_long_load_acquire() are used when waiting the transitions out of
QUEUEING and DISPATCHING states. ie. the interlocking between writer and
reader is necessary only when transitioning out of those states. In the
above, @p is going into QUEUEING and release/acquire isn't necessary.
Selectively using them is kinda subtle but it's less confusing to keep it
that way, I think.

Thanks.

-- 
tejun