From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from canpmsgout07.his.huawei.com (canpmsgout07.his.huawei.com [113.46.200.222]) (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 512203BD63B for ; Fri, 31 Jul 2026 08:31:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=113.46.200.222 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785486705; cv=none; b=O3lzOkByF7RkNsqpsleyNo680xcOkrwlF3VqPozIgn2e1OOSS5M1hqjzMX7LqQ1VFMsMkOOi/4JzbVGj5FBtfP7VfOzS+BiGLpW70IvCXgEqWTf/2/+tSkopFbEgR+Sv+JaOJ+sV5f0dkH/mcByvWeFItDTvoU9xwdIneQjNCck= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785486705; c=relaxed/simple; bh=TbegMpweqkIFr7mWVyvvfstYDKfVmfrYaNdxHB8U5c8=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=rIu1wNijEWKN3SbohpuL5RrBtpx0vfyLww4Jcy+XAS4jvTQij6V7fw1WtKhdqULQCpoOhRd3ufT+2EAL8ydaofvtDyEZffkUiKbjJ1rlyoj4H/JTlznrt3XLgoZneR0iSOX6go6pYcku6MMTVgxLGZj05bxfOeOwZrGTnYwWAQ0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b=FZMS33t7; arc=none smtp.client-ip=113.46.200.222 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b="FZMS33t7" dkim-signature: v=1; a=rsa-sha256; d=huawei.com; s=dkim; c=relaxed/relaxed; q=dns/txt; h=From; bh=fwYNk9pJImW872q26Dg1UvzpDPbwRu4RI02bSA8sb2Q=; b=FZMS33t7vw4kWpZSzO6fwsw6B7Jte85cmYDkqdMT9qhRCpRpV2gPqzaSjDqpK2HyEk6Fcv003 GAJtbW53JuVoJ1nz2mvKYcpu8q3hfOV/r3ii4BdipozTuTRBqn8sCRBFYIIxdefXqqub+AWfJkY 8LVmG5G04pVI+q8qCAEDHOY= Received: from mail.maildlp.com (unknown [172.19.163.127]) by canpmsgout07.his.huawei.com (SkyGuard) with ESMTPS id 4hBJvc2jMyzLlXC; Fri, 31 Jul 2026 16:22:04 +0800 (CST) Received: from kwepemj100017.china.huawei.com (unknown [7.202.194.11]) by mail.maildlp.com (Postfix) with ESMTPS id 937D540573; Fri, 31 Jul 2026 16:31:32 +0800 (CST) Received: from [10.67.108.244] (10.67.108.244) by kwepemj100017.china.huawei.com (7.202.194.11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.36; Fri, 31 Jul 2026 16:31:31 +0800 Message-ID: <60a02a7a-22ba-47d3-8bc6-e3eba6f7b4c7@huawei.com> Date: Fri, 31 Jul 2026 16:31:31 +0800 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 v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus Content-Language: en-US To: "Chen, Yu C" , Tim Chen CC: , , , , , , , , , , , Chen Yu References: <20260723040429.630176-1-luogengkun2@huawei.com> <20260723040429.630176-2-luogengkun2@huawei.com> <1dc03c84-9bc2-4db1-bab4-3f603fba54cd@huawei.com> <8b7cd508-3b89-4fc0-85fd-5a8d35ed06ca@huawei.com> <7d1ae7709bbe052f1d5737b6f0ef6cddf2f5e541.camel@linux.intel.com> <2d191aead79140de43022f480c3b542c101613f1.camel@linux.intel.com> <0ab7ec57-750a-4eb8-8697-aeafdd0f1786@huawei.com> <94e4418e7545df9509a0b8bd1233c0b00184455d.camel@linux.intel.com> <84f81335-796b-4eef-9d01-510af9d9cf14@intel.com> From: Luo Gengkun In-Reply-To: <84f81335-796b-4eef-9d01-510af9d9cf14@intel.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: kwepems500002.china.huawei.com (7.221.188.17) To kwepemj100017.china.huawei.com (7.202.194.11) On 2026/7/31 14:58, Chen, Yu C wrote: > On 7/31/2026 12:57 AM, Tim Chen wrote: > > [ ... ] > >>> >>> When talking about mm->sc_stat.lock, I found an issue that may be >>> worth paying attention. Below is the relevant code snippet: >>> >>> task_tick_cache() >>> { >>>     ... >>>           /* avoid moving backwards */ >>>           if (time_after_eq(mm->sc_stat.epoch, epoch)) >>>                   return; >>> >>>           guard(raw_spinlock)(&mm->sc_stat.lock); >>> >>>           if (work->next == work) { >>>                   task_work_add(p, work, TWA_RESUME); >>>                   WRITE_ONCE(mm->sc_stat.epoch, epoch); >>>           } >>>     .. >>> } >>> Actually, I don't think time_after_eq() can effectively avoid moving >>> backwards because this check is performed entirely outside the >>> protection of the spinlock. The following sequence diagram describes >>> this race condition in detail: >>> >>>        Thread A (CPU 0)                             Thread B (CPU 1) >>>        ============================                 ============================ >>>        [ Initial State: mm->sc_stat.epoch = 90 ] >>>        task_tick_cache()                            task_tick_cache() >>>          |                                            | >>>          +-> Read rq->cpu_epoch = 100                 +-> Read rq->cpu_epoch = 101 >>>          |                                            | >>>          +-> Lockless Check (90, 100) -> PASS         +-> Lockless Check (90, 101) -> PASS >>>          |                                            | >>>          |                                            +-> Acquires lock first >>>          |                                            +-> Writes mm->sc_stat.epoch = 101 >>>          |                                            +-> Releases lock >>>          |                                                [ mm->sc_stat.epoch is now 101 ] >>>          | >>>          +-> Acquires lock >>>          | >>>          +-> work->next == work (STILL TRUE! >>>          |   Because cache_work is per-thread, Thread B's >>>          |   submission cannot clear Thread A's local state) >>>          | >>>          +-> WRITE_ONCE(mm->sc_stat.epoch, 100) !!! <-- BUG: Epoch moves backwards! >> >> >> That would not happen because in __update_mm_sched(), Thread A will be >> reading the updated rq->cpu_epoch and again check >> whether time is moving backwards and will skip the update if >> that's the case. >> >> > > Maybe the backward comment in the code was a little confusing. > The "avoid moving backwards" logic was introduced to prevent a negative > timeout value in commit df0d98475954: > > if (epoch - READ_ONCE(mm->sc_stat.epoch) > EPOCH_LLC_AFFINITY_TIMEOUT) > > That is to say, by design, we want mm->sc_stat.epoch to chase after the > CPU's epoch and never jump ahead of any CPU's epoch. Otherwise, the subtraction > above could result in a huge value. > > Later, in commit c1e7fe5e75ed, that negative delta was avoided by: > > if ((long)(epoch - READ_ONCE(mm->sc_stat.epoch)) > So now, mm->sc_stat.epoch is only best-effort to not go backward. If it > actually does go backward, in my opinion it’s not a big deal. > > thanks, > Chenyu > > Thank you for your clarifying explanation :) Gengkun >