From: Qais Yousef <qais.yousef@arm.com> To: Li Zefan <lizefan@huawei.com>, Tejun Heo <tj@kernel.org>, Johannes Weiner <hannes@cmpxchg.org> Cc: cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, Qais Yousef <qais.yousef@arm.com> Subject: [PATCH] cgroup/cpuset: Fix a race condition when reading cpuset.* Date: Tue, 11 Feb 2020 14:15:54 +0000 [thread overview] Message-ID: <20200211141554.24181-1-qais.yousef@arm.com> (raw) LTP cpuset_hotplug_test.sh was failing with the following error message cpuset_hotplug 1 TFAIL: root group's cpus isn't expected(Result: 0-5, Expect: 0,2-5). Which is due to a race condition between cpu hotplug operation and reading cpuset.cpus file. When a cpu is onlined/offlined, cpuset schedules a workqueue to sync its internal data structures with the new values. If a read happens during this window, the user will read a stale value, hence triggering the failure above. To fix the issue make sure cpuset_wait_for_hotplug() is called before allowing any value to be read, hence forcing the synchronization to happen before the read. I ran 500 iterations with this fix applied with no failure triggered. Signed-off-by: Qais Yousef <qais.yousef@arm.com> --- I think it's okay to flush the workqueue from the read context? We do it on the write, so I assumed it's okay on the read too. But it'd be good to confirm it doesn't break any rule I'm not aware of. kernel/cgroup/cpuset.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c index 58f5073acff7..593055522626 100644 --- a/kernel/cgroup/cpuset.c +++ b/kernel/cgroup/cpuset.c @@ -2405,6 +2405,9 @@ static int cpuset_common_seq_show(struct seq_file *sf, void *v) cpuset_filetype_t type = seq_cft(sf)->private; int ret = 0; + /* Ensure all hotplug ops were done before reading any value */ + cpuset_wait_for_hotplug(); + spin_lock_irq(&callback_lock); switch (type) { -- 2.17.1
WARNING: multiple messages have this Message-ID (diff)
From: Qais Yousef <qais.yousef-5wv7dgnIgG8@public.gmane.org> To: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org> Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Qais Yousef <qais.yousef-5wv7dgnIgG8@public.gmane.org> Subject: [PATCH] cgroup/cpuset: Fix a race condition when reading cpuset.* Date: Tue, 11 Feb 2020 14:15:54 +0000 [thread overview] Message-ID: <20200211141554.24181-1-qais.yousef@arm.com> (raw) LTP cpuset_hotplug_test.sh was failing with the following error message cpuset_hotplug 1 TFAIL: root group's cpus isn't expected(Result: 0-5, Expect: 0,2-5). Which is due to a race condition between cpu hotplug operation and reading cpuset.cpus file. When a cpu is onlined/offlined, cpuset schedules a workqueue to sync its internal data structures with the new values. If a read happens during this window, the user will read a stale value, hence triggering the failure above. To fix the issue make sure cpuset_wait_for_hotplug() is called before allowing any value to be read, hence forcing the synchronization to happen before the read. I ran 500 iterations with this fix applied with no failure triggered. Signed-off-by: Qais Yousef <qais.yousef-5wv7dgnIgG8@public.gmane.org> --- I think it's okay to flush the workqueue from the read context? We do it on the write, so I assumed it's okay on the read too. But it'd be good to confirm it doesn't break any rule I'm not aware of. kernel/cgroup/cpuset.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c index 58f5073acff7..593055522626 100644 --- a/kernel/cgroup/cpuset.c +++ b/kernel/cgroup/cpuset.c @@ -2405,6 +2405,9 @@ static int cpuset_common_seq_show(struct seq_file *sf, void *v) cpuset_filetype_t type = seq_cft(sf)->private; int ret = 0; + /* Ensure all hotplug ops were done before reading any value */ + cpuset_wait_for_hotplug(); + spin_lock_irq(&callback_lock); switch (type) { -- 2.17.1
next reply other threads:[~2020-02-11 14:16 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-11 14:15 Qais Yousef [this message] 2020-02-11 14:15 ` [PATCH] cgroup/cpuset: Fix a race condition when reading cpuset.* Qais Yousef 2020-02-12 22:15 ` Tejun Heo 2020-02-12 22:15 ` Tejun Heo 2020-02-13 11:50 ` Qais Yousef 2020-02-13 11:50 ` Qais Yousef 2020-02-13 13:56 ` Tejun Heo 2020-02-13 14:36 ` Qais Yousef
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200211141554.24181-1-qais.yousef@arm.com \ --to=qais.yousef@arm.com \ --cc=cgroups@vger.kernel.org \ --cc=hannes@cmpxchg.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lizefan@huawei.com \ --cc=tj@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.