From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BD5949460 for ; Fri, 27 Dec 2024 00:39:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=178.60.130.6 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735259949; cv=none; b=dfBzoJG4oTIgxgCKTRA7u7ADoxbnmfJYKpWTaxJsb5bcLkPaoFQZpuOvYys6Jl5ZdeutjSxO1VCXkdCDuH8RH6doaumYc5I/VKttTd4jonA3ewi36sEFhGCxscSqh+Uy5z6XGJzUL71KkAH72e+fpp75i1y/GfC4h7LF2VtqsKU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735259949; c=relaxed/simple; bh=XkK8OzihK+onHw7sGenHW3JefX+/VmC6cg0UF2fArto=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=S1IleaOqBX0kA0R7Ki7/NS/Lz5FwHFcAdRRQwTGPKa6+j2MJU+TcHLvkLr8PJ347nFfnYoBxuWrHFsEBfgRYOcjA/HWIqDJW97Dwu+WM/tlTme1MXIGMV79TjhjaG+SPzVK7Re3KZRi0c/wPViPU2U26wMJv5T925bDxPfulT0A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=igalia.com; spf=pass smtp.mailfrom=igalia.com; dkim=pass (2048-bit key) header.d=igalia.com header.i=@igalia.com header.b=J39qE2cs; arc=none smtp.client-ip=178.60.130.6 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=igalia.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=igalia.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=igalia.com header.i=@igalia.com header.b="J39qE2cs" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=sQdhZJmAYsLvwHHa6AaaNl6O7uq2A0+uT0cQBEcfto0=; b=J39qE2csKMVtyDFCiKq7hXfUyE vEZv5BE4BKqK8P37FesQyZDuTANg+RzIkXuUzw5J3A1s1kbQ8OilxQgnXyKlIE6sPst7i3m2m91Da Pyuhu4abQnPf1937iNHeqS+GyIxY7mi4giN7FDohSoavAF2hJc1vf9FExrXbOqXW3v3qc4NMfdAhX VmjY5GbHVtWYvs2zH/53btT6iBfJiyM6f8UICOci7bwjiK8NPrC6C6D9Yu8P1xNb6OC1Vaf3DmqN5 4u98cW2TE4VBGNHlZhrSoHZkO9p6qje8DQmbLZxHf0hzIGEqcDw7SeIoxKpTkLBwF6862R2PEPvkI cCzCEO6g==; Received: from [58.29.143.236] (helo=[192.168.1.6]) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128) (Exim) id 1tQyNL-0085HA-3U; Fri, 27 Dec 2024 01:38:47 +0100 Message-ID: <5eeef2e1-7dc4-4aa8-8be4-c6e0fc2b5267@igalia.com> Date: Fri, 27 Dec 2024 09:38:37 +0900 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 2/6] sched_ext: Implement scx_bpf_now_ns() To: Tejun Heo , Changwoo Min Cc: void@manifault.com, arighi@nvidia.com, mingo@redhat.com, peterz@infradead.org, kernel-dev@igalia.com, linux-kernel@vger.kernel.org References: <20241220062025.27724-1-changwoo@igalia.com> <20241220062025.27724-3-changwoo@igalia.com> From: Changwoo Min Content-Language: en-US, ko-KR, en-US-large, ko In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello, On 24. 12. 25. 06:47, Tejun Heo wrote: > Hello, > > On Fri, Dec 20, 2024 at 03:20:21PM +0900, Changwoo Min wrote: > ... >> +__bpf_kfunc u64 scx_bpf_now_ns(void) > > Given that the default time unit is ns for the scheduler, the _ns suffix > can probably go. Ok. I will change is as suggested. > >> + if (!(READ_ONCE(rq->scx.flags) & SCX_RQ_CLK_VALID)) { >> + clock = sched_clock_cpu(cpu_of(rq)); >> + >> + /* >> + * The rq clock is updated outside of the rq lock. >> + * In this case, keep the updated rq clock invalid so the next >> + * kfunc call outside the rq lock gets a fresh rq clock. >> + */ >> + scx_rq_clock_update(rq, clock, false); > > Hmm... what does this update do? It can be dropped as we do not track prev_clock. > ... >> +static inline void scx_rq_clock_update(struct rq *rq, u64 clock, bool valid) >> +{ >> + if (!scx_enabled()) >> + return; >> + WRITE_ONCE(rq->scx.clock, clock); >> + if (valid) >> + WRITE_ONCE(rq->scx.flags, rq->scx.flags | SCX_RQ_CLK_VALID); >> +} > > Isn't rq->scx.clock used iff VALID is set? If so, why does !VALID read need > to update rq->scx.clock? If we drop the previous scx_rq_clock_update(.., false), we can drop the if condition checking the valid flag. Regards, Changwoo Min