* [PATCH v2] pids: introduce find_get_task_by_vpid helper
@ 2017-10-27 17:52 ` Mike Rapoport
0 siblings, 0 replies; 8+ messages in thread
From: Mike Rapoport @ 2017-10-27 17:52 UTC (permalink / raw)
To: Andrew Morton
Cc: Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Darren Hart,
Oleg Nesterov, Balbir Singh, linux-mm, lkml, Mike Rapoport
There are several functions that do find_task_by_vpid() followed by
get_task_struct(). We can use a helper function instead.
Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
---
v2: remove futex_find_get_task() and ptrace_get_task_struct() as Oleg
suggested
include/linux/sched.h | 5 +++++
kernel/futex.c | 20 +-------------------
kernel/pid.c | 13 +++++++++++++
kernel/ptrace.c | 27 ++++++---------------------
kernel/taskstats.c | 6 +-----
mm/process_vm_access.c | 6 +-----
6 files changed, 27 insertions(+), 50 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 26a7df4e558c..4c3af5255fcf 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1484,6 +1484,11 @@ static inline struct thread_info *task_thread_info(struct task_struct *task)
extern struct task_struct *find_task_by_vpid(pid_t nr);
extern struct task_struct *find_task_by_pid_ns(pid_t nr, struct pid_namespace *ns);
+/*
+ * find a task by its virtual pid and get the task struct
+ */
+extern struct task_struct *find_get_task_by_vpid(pid_t nr);
+
extern int wake_up_state(struct task_struct *tsk, unsigned int state);
extern int wake_up_process(struct task_struct *tsk);
extern void wake_up_new_task(struct task_struct *tsk);
diff --git a/kernel/futex.c b/kernel/futex.c
index 0518a0bfc746..e2a160549a0c 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -862,24 +862,6 @@ static void put_pi_state(struct futex_pi_state *pi_state)
}
}
-/*
- * Look up the task based on what TID userspace gave us.
- * We dont trust it.
- */
-static struct task_struct *futex_find_get_task(pid_t pid)
-{
- struct task_struct *p;
-
- rcu_read_lock();
- p = find_task_by_vpid(pid);
- if (p)
- get_task_struct(p);
-
- rcu_read_unlock();
-
- return p;
-}
-
#ifdef CONFIG_FUTEX_PI
/*
@@ -1166,7 +1148,7 @@ static int attach_to_pi_owner(u32 uval, union futex_key *key,
*/
if (!pid)
return -ESRCH;
- p = futex_find_get_task(pid);
+ p = find_get_task_by_vpid(pid);
if (!p)
return -ESRCH;
diff --git a/kernel/pid.c b/kernel/pid.c
index 020dedbdf066..ead086b0ef8e 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -462,6 +462,19 @@ struct task_struct *find_task_by_vpid(pid_t vnr)
return find_task_by_pid_ns(vnr, task_active_pid_ns(current));
}
+struct task_struct *find_get_task_by_vpid(pid_t nr)
+{
+ struct task_struct *task;
+
+ rcu_read_lock();
+ task = find_task_by_vpid(nr);
+ if (task)
+ get_task_struct(task);
+ rcu_read_unlock();
+
+ return task;
+}
+
struct pid *get_task_pid(struct task_struct *task, enum pid_type type)
{
struct pid *pid;
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 84b1367935e4..6f3de14313f5 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -1099,21 +1099,6 @@ int ptrace_request(struct task_struct *child, long request,
return ret;
}
-static struct task_struct *ptrace_get_task_struct(pid_t pid)
-{
- struct task_struct *child;
-
- rcu_read_lock();
- child = find_task_by_vpid(pid);
- if (child)
- get_task_struct(child);
- rcu_read_unlock();
-
- if (!child)
- return ERR_PTR(-ESRCH);
- return child;
-}
-
#ifndef arch_ptrace_attach
#define arch_ptrace_attach(child) do { } while (0)
#endif
@@ -1131,9 +1116,9 @@ SYSCALL_DEFINE4(ptrace, long, request, long, pid, unsigned long, addr,
goto out;
}
- child = ptrace_get_task_struct(pid);
- if (IS_ERR(child)) {
- ret = PTR_ERR(child);
+ child = find_get_task_by_vpid(pid);
+ if (!child) {
+ ret = -ESRCH;
goto out;
}
@@ -1278,9 +1263,9 @@ COMPAT_SYSCALL_DEFINE4(ptrace, compat_long_t, request, compat_long_t, pid,
goto out;
}
- child = ptrace_get_task_struct(pid);
- if (IS_ERR(child)) {
- ret = PTR_ERR(child);
+ child = find_get_task_by_vpid(pid);
+ if (!child) {
+ ret = -ESRCH;
goto out;
}
diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 4559e914452b..4e62a4a8fa91 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -194,11 +194,7 @@ static int fill_stats_for_pid(pid_t pid, struct taskstats *stats)
{
struct task_struct *tsk;
- rcu_read_lock();
- tsk = find_task_by_vpid(pid);
- if (tsk)
- get_task_struct(tsk);
- rcu_read_unlock();
+ tsk = find_get_task_by_vpid(pid);
if (!tsk)
return -ESRCH;
fill_stats(current_user_ns(), task_active_pid_ns(current), tsk, stats);
diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c
index 8973cd231ece..16424b9ae424 100644
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -197,11 +197,7 @@ static ssize_t process_vm_rw_core(pid_t pid, struct iov_iter *iter,
}
/* Get process information */
- rcu_read_lock();
- task = find_task_by_vpid(pid);
- if (task)
- get_task_struct(task);
- rcu_read_unlock();
+ task = find_get_task_by_vpid(pid);
if (!task) {
rc = -ESRCH;
goto free_proc_pages;
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2] pids: introduce find_get_task_by_vpid helper
@ 2017-10-27 17:52 ` Mike Rapoport
0 siblings, 0 replies; 8+ messages in thread
From: Mike Rapoport @ 2017-10-27 17:52 UTC (permalink / raw)
To: Andrew Morton
Cc: Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Darren Hart,
Oleg Nesterov, Balbir Singh, linux-mm, lkml, Mike Rapoport
There are several functions that do find_task_by_vpid() followed by
get_task_struct(). We can use a helper function instead.
Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
---
v2: remove futex_find_get_task() and ptrace_get_task_struct() as Oleg
suggested
include/linux/sched.h | 5 +++++
kernel/futex.c | 20 +-------------------
kernel/pid.c | 13 +++++++++++++
kernel/ptrace.c | 27 ++++++---------------------
kernel/taskstats.c | 6 +-----
mm/process_vm_access.c | 6 +-----
6 files changed, 27 insertions(+), 50 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 26a7df4e558c..4c3af5255fcf 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1484,6 +1484,11 @@ static inline struct thread_info *task_thread_info(struct task_struct *task)
extern struct task_struct *find_task_by_vpid(pid_t nr);
extern struct task_struct *find_task_by_pid_ns(pid_t nr, struct pid_namespace *ns);
+/*
+ * find a task by its virtual pid and get the task struct
+ */
+extern struct task_struct *find_get_task_by_vpid(pid_t nr);
+
extern int wake_up_state(struct task_struct *tsk, unsigned int state);
extern int wake_up_process(struct task_struct *tsk);
extern void wake_up_new_task(struct task_struct *tsk);
diff --git a/kernel/futex.c b/kernel/futex.c
index 0518a0bfc746..e2a160549a0c 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -862,24 +862,6 @@ static void put_pi_state(struct futex_pi_state *pi_state)
}
}
-/*
- * Look up the task based on what TID userspace gave us.
- * We dont trust it.
- */
-static struct task_struct *futex_find_get_task(pid_t pid)
-{
- struct task_struct *p;
-
- rcu_read_lock();
- p = find_task_by_vpid(pid);
- if (p)
- get_task_struct(p);
-
- rcu_read_unlock();
-
- return p;
-}
-
#ifdef CONFIG_FUTEX_PI
/*
@@ -1166,7 +1148,7 @@ static int attach_to_pi_owner(u32 uval, union futex_key *key,
*/
if (!pid)
return -ESRCH;
- p = futex_find_get_task(pid);
+ p = find_get_task_by_vpid(pid);
if (!p)
return -ESRCH;
diff --git a/kernel/pid.c b/kernel/pid.c
index 020dedbdf066..ead086b0ef8e 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -462,6 +462,19 @@ struct task_struct *find_task_by_vpid(pid_t vnr)
return find_task_by_pid_ns(vnr, task_active_pid_ns(current));
}
+struct task_struct *find_get_task_by_vpid(pid_t nr)
+{
+ struct task_struct *task;
+
+ rcu_read_lock();
+ task = find_task_by_vpid(nr);
+ if (task)
+ get_task_struct(task);
+ rcu_read_unlock();
+
+ return task;
+}
+
struct pid *get_task_pid(struct task_struct *task, enum pid_type type)
{
struct pid *pid;
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 84b1367935e4..6f3de14313f5 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -1099,21 +1099,6 @@ int ptrace_request(struct task_struct *child, long request,
return ret;
}
-static struct task_struct *ptrace_get_task_struct(pid_t pid)
-{
- struct task_struct *child;
-
- rcu_read_lock();
- child = find_task_by_vpid(pid);
- if (child)
- get_task_struct(child);
- rcu_read_unlock();
-
- if (!child)
- return ERR_PTR(-ESRCH);
- return child;
-}
-
#ifndef arch_ptrace_attach
#define arch_ptrace_attach(child) do { } while (0)
#endif
@@ -1131,9 +1116,9 @@ SYSCALL_DEFINE4(ptrace, long, request, long, pid, unsigned long, addr,
goto out;
}
- child = ptrace_get_task_struct(pid);
- if (IS_ERR(child)) {
- ret = PTR_ERR(child);
+ child = find_get_task_by_vpid(pid);
+ if (!child) {
+ ret = -ESRCH;
goto out;
}
@@ -1278,9 +1263,9 @@ COMPAT_SYSCALL_DEFINE4(ptrace, compat_long_t, request, compat_long_t, pid,
goto out;
}
- child = ptrace_get_task_struct(pid);
- if (IS_ERR(child)) {
- ret = PTR_ERR(child);
+ child = find_get_task_by_vpid(pid);
+ if (!child) {
+ ret = -ESRCH;
goto out;
}
diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 4559e914452b..4e62a4a8fa91 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -194,11 +194,7 @@ static int fill_stats_for_pid(pid_t pid, struct taskstats *stats)
{
struct task_struct *tsk;
- rcu_read_lock();
- tsk = find_task_by_vpid(pid);
- if (tsk)
- get_task_struct(tsk);
- rcu_read_unlock();
+ tsk = find_get_task_by_vpid(pid);
if (!tsk)
return -ESRCH;
fill_stats(current_user_ns(), task_active_pid_ns(current), tsk, stats);
diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c
index 8973cd231ece..16424b9ae424 100644
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -197,11 +197,7 @@ static ssize_t process_vm_rw_core(pid_t pid, struct iov_iter *iter,
}
/* Get process information */
- rcu_read_lock();
- task = find_task_by_vpid(pid);
- if (task)
- get_task_struct(task);
- rcu_read_unlock();
+ task = find_get_task_by_vpid(pid);
if (!task) {
rc = -ESRCH;
goto free_proc_pages;
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] pids: introduce find_get_task_by_vpid helper
2017-10-27 17:52 ` Mike Rapoport
@ 2017-10-30 8:51 ` Balbir Singh
-1 siblings, 0 replies; 8+ messages in thread
From: Balbir Singh @ 2017-10-30 8:51 UTC (permalink / raw)
To: Mike Rapoport
Cc: Andrew Morton, Ingo Molnar, Peter Zijlstra, Thomas Gleixner,
Darren Hart, Oleg Nesterov, linux-mm, lkml
On Sat, Oct 28, 2017 at 4:52 AM, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> There are several functions that do find_task_by_vpid() followed by
> get_task_struct(). We can use a helper function instead.
>
> Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> ---
I did a quick grep and found other similar patterns in
kernel/events/core.c, kernel/kcmp.c
kernel/sys.c , kernel/time/posix-cpu-timers.c,
arch/x86/kernel/cpu/intel_rdt_rdtgroup.c,
security/yama/yama_lsm.c, mm/process_vm_access.c, mm/mempolicy.c and
arch/ia64/kernel/perfmon.c
Balbir Singh.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] pids: introduce find_get_task_by_vpid helper
@ 2017-10-30 8:51 ` Balbir Singh
0 siblings, 0 replies; 8+ messages in thread
From: Balbir Singh @ 2017-10-30 8:51 UTC (permalink / raw)
To: Mike Rapoport
Cc: Andrew Morton, Ingo Molnar, Peter Zijlstra, Thomas Gleixner,
Darren Hart, Oleg Nesterov, linux-mm, lkml
On Sat, Oct 28, 2017 at 4:52 AM, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> There are several functions that do find_task_by_vpid() followed by
> get_task_struct(). We can use a helper function instead.
>
> Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> ---
I did a quick grep and found other similar patterns in
kernel/events/core.c, kernel/kcmp.c
kernel/sys.c , kernel/time/posix-cpu-timers.c,
arch/x86/kernel/cpu/intel_rdt_rdtgroup.c,
security/yama/yama_lsm.c, mm/process_vm_access.c, mm/mempolicy.c and
arch/ia64/kernel/perfmon.c
Balbir Singh.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] pids: introduce find_get_task_by_vpid helper
2017-10-30 8:51 ` Balbir Singh
@ 2017-10-30 9:44 ` Mike Rapoport
-1 siblings, 0 replies; 8+ messages in thread
From: Mike Rapoport @ 2017-10-30 9:44 UTC (permalink / raw)
To: Balbir Singh
Cc: Andrew Morton, Ingo Molnar, Peter Zijlstra, Thomas Gleixner,
Darren Hart, Oleg Nesterov, linux-mm, lkml
On Mon, Oct 30, 2017 at 07:51:42PM +1100, Balbir Singh wrote:
> On Sat, Oct 28, 2017 at 4:52 AM, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> > There are several functions that do find_task_by_vpid() followed by
> > get_task_struct(). We can use a helper function instead.
> >
> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> > ---
>
> I did a quick grep and found other similar patterns in
(reordered the file list a bit)
> kernel/events/core.c,
> arch/x86/kernel/cpu/intel_rdt_rdtgroup.c,
> mm/mempolicy.c,
Those and mm/migrate.c indeed have a similar pattern, but they all do
task = pid ? find_task_by_vpid(pid) : current;
And I don't see an elegant way to use find_get_task_by_vpid() in this case.
> kernel/kcmp.c,
kcmp gets both tasks between rcu_read_lock/unlock and I think it's better
to keep it this way.
> kernel/sys.c,
There is no get_task_struct() after find_task_by_vpid(), unless I've missed
something
> kernel/time/posix-cpu-timers.c,
Here the task is selected with more complex logic than just
find_task_by_vpid()
> mm/process_vm_access.c,
Converted in the patch
> security/yama/yama_lsm.c,
> arch/ia64/kernel/perfmon.c
I've missed these two, indeed.
The arch/ia64/kernel/perfmon.c even still uses read_lock(&tasklist) rather
than rcu_read_lock()...
> Balbir Singh.
>
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] pids: introduce find_get_task_by_vpid helper
@ 2017-10-30 9:44 ` Mike Rapoport
0 siblings, 0 replies; 8+ messages in thread
From: Mike Rapoport @ 2017-10-30 9:44 UTC (permalink / raw)
To: Balbir Singh
Cc: Andrew Morton, Ingo Molnar, Peter Zijlstra, Thomas Gleixner,
Darren Hart, Oleg Nesterov, linux-mm, lkml
On Mon, Oct 30, 2017 at 07:51:42PM +1100, Balbir Singh wrote:
> On Sat, Oct 28, 2017 at 4:52 AM, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> > There are several functions that do find_task_by_vpid() followed by
> > get_task_struct(). We can use a helper function instead.
> >
> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> > ---
>
> I did a quick grep and found other similar patterns in
(reordered the file list a bit)
> kernel/events/core.c,
> arch/x86/kernel/cpu/intel_rdt_rdtgroup.c,
> mm/mempolicy.c,
Those and mm/migrate.c indeed have a similar pattern, but they all do
task = pid ? find_task_by_vpid(pid) : current;
And I don't see an elegant way to use find_get_task_by_vpid() in this case.
> kernel/kcmp.c,
kcmp gets both tasks between rcu_read_lock/unlock and I think it's better
to keep it this way.
> kernel/sys.c,
There is no get_task_struct() after find_task_by_vpid(), unless I've missed
something
> kernel/time/posix-cpu-timers.c,
Here the task is selected with more complex logic than just
find_task_by_vpid()
> mm/process_vm_access.c,
Converted in the patch
> security/yama/yama_lsm.c,
> arch/ia64/kernel/perfmon.c
I've missed these two, indeed.
The arch/ia64/kernel/perfmon.c even still uses read_lock(&tasklist) rather
than rcu_read_lock()...
> Balbir Singh.
>
--
Sincerely yours,
Mike.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] pids: introduce find_get_task_by_vpid helper
2017-10-27 17:52 ` Mike Rapoport
@ 2017-10-31 16:39 ` Oleg Nesterov
-1 siblings, 0 replies; 8+ messages in thread
From: Oleg Nesterov @ 2017-10-31 16:39 UTC (permalink / raw)
To: Mike Rapoport
Cc: Andrew Morton, Ingo Molnar, Peter Zijlstra, Thomas Gleixner,
Darren Hart, Balbir Singh, linux-mm, lkml
On 10/27, Mike Rapoport wrote:
>
> There are several functions that do find_task_by_vpid() followed by
> get_task_struct(). We can use a helper function instead.
>
> Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> ---
>
> v2: remove futex_find_get_task() and ptrace_get_task_struct() as Oleg
> suggested
Acked-by: Oleg Nesterov <oleg@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] pids: introduce find_get_task_by_vpid helper
@ 2017-10-31 16:39 ` Oleg Nesterov
0 siblings, 0 replies; 8+ messages in thread
From: Oleg Nesterov @ 2017-10-31 16:39 UTC (permalink / raw)
To: Mike Rapoport
Cc: Andrew Morton, Ingo Molnar, Peter Zijlstra, Thomas Gleixner,
Darren Hart, Balbir Singh, linux-mm, lkml
On 10/27, Mike Rapoport wrote:
>
> There are several functions that do find_task_by_vpid() followed by
> get_task_struct(). We can use a helper function instead.
>
> Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> ---
>
> v2: remove futex_find_get_task() and ptrace_get_task_struct() as Oleg
> suggested
Acked-by: Oleg Nesterov <oleg@redhat.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-31 16:39 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-27 17:52 [PATCH v2] pids: introduce find_get_task_by_vpid helper Mike Rapoport
2017-10-27 17:52 ` Mike Rapoport
2017-10-30 8:51 ` Balbir Singh
2017-10-30 8:51 ` Balbir Singh
2017-10-30 9:44 ` Mike Rapoport
2017-10-30 9:44 ` Mike Rapoport
2017-10-31 16:39 ` Oleg Nesterov
2017-10-31 16:39 ` Oleg Nesterov
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.