From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AE3EFCD6E67 for ; Wed, 11 Oct 2023 12:55:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346933AbjJKMzu (ORCPT ); Wed, 11 Oct 2023 08:55:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55184 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346360AbjJKMzo (ORCPT ); Wed, 11 Oct 2023 08:55:44 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0988BC6 for ; Wed, 11 Oct 2023 05:54:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1697028899; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=C3Q22l8FHOtSNLSPzzVZTuNcPC2ZofW6W5zArgJ64/4=; b=Ba4klxknMS5VMWO6UoWHBqn9+eFEN4jTh4L7OHbo5ILE87UxFWqbn0JcYjlas7aRe8c/7y E6662thTJEZxt1a3SU2/y59ckILrFOFJJ3Ck14NydST0ouUXJvn+QRJj0npUtUmVZwQhv3 3/lxfLr8DvLBDFtqZvPE+cRwGmE6B4U= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-298-eBsTBDv4PaiW8hiycp8iNg-1; Wed, 11 Oct 2023 08:54:56 -0400 X-MC-Unique: eBsTBDv4PaiW8hiycp8iNg-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2A8E33822E87; Wed, 11 Oct 2023 12:54:56 +0000 (UTC) Received: from [10.22.16.239] (unknown [10.22.16.239]) by smtp.corp.redhat.com (Postfix) with ESMTP id 886E3215670B; Wed, 11 Oct 2023 12:54:54 +0000 (UTC) Message-ID: <389a8abc-7f0f-7bcc-bc58-f70f045d00a5@redhat.com> Date: Wed, 11 Oct 2023 08:54:54 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.14.0 Subject: Re: [PATCH] cgroup/cpuset: Change nr_deadline_tasks to an atomic_t value Content-Language: en-US To: Juri Lelli Cc: Tejun Heo , Zefan Li , Johannes Weiner , linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, Peter Zijlstra , Ingo Molnar , Qais Yousef , Hao Luo , Dietmar Eggemann , Steven Rostedt , Xia Fukun References: <20231009191515.3262292-1-longman@redhat.com> <6b769316-6434-5054-43f5-7933fc2bee01@redhat.com> <31e06652-1dbd-e32f-3123-d17e178c5c27@redhat.com> From: Waiman Long In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 Precedence: bulk List-ID: X-Mailing-List: cgroups@vger.kernel.org On 10/11/23 04:14, Juri Lelli wrote: > On 10/10/23 16:03, Waiman Long wrote: >> On 10/10/23 15:44, Waiman Long wrote: >>> On 10/10/23 01:34, Juri Lelli wrote: >>>> Hi, >>>> >>>> On 09/10/23 15:15, Waiman Long wrote: >>>>> The nr_deadline_tasks field in cpuset structure was introduced by >>>>> commit 6c24849f5515 ("sched/cpuset: Keep track of SCHED_DEADLINE task >>>>> in cpusets"). Unlike nr_migrate_dl_tasks which is only modified under >>>>> cpuset_mutex, nr_deadline_tasks can be updated in various contexts >>>>> under different locks. As a result, data races may happen that cause >>>>> incorrect value to be stored in nr_deadline_tasks leading to incorrect >>>> Could you please make an example of such data races? >>> Since update to cs->nr_deadline_tasks is not protected by a single lock, >>> it is possible that multiple CPUs may try to modify it at the same >>> time.  It is possible that nr_deadline_tasks++ and nr_deadline_tasks-- >>> can be done in a single instruction like in x86 and hence atomic. >>> However, operation like "cs->nr_deadline_tasks += >>> cs->nr_migrate_dl_tasks" is likely a RMW operation and so is subjected >>> to racing. It is mostly theoretical, but probably not impossible. >> Sorry, even increment and decrement operators are not atomic. >> >> inc_dl_tasks_cs() is only called from switched_to_dl() in deadline.c which >> is protected by the rq_lock, but there are multiple rq's. dec_dl_tasks_cs() >> is called from switched_from_dl() in deadline.c and cgroup_exit() in >> cgroup.c. The later one is protected by css_set_lock. The other place where >> nr_deadline_tasks can be changed is in cpuset_attach() protected by >> cpuset_mutex. > So, let's see. :) > > switched_to_dl(), switched_from_dl() and cpuset_attach() should all be > protected (for DEADLINE tasks) by cpuset_mutex, see [1] for the former > two. Yes, I missed the cpuset_lock() call. > What leaves me perplexed is indeed cgroup_exit(), which seems to operate > under css_set_lock as you say. I however wonder why is that not racy > already wrt, say, cpuset_attach() which AFAIU uses css information w/o > holding css_set_lock? The css_set_lock protects changes made to css_set. Looking at cgroup_migrate_execute(), css_set_lock is taken when the tasks are actually moving from one css_set to another one. cpuset_attach() is called just to update the CPU and node affinity and cpuset_mutex is taken to ensure stability of the CPU and node masks. There is no change to css_set and so css_set_lock isn't needed. We can argue that there can be racing between cgroup_exit() and the iteration of tasks in cpuset_attach() or cpuset_can_attach(). An rcu_read_lock() is probably needed. I am stilling investigating that. Cheers, Longman > > Thanks, > Juri > > 1 - https://elixir.bootlin.com/linux/latest/source/kernel/sched/core.c#L7688 >