From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: kernel test robot <lkp@intel.com>,
Vipin Sharma <vipinsh@google.com>,
kbuild-all@lists.01.org, mkoutny@suse.com, tj@kernel.org,
lizefan.x@bytedance.com, hannes@cmpxchg.org, dmatlack@google.com,
jiangshanlai@gmail.com, kvm@vger.kernel.org,
cgroups@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] KVM: Move VM's worker kthreads back to the original cgroup before exiting.
Date: Sat, 19 Feb 2022 00:30:56 +0000 [thread overview]
Message-ID: <YhA6QIDME2wFbgIU@google.com> (raw)
In-Reply-To: <3113f00a-e910-2dfb-479f-268566445630@redhat.com>
On Thu, Feb 17, 2022, Paolo Bonzini wrote:
> On 2/17/22 13:34, kernel test robot wrote:
> > > 5859 reattach_err = cgroup_attach_task_all(current->real_parent, current);
>
> This needs to be within rcu_dereference().
As Vipin found out the hard way, cgroup_attach_task_all() can't be inside an RCU
critical section as it might sleep due to mutex_lock().
My sched knowledge is sketchy at best, but I believe KVM just needs to guarantee
the liveliness of real_parent when passing it to cgroup_attach_task_all().
Presumably kthreadd_task can (in theory) be killed/exiting while this is going on,
but at that point I doubt anyone cares much about cgroup accuracy.
So I think this can be:
rcu_read_lock();
parent = rcu_dereference(current->real_parent);
get_task_struct(parent);
rcu_read_unlock();
(void)cgroup_attach_task_all(parent, current);
put_task_struct(parent);
> > 5860 if (reattach_err) {
> > 5861 kvm_err("%s: cgroup_attach_task_all failed on reattach with err %d\n",
> > 5862 __func__, reattach_err);
Eh, I wouldn't bother logging the error, userspace can't take action on it, and
if the system is OOM it's just more noise in the log to dig through.
next prev parent reply other threads:[~2022-02-19 0:31 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-17 6:16 [PATCH v3] KVM: Move VM's worker kthreads back to the original cgroup before exiting Vipin Sharma
2022-02-17 12:34 ` kernel test robot
2022-02-17 16:05 ` Paolo Bonzini
2022-02-19 0:30 ` Sean Christopherson [this message]
2022-02-19 7:55 ` Paolo Bonzini
2022-02-17 19:47 ` kernel test robot
2022-02-17 23:22 ` kernel test robot
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=YhA6QIDME2wFbgIU@google.com \
--to=seanjc@google.com \
--cc=cgroups@vger.kernel.org \
--cc=dmatlack@google.com \
--cc=hannes@cmpxchg.org \
--cc=jiangshanlai@gmail.com \
--cc=kbuild-all@lists.01.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lizefan.x@bytedance.com \
--cc=lkp@intel.com \
--cc=mkoutny@suse.com \
--cc=pbonzini@redhat.com \
--cc=tj@kernel.org \
--cc=vipinsh@google.com \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).