lkml.org 
[lkml]   [2020]   [Sep]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] kernel/pid: Rename `find_ge_pid` to `find_get_vpid`.
On Wed, Sep 02, 2020 at 12:23:10PM +0000, Harley Paterson wrote:
> Most PID functions in `kernel/pid.c` are named in the format
> [do_something]_pid, and functions with a VPID counterpart have
> the corresponding name [do_something]_vpid.
>
> `find_get_pid` and its VPID counterpart, `find_ge_pid` do not share
> this convention.
>
> This patch renames `find_ge_pid` to `find_get_vpid` to conform to the
> existing PID function naming idioms. I believe the proposed name
> makes the purpose of the function clearer.
>
> I have built and tested this patch on an x64 virtual machine, running
> Ubuntu 20.04 LTS. My tests involved building the modified kernel,
> installing it, and booting into a Ubuntu GUI session.
>
> This patch is applied on top of the linux-5.9-rc3 kernel
>
> This patch effects four lines, one in each of four files.
>
> Signed-off-by: H Paterson <harley.paterson@hotmail.co.nz>
> ---
> fs/proc/base.c | 2 +-
> include/linux/pid.h | 2 +-
> kernel/bpf/task_iter.c | 2 +-
> kernel/pid.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 617db4e0f..e2ff20311 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -3390,7 +3390,7 @@ static struct tgid_iter next_tgid(struct pid_namespace *ns, struct tgid_iter ite
> rcu_read_lock();
> retry:
> iter.task = NULL;
> - pid = find_ge_pid(iter.tgid, ns);
> + pid = find_get_vpid(iter.tgid, ns);
> if (pid) {
> iter.tgid = pid_nr_ns(pid, ns);
> iter.task = pid_task(pid, PIDTYPE_TGID);
> diff --git a/include/linux/pid.h b/include/linux/pid.h
> index 176d6cf80..2875b4189 100644
> --- a/include/linux/pid.h
> +++ b/include/linux/pid.h
> @@ -128,7 +128,7 @@ extern struct pid *find_vpid(int nr);
> * Lookup a PID in the hash table, and return with it's count elevated.
> */
> extern struct pid *find_get_pid(int nr);
> -extern struct pid *find_ge_pid(int nr, struct pid_namespace *);
> +extern struct pid *find_get_vpid(int nr, struct pid_namespace *);
>
> extern struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
> size_t set_tid_size);
> diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
> index 99af4cea1..334cb8831 100644
> --- a/kernel/bpf/task_iter.c
> +++ b/kernel/bpf/task_iter.c
> @@ -29,7 +29,7 @@ static struct task_struct *task_seq_get_next(struct pid_namespace *ns,
>
> rcu_read_lock();
> retry:
> - pid = find_ge_pid(*tid, ns);
> + pid = find_get_vpid(*tid, ns);
> if (pid) {
> *tid = pid_nr_ns(pid, ns);
> task = get_pid_task(pid, PIDTYPE_PID);
> diff --git a/kernel/pid.c b/kernel/pid.c
> index b2562a7ce..0c77fef72 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -514,7 +514,7 @@ EXPORT_SYMBOL_GPL(task_active_pid_ns);
> *
> * If there is a pid at nr this function is exactly the same as find_pid_ns.
> */
> -struct pid *find_ge_pid(int nr, struct pid_namespace *ns)
> +struct pid *find_get_vpid(int nr, struct pid_namespace *ns)

Hey Harley,

Thanks for the patch.

I think we shouldn't rename this function but I see why you got
confused.
find_ge_pid() gives you the struct pid associated with the next greatest
or equal pid number in the provided pid namespace.

A few notes why this patch is problematic:

- The "_vpid" prefix is usually used in contexts where a (virtual)
pid nr is looked up in the caller's pid namespace and some object
associated with it returned. But this takes a pid namespace argument
and so can be used to look up a pid based on a pid nr in any pid
namespace not just in the caller's.
- The "ge" here is actually a meaningful abbreviation afaict for
"greater than or equal" so replacing it with "get" would lose this
important piece of information.

- "get" is usually used - and especially here - to indicate that a
reference count is increased which isn't happening here so this would
be potentially dangerous as it would signal callers that a reference
count is incremented when it really isn't.

Thanks!
Christian

\
 
 \ /
  Last update: 2020-09-02 15:18    [W:0.034 / U:1.764 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site