From: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
To: "Pratik R. Sampat" <psampat-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org>
Cc: bristot-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
christian-STijNZzMWpgWenYVfaLwtA@public.gmane.org,
ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org,
lizefan.x-EC8Uxl6Npydl57MIdRCFDg@public.gmane.org,
tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org,
mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
juri.lelli-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
containers-cunTk1MwBs/YUNznpcFYbw@public.gmane.org,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
pratik.r.sampat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Subject: Re: [RFC 1/5] ns: Introduce CPU Namespace
Date: Sun, 10 Oct 2021 00:37:39 +0200 [thread overview]
Message-ID: <20211009223739.GY174703@worktop.programming.kicks-ass.net> (raw)
In-Reply-To: <20211009151243.8825-2-psampat-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org>
On Sat, Oct 09, 2021 at 08:42:39PM +0530, Pratik R. Sampat wrote:
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 2d9ff40f4661..0413175e6d73 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -27,6 +27,8 @@
> #include "pelt.h"
> #include "smp.h"
>
> +#include <linux/cpu_namespace.h>
> +
> /*
> * Export tracepoints that act as a bare tracehook (ie: have no trace event
> * associated with them) to allow external modules to probe them.
> @@ -7559,6 +7561,7 @@ long sched_setaffinity(pid_t pid, const struct cpumask *in_mask)
> {
> cpumask_var_t cpus_allowed, new_mask;
> struct task_struct *p;
> + cpumask_t temp;
> int retval;
>
> rcu_read_lock();
You're not supposed to put a cpumask_t on stack. Those things can be
huge.
> @@ -7682,8 +7686,9 @@ SYSCALL_DEFINE3(sched_setaffinity, pid_t, pid, unsigned int, len,
> long sched_getaffinity(pid_t pid, struct cpumask *mask)
> {
> struct task_struct *p;
> + cpumask_var_t temp;
> unsigned long flags;
> - int retval;
> + int retval, cpu;
>
> rcu_read_lock();
>
> @@ -7698,6 +7703,13 @@ long sched_getaffinity(pid_t pid, struct cpumask *mask)
>
> raw_spin_lock_irqsave(&p->pi_lock, flags);
> cpumask_and(mask, &p->cpus_mask, cpu_active_mask);
> + cpumask_clear(temp);
There's a distinct lack of allocating temp before use. Are you sure you
actually tested this?
WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org>
To: "Pratik R. Sampat" <psampat@linux.ibm.com>
Cc: bristot@redhat.com, christian@brauner.io, ebiederm@xmission.com,
lizefan.x@bytedance.com, tj@kernel.org, hannes@cmpxchg.org,
mingo@kernel.org, juri.lelli@redhat.com,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
cgroups@vger.kernel.org, containers@lists.linux.dev,
containers@lists.linux-foundation.org, pratik.r.sampat@gmail.com
Subject: Re: [RFC 1/5] ns: Introduce CPU Namespace
Date: Sun, 10 Oct 2021 00:37:39 +0200 [thread overview]
Message-ID: <20211009223739.GY174703@worktop.programming.kicks-ass.net> (raw)
In-Reply-To: <20211009151243.8825-2-psampat@linux.ibm.com>
On Sat, Oct 09, 2021 at 08:42:39PM +0530, Pratik R. Sampat wrote:
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 2d9ff40f4661..0413175e6d73 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -27,6 +27,8 @@
> #include "pelt.h"
> #include "smp.h"
>
> +#include <linux/cpu_namespace.h>
> +
> /*
> * Export tracepoints that act as a bare tracehook (ie: have no trace event
> * associated with them) to allow external modules to probe them.
> @@ -7559,6 +7561,7 @@ long sched_setaffinity(pid_t pid, const struct cpumask *in_mask)
> {
> cpumask_var_t cpus_allowed, new_mask;
> struct task_struct *p;
> + cpumask_t temp;
> int retval;
>
> rcu_read_lock();
You're not supposed to put a cpumask_t on stack. Those things can be
huge.
> @@ -7682,8 +7686,9 @@ SYSCALL_DEFINE3(sched_setaffinity, pid_t, pid, unsigned int, len,
> long sched_getaffinity(pid_t pid, struct cpumask *mask)
> {
> struct task_struct *p;
> + cpumask_var_t temp;
> unsigned long flags;
> - int retval;
> + int retval, cpu;
>
> rcu_read_lock();
>
> @@ -7698,6 +7703,13 @@ long sched_getaffinity(pid_t pid, struct cpumask *mask)
>
> raw_spin_lock_irqsave(&p->pi_lock, flags);
> cpumask_and(mask, &p->cpus_mask, cpu_active_mask);
> + cpumask_clear(temp);
There's a distinct lack of allocating temp before use. Are you sure you
actually tested this?
next prev parent reply other threads:[~2021-10-09 22:37 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-09 15:12 [RFC 0/5] kernel: Introduce CPU Namespace Pratik R. Sampat
2021-10-09 15:12 ` Pratik R. Sampat
2021-10-09 15:12 ` [RFC 1/5] ns: " Pratik R. Sampat
[not found] ` <20211009151243.8825-2-psampat-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org>
2021-10-09 22:37 ` Peter Zijlstra [this message]
2021-10-09 22:37 ` Peter Zijlstra
[not found] ` <20211009151243.8825-1-psampat-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org>
2021-10-09 15:12 ` [RFC 2/5] ns: Add scrambling functionality to CPU namespace Pratik R. Sampat
2021-10-09 15:12 ` Pratik R. Sampat
2021-10-09 15:12 ` [RFC 4/5] cpu/cpuns: Make sysfs CPU namespace aware Pratik R. Sampat
2021-10-09 15:12 ` Pratik R. Sampat
2021-10-09 15:12 ` [RFC 5/5] proc/cpuns: Make procfs load stats " Pratik R. Sampat
2021-10-09 15:12 ` Pratik R. Sampat
2021-10-11 10:11 ` [RFC 0/5] kernel: Introduce CPU Namespace Christian Brauner
2021-10-11 10:11 ` Christian Brauner
2021-10-11 14:17 ` Michal Koutný
2021-10-11 14:17 ` Michal Koutný
2021-10-11 17:42 ` Tejun Heo
2021-10-12 8:42 ` Pratik Sampat
2021-10-14 22:14 ` Tejun Heo
2021-10-18 15:29 ` Pratik Sampat
2021-10-18 16:29 ` Tejun Heo
[not found] ` <YW2g73Lwmrhjg/sv-NiLfg/pYEd1N0TnZuCh8vA@public.gmane.org>
2021-10-20 10:44 ` Pratik Sampat
2021-10-20 10:44 ` Pratik Sampat
[not found] ` <77854748-081f-46c7-df51-357ca78b83b3-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org>
2021-10-20 16:35 ` Tejun Heo
2021-10-20 16:35 ` Tejun Heo
[not found] ` <YXBFVCc61nCG5rto-NiLfg/pYEd1N0TnZuCh8vA@public.gmane.org>
2021-10-21 7:44 ` Pratik Sampat
2021-10-21 7:44 ` Pratik Sampat
[not found] ` <bd1811cc-0e04-9e44-0b46-02689ff9a238-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org>
2021-10-21 17:06 ` Tejun Heo
2021-10-21 17:06 ` Tejun Heo
2021-10-21 17:15 ` Eric W. Biederman
2021-10-21 17:15 ` Eric W. Biederman
2021-10-09 15:12 ` [RFC 3/5] cpuset/cpuns: Make cgroup CPUset CPU namespace aware Pratik R. Sampat
2021-10-09 22:41 ` [RFC 0/5] kernel: Introduce CPU Namespace Peter Zijlstra
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=20211009223739.GY174703@worktop.programming.kicks-ass.net \
--to=peterz-wegcikhe2lqwvfeawa7xhq@public.gmane.org \
--cc=bristot-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=christian-STijNZzMWpgWenYVfaLwtA@public.gmane.org \
--cc=containers-cunTk1MwBs/YUNznpcFYbw@public.gmane.org \
--cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
--cc=ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org \
--cc=hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org \
--cc=juri.lelli-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=lizefan.x-EC8Uxl6Npydl57MIdRCFDg@public.gmane.org \
--cc=mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=pratik.r.sampat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=psampat-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org \
--cc=tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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: link
Be 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.