OpenVZ Forum - RDF feed
type: channel
items: [RFC][PATCH] rename 'struct pid'
items: Re: [RFC][PATCH] rename 'struct pid'

[RFC][PATCH] rename 'struct pid'
type: item
description: I've been hacking quite a bit on the pidspace code. I've run into a bug or two, and had a heck of a time debugging it. Other than my inferior puny monkey brain, I'm directing some of the blame squarely in the direction of the 'struct pid'. We have pid_t, pid_ns, struct pid and pid_link, at _least_. Seeing code like get_pid(pid->pid_ns->pid_type[PIDTYPE_PID]) is mind-numbing to say the least. It makes it really hard to comprehend, and even harder to debug. We honestly have a lot of code like this: pid = pid_nr(filp->f_owner.pid); WTF? It's getting a pid from a pid? Huh? :) It makes sense when you go look at the structures, but sitting in the middle of a function and when you can't see all of the struct declarations can be really sketchy. So, I propose that we rename the one structure that seems to be the focus of the problem: 'struct pid'. Fundamentally, it is a 'process identifier': it helps the kernel to identifty processes. However, as I noted, 'pid' is a wee bit overloaded. In addition to "identifying" processes, this structure acts as an indirection or handle to them. So, I propse we call these things 'struct task_ref'. Just reading some of the code that I've modified in here makes me feel like this is the right way. Compare the two sentences below: Oh, I have a task_ref? What kind is it? Oh, it's a pgid reference because I have REFTYPE_PGID. Oh, I have a pid? What kind is it? Oh, it's a pid because I have PIDTYPE_PID. Which makes more sense? So, this still needs some work converting some of the function names, but it compiles as-is. Any ideas for better names? Signed-off-by: Dave Hansen <hansendc@us.ibm.com> --- lxc-dave/drivers/char/keyboard.c | 10 +- lxc-dave/drivers/char/n_r3964.c | 32 +++--- lxc-dave/drivers/char/tty_io.c | 66 ++++++------- lxc-dave/drivers/char/vt.c | 2 lxc-dave/drivers/char/vt_ioctl.c | 16 +-- lxc-dave/drivers/net/tun.c | 2 lxc-dave/drivers/s390/char/fs3270.c | 4 lxc-dave/drivers/usb/core/devio.c | 12 +- lxc-dave/drivers/usb/core/inode.c …
creator: Dave Hansen
date: 2007-04-10T23:20:14-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: Quoting Dave Hansen (hansendc@us.ibm.com): > > I've been hacking quite a bit on the pidspace code. I've run > into a bug or two, and had a heck of a time debugging it. > Other than my inferior puny monkey brain, I'm directing some > of the blame squarely in the direction of the 'struct pid'. > > We have pid_t, pid_ns, struct pid and pid_link, at _least_. > Seeing code like get_pid(pid->pid_ns->pid_type[PIDTYPE_PID]) > is mind-numbing to say the least. It makes it really hard > to comprehend, and even harder to debug. > > We honestly have a lot of code like this: > > pid = pid_nr(filp->f_owner.pid); > > WTF? It's getting a pid from a pid? Huh? :) > > It makes sense when you go look at the structures, but > sitting in the middle of a function and when you can't see > all of the struct declarations can be really sketchy. > > So, I propose that we rename the one structure that seems to > be the focus of the problem: 'struct pid'. Fundamentally, it > is a 'process identifier': it helps the kernel to identifty > processes. However, as I noted, 'pid' is a wee bit overloaded. > > In addition to "identifying" processes, this structure acts > as an indirection or handle to them. So, I propse we call > these things 'struct task_ref'. Just reading some of the I haven't tested the patch, but I enthusiastically, wholeheartedly, excitedly agree with this name change. -serge (I plan to run some cross-arch tests in the morning) > code that I've modified in here makes me feel like this is > the right way. > > Compare the two sentences below: > > Oh, I have a task_ref? What kind is it? Oh, it's a pgid > reference because I have REFTYPE_PGID. > > Oh, I have a pid? What kind is it? Oh, it's a pid because > I have PIDTYPE_PID. > > Which makes more sense? > > So, this still needs some work converting some of the > function names, but it compiles as-is. Any ideas for better > names? > > Signed-off-by: Dave Hansen <hansendc@us.ibm.com> > --- > > lxc-dave/drivers/char/keyboard.c | 10 +- > lx…
creator: serue
date: 2007-04-11T00:57:33-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: Dave Hansen <hansendc@us.ibm.com> writes: > I've been hacking quite a bit on the pidspace code. I've run > into a bug or two, and had a heck of a time debugging it. > Other than my inferior puny monkey brain, I'm directing some > of the blame squarely in the direction of the 'struct pid'. > > We have pid_t, pid_ns, struct pid and pid_link, at _least_. > Seeing code like get_pid(pid->pid_ns->pid_type[PIDTYPE_PID]) > is mind-numbing to say the least. get_pid(pid->pid_ns->pid_type[PIDTYPE_PID]) is complete and utter nonsense. > It makes it really hard to comprehend, and even harder to debug. Given that you quoted nonsense I can understand the comprehension problem. > We honestly have a lot of code like this: > > pid = pid_nr(filp->f_owner.pid); > > WTF? It's getting a pid from a pid? Huh? :) Clearer would be: user_pid = pid_to_user(filp->f_owner.pid); > It makes sense when you go look at the structures, but > sitting in the middle of a function and when you can't see > all of the struct declarations can be really sketchy. > > So, I propose that we rename the one structure that seems to > be the focus of the problem: 'struct pid'. NAK. > Fundamentally, it > is a 'process identifier': it helps the kernel to identifty > processes. However, as I noted, 'pid' is a wee bit overloaded. > > In addition to "identifying" processes, this structure acts > as an indirection or handle to them. So, I propse we call > these things 'struct task_ref'. Renaming the structure above doesn't help and the structure represents a pid in a more fundamental way then pid_t can. A pid (pid_t or struct pid) isn't just an identifier it is a handle to processes. struct pid just does so more directly because it is inside the kernel. > Just reading some of the > code that I've modified in here makes me feel like this is > the right way. I get exactly the opposite impression. > Compare the two sentences below: > > Oh, I have a task_ref? What kind is it? Oh, it's a pgid > reference because I have REFTYPE…
creator: ebiederm
date: 2007-04-11T01:28:02-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: On Tue, 2007-04-10 at 19:28 -0600, Eric W. Biederman wrote: > Dave Hansen <hansendc@us.ibm.com> writes: > > > I've been hacking quite a bit on the pidspace code. I've run > > into a bug or two, and had a heck of a time debugging it. > > Other than my inferior puny monkey brain, I'm directing some > > of the blame squarely in the direction of the 'struct pid'. > > > > We have pid_t, pid_ns, struct pid and pid_link, at _least_. > > Seeing code like get_pid(pid->pid_ns->pid_type[PIDTYPE_PID]) > > is mind-numbing to say the least. > > get_pid(pid->pid_ns->pid_type[PIDTYPE_PID]) is complete and utter > nonsense. Guilty as charged. It was utter nonsense. > > It makes it really hard to comprehend, and even harder to debug. > > Given that you quoted nonsense I can understand the comprehension > problem. Here are five actual examples that I see in my tree right now. Each has the string "pid" occur in it at least 4 times: struct pid_nr *pid_nr = alloc_pid_nr(ppid_nr->pid_ns, new_pid); hlist_add_head_rcu(&pid->pid_chain, &pid_hash[pid_hashfn(pid->nr)]); pid_nr = alloc_pid_nr(ppid_nr->pid_ns, new_pid); struct pid_nr *pid_nr = alloc_pid_nr(ppid_nr->pid_ns, new_pid); hlist_add_head_rcu(&pid->pid_chain, &pid_hash[pid_hashfn(pid->nr)]); What I'm telling you is that I really think that this naming is making it hard to enhance and debug this code. > > We honestly have a lot of code like this: > > > > pid = pid_nr(filp->f_owner.pid); > > > > WTF? It's getting a pid from a pid? Huh? :) > > Clearer would be: > > user_pid = pid_to_user(filp->f_owner.pid); Yes. Having a user vs. kernel process id distinction would be nice. The fact remains, though, that we call the numbers pids in userspace. We deal with processes very differently in the kernel, and we can and should make that distinction in the naming of our structures. > > It makes sense when you go look at the structures, but > > sitting in the middle of a function and when you can't see > > all of the struct declarations can be really…
creator: Dave Hansen
date: 2007-04-11T02:17:34-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: Dave Hansen <hansendc@us.ibm.com> writes: >> A pid (pid_t or >> struct pid) isn't just an identfier it is a handle to processes. >> struct pid just does so more directly because it is inside the kernel. > > Let's face it, "pid" has a meaning. It's a number. It's what you > kill(1). The meaning has been there for a long, long time. 'struct > pid' is a completely different concept, and it's certainly more than > "just a number". Yes. "pid" has a meaning. The meaning is old and well established. That meaning is not just a number, just like a file descriptor is not just a number. > So, please consider that there are actual kernel developers hacking on > this stuff who are having problem with it. The function of 'struct pid' > is great, it's a wonderful concept. It just needs a slightly different > name in order to more easily relate that concept to those that are > trying to use it. > > If anyone can think of some more incremental ways to do this, or has > other suggestions on how to make it more clear, I'm all ears. So what I have seen are examples down in the guts of the pid hash table that are problematic. And a few complaints about pid_nr. However the conversions I have done and I have looked at have just been a drop in replacement for pid_t except for reference counting issues. That to me at least is rather convincing. So I'm not convinced there is a fundamental problem. Just a bit of a problem in the guts of things where everything seems to have the same name. I'm not at all certain how a different prefix would sort that out. My feeling is that changing this just caters to people who are not going to be able to understand what is going on no matter what the structure is named, and is going to make it harder for me to update the code when I find the time to do it. Eric _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: ebiederm
date: 2007-04-11T04:52:53-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: On Tue, 2007-04-10 at 22:52 -0600, Eric W. Biederman wrote: > Dave Hansen <hansendc@us.ibm.com> writes: > >> A pid (pid_t or > >> struct pid) isn't just an identfier it is a handle to processes. > >> struct pid just does so more directly because it is inside the kernel. > > > > Let's face it, "pid" has a meaning. It's a number. It's what you > > kill(1). The meaning has been there for a long, long time. 'struct > > pid' is a completely different concept, and it's certainly more than > > "just a number". > > Yes. "pid" has a meaning. The meaning is old and well established. > That meaning is not just a number, just like a file descriptor is not > just a number. That's a great example. Userspace fds are to 'struct file' as pids are to 'struct pid', right? I actually think 'struct file' is a pretty good name. Think of what do_sys_open() might look like if we called 'struct file' 'struct fd' instead and 'fdp' instead of 'filp'. We end up with lines like: fd_install(fd, fdp); Which makes it confusing which fd we're dealing with or what the 'fd_' in the name refers to, the 'fd' or the 'fdp'. It makes quite a bit of sense to have 'fd' and 'struct file' named quite distinctly. > > So, please consider that there are actual kernel developers hacking on > > this stuff who are having problem with it. The function of 'struct pid' > > is great, it's a wonderful concept. It just needs a slightly different > > name in order to more easily relate that concept to those that are > > trying to use it. > > > > If anyone can think of some more incremental ways to do this, or has > > other suggestions on how to make it more clear, I'm all ears. > > So what I have seen are examples down in the guts of the pid hash > table that are problematic. And a few complaints about pid_nr. > > However the conversions I have done and I have looked at have just > been a drop in replacement for pid_t except for reference counting > issues. That to me at least is rather convincing. I think this is more in…
creator: Dave Hansen
date: 2007-04-11T06:31:43-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: Dave Hansen wrote: > On Tue, 2007-04-10 at 22:52 -0600, Eric W. Biederman wrote: > >>Dave Hansen <hansendc@us.ibm.com> writes: >> >>>>A pid (pid_t or >>>>struct pid) isn't just an identfier it is a handle to processes. >>>>struct pid just does so more directly because it is inside the kernel. >>> >>>Let's face it, "pid" has a meaning. It's a number. It's what you >>>kill(1). The meaning has been there for a long, long time. 'struct >>>pid' is a completely different concept, and it's certainly more than >>>"just a number". >> >>Yes. "pid" has a meaning. The meaning is old and well established. >>That meaning is not just a number, just like a file descriptor is not >>just a number. > > > That's a great example. Userspace fds are to 'struct file' as pids are > to 'struct pid', right? > > I actually think 'struct file' is a pretty good name. Think of what > do_sys_open() might look like if we called 'struct file' 'struct fd' > instead and 'fdp' instead of 'filp'. > > We end up with lines like: > > fd_install(fd, fdp); > > Which makes it confusing which fd we're dealing with or what the 'fd_' > in the name refers to, the 'fd' or the 'fdp'. It makes quite a bit of > sense to have 'fd' and 'struct file' named quite distinctly. Totally agree with Dave. Current code looks like a mess of word 'pid'. Eric, why do you object so much? it doesn't change any functionality at all just makes code a bit more readable/clear. Dave, taskref sounds a bit too much generic for me... But I can't provide some better name :/ pid - number pref (or tref) - process (task) ref, e.g. pid(filp->f_owner.pref) pref_struct - former pid_struct, e.g. struct pref_struct pref; ? Thanks, Kirill _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: dev
date: 2007-04-11T07:59:40-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: Quoting Eric W. Biederman (ebiederm@xmission.com): > Dave Hansen <hansendc@us.ibm.com> writes: > > > I've been hacking quite a bit on the pidspace code. I've run > > into a bug or two, and had a heck of a time debugging it. > > Other than my inferior puny monkey brain, I'm directing some > > of the blame squarely in the direction of the 'struct pid'. > > > > We have pid_t, pid_ns, struct pid and pid_link, at _least_. > > Seeing code like get_pid(pid->pid_ns->pid_type[PIDTYPE_PID]) > > is mind-numbing to say the least. > > get_pid(pid->pid_ns->pid_type[PIDTYPE_PID]) is complete and utter > nonsense. > > > It makes it really hard to comprehend, and even harder to debug. > > Given that you quoted nonsense I can understand the comprehension > problem. > > > We honestly have a lot of code like this: > > > > pid = pid_nr(filp->f_owner.pid); > > > > WTF? It's getting a pid from a pid? Huh? :) > > Clearer would be: > > user_pid = pid_to_user(filp->f_owner.pid); > > > > It makes sense when you go look at the structures, but > > sitting in the middle of a function and when you can't see > > all of the struct declarations can be really sketchy. > > > > So, I propose that we rename the one structure that seems to > > be the focus of the problem: 'struct pid'. > > NAK. > > > Fundamentally, it > > is a 'process identifier': it helps the kernel to identifty > > processes. However, as I noted, 'pid' is a wee bit overloaded. > > > > In addition to "identifying" processes, this structure acts > > as an indirection or handle to them. So, I propse we call > > these things 'struct task_ref'. > > Renaming the structure above doesn't help and the structure represents > a pid in a more fundamental way then pid_t can. A pid (pid_t or > struct pid) isn't just an identifier it is a handle to processes. Exactly, so why call it a 'pid == process id' if it's not just an id? If all we wanted was a kernel level process id, we could use the address of the task struct. 'struct pid' is there to provi…
creator: serue
date: 2007-04-11T13:31:17-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: On Wed, 2007-04-11 at 11:59 +0400, Kirill Korotaev wrote: > Dave, taskref sounds a bit too much generic for me... I completely agree. It's a pretty generic name. In the kernel, though it does provide lookups to tasks. I think the in-kernel task vs. process naming means that it is more consistent if we use something with "task" in it. It may be called a "process identifier" in userspace but, in the kernel, it appears to deal squarely with tasks. > But I can't provide some better name :/ > > pid - number > pref (or tref) - process (task) ref, e.g. pid(filp->f_owner.pref) > pref_struct - former pid_struct, e.g. struct pref_struct pref; Not bad. But, it would be nice to get away from pid-like names. Part of the problem with things like 'struct pid_struct' is that the structure name is nice, but people will still do: struct pid_struct pid; And we're back to square one. :( -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-04-11T15:58:20-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: "Serge E. Hallyn" <serue@us.ibm.com> writes: > Exactly, so why call it a 'pid == process id' if it's not just an id? > > If all we wanted was a kernel level process id, we could use the address > of the task struct. No we can not. The address of the task struct can change for PIDTYPE_PID when we call exec, if you don't call exec from your primary thread. Further we could not compare the pids of a session and a process group and a task if we were using addresses of struct task. > 'struct pid' is there to provide lightweight, > lasting references. So if we didn't want it to serve as a task_ref, we > wouldn't need it at all. Not at all true. Like any good solution it simultaneously has several purposes, and in fact what is the current struct pid has been in the kernel a very long time as the has table entry you find when doing a pid lookup. > Look it's not that we're saying struct pid was such a bad name, it's > that we have struct pid, struct pid_nr, struct pid_ns, pid_t, etc, and > some of those *need* to be renamed. The pid_ns is in fact a pid > namespace, the pid_nr is in fact a pid number for a task in some pid > namespace, and struct pid happens to also serve as task_ref. Renaming > just that one should suddenly make pid_nr and pid_ns much less > confusing imo. I do agree with seem to be swarming in a see of pids. Largely that is a problem in kernel/pid.c where you are expected to know what you are doing. We can easily rename something besides struct pid and get the same effect, especially in the core where everything is static. I think the later file descriptor analogy is very apt, and as such I would be happy to call it struct process_identifier. Other then that likes rename things like struct pid_nr. The one should be local to kernel/pid.c so we could call it something. Also for fixing the problem of swimming in a see of pids it is the functions, variable and field names that we have to fix not the nearly so much the structure name. I suspect if we start chang…
creator: ebiederm
date: 2007-04-11T16:03:45-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: Kirill Korotaev <dev@sw.ru> writes: > Totally agree with Dave. > Current code looks like a mess of word 'pid'. Yes the current code appears to be a mess of the word pid. > Eric, why do you object so much? it doesn't change any functionality at all > just makes code a bit more readable/clear. Because the suggestions I have seen are based on a partial understanding of struct pid, and the suggested renames will reinforce misunderstandings of what struct pid is. I think having a core data structure named in a way that suggests it is less then it is and reinforces peoples misconceptions is likely to be more of a problem for maintenance than having your eyes glaze over with seeing pid pid pid pid pid pid pid. Further if we can't find a better name a rename is simply useless noise. Please let's address the of peoples eyes glazing over but not by renaming struct pid. But by renaming the other sructures, variables of field names that surround it. Especially in kernel/pid.c where anything that is local doesn't need a global prefix. Eric _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: ebiederm
date: 2007-04-11T16:13:17-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: Dave Hansen wrote: > On Wed, 2007-04-11 at 11:59 +0400, Kirill Korotaev wrote: > >>Dave, taskref sounds a bit too much generic for me... > > > I completely agree. It's a pretty generic name. In the kernel, though > it does provide lookups to tasks. I think the in-kernel task vs. > process naming means that it is more consistent if we use something with > "task" in it. It may be called a "process identifier" in userspace but, > in the kernel, it appears to deal squarely with tasks. > > >>But I can't provide some better name :/ >> >>pid - number >>pref (or tref) - process (task) ref, e.g. pid(filp->f_owner.pref) >>pref_struct - former pid_struct, e.g. struct pref_struct pref; > > > Not bad. But, it would be nice to get away from pid-like names. Part > of the problem with things like 'struct pid_struct' is that the > structure name is nice, but people will still do: > > struct pid_struct pid; > > And we're back to square one. :( exactly! that's why I propose to call it pref_struct and do: struct pref_struct pref; i.e. to remove word "pid" from any code which is not dealing with numbers. pid(pref) macro on the other hand returns numeric identifier. Thanks, Kirill _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: dev
date: 2007-04-11T16:25:03-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: On Wed, 2007-04-11 at 10:13 -0600, Eric W. Biederman wrote: > Because the suggestions I have seen are based on a partial understanding > of struct pid, and the suggested renames will reinforce misunderstandings > of what struct pid is. Let's take a step back, then and see if we can nicely define what it does. Maybe we can come up with a bit better comment for 'struct pid'. --- 'struct pid' is a way for the kernel to get (and keep) a reference to a task (or set of tasks) with a particular numeric value. If there are any tasks with this numeric value from now on, the structure will stay pointed to those tasks. In general, the underlying task will not change, but it can in cases like when a non-thread-leader does an exec() and takes over as the leader (the pid will change). This reference will not keep the tasks from exiting. But, even if all of these tasks exit, the kernel will honor the fact that your reference exists and will not re-create any tasks with the same numeric value, as long as you allow your reference to persist. -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-04-11T16:53:28-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: Dave Hansen <hansendc@us.ibm.com> writes: > On Wed, 2007-04-11 at 10:13 -0600, Eric W. Biederman wrote: >> Because the suggestions I have seen are based on a partial understanding >> of struct pid, and the suggested renames will reinforce misunderstandings >> of what struct pid is. > > Let's take a step back, then and see if we can nicely define what it > does. Maybe we can come up with a bit better comment for 'struct > pid'. > > --- > > 'struct pid' is a way for the kernel to get (and keep) a reference to a > task (or set of tasks) with a particular numeric value. If there are > any tasks with this numeric value from now on, the structure will stay > pointed to those tasks. > > In general, the underlying task will not change, but it can in cases > like when a non-thread-leader does an exec() and takes over as the > leader (the pid will change). > > This reference will not keep the tasks from exiting. But, even if all > of these tasks exit, the kernel will honor the fact that your reference > exists and will not re-create any tasks with the same numeric value, as > long as you allow your reference to persist. The user space pid values do and should get recycled even if you are holding a reference to a struct pid. Otherwise there are some easy denial of service attacks by opening /proc/<child> directories and then having your child exit. ... A struct pid is very similar to a list symbol. Two of them can be compared for equality just by comparing pointers. You can get different information about a struct pid by examining different ''slots''. A struct pid has a name, but unlike lisp the name is numeric instead of a string. Furthermore there is a one to one mapping from all valid user space pid numbers (in a single namespace) to a kernel struct pid. Whether you compare the pointer is kernel space or the numeric user space value you will get the same answer. The kernel struct pid can however live on after it's numeric name is no longer visible to user space. The kernel …
creator: ebiederm
date: 2007-04-11T17:34:02-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: On Wed, 2007-04-11 at 11:34 -0600, Eric W. Biederman wrote: > A struct pid is very similar to a list symbol. Two of them can be compared for > equality just by comparing pointers. You can get different information about > a struct pid by examining different ''slots''. A struct pid has a > name, but unlike lisp the name is numeric instead of a string. I'm not sure what you mean by "list symbol". Is that a lisp concept? > Furthermore there is a one to one mapping from all valid user space > pid numbers (in a single namespace) to a kernel struct pid. Whether > you compare the pointer is kernel space or the numeric user space > value you will get the same answer. > > The kernel struct pid can however live on after it's numeric name is > no longer visible to user space. > > The kernel struct pid is an identifier even if it isn't numeric. > > The point of struct pid is so you can operate on pids without having > to worry about the pesky user space numeric names. Userspace refers to processes, sessions, and process groups by numbers which are finite and can wrap over time. Instead of using these numbers inside the kernel, we use a 'struct pid'. Unlike a plain number, a 'struct pid' uniquely refers to a particular process, session, or process group, and does not suffer from any wrapping effects. In general, the underlying task to which a 'struct pid' refers will not change. It is possible such as when a non-thread-leader does an exec() and takes over as the leader, tat the tasks to which a 'struct pid' refers will change. This effectively lets the kernel do a pid_t (process, session, or process group) to task lookup at a particular time and keep the results of that lookup meaningful for a long, long time, not worrying about if userspace has re-used those values. --- I think at least part of the problem is that a 'struct pid' creates a relationship to pid, as well as session ids and process groups. This is a bit muddled relationship that comes out of userspace, but it would …
creator: Dave Hansen
date: 2007-04-11T18:11:32-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: Dave Hansen <hansendc@us.ibm.com> writes: > On Wed, 2007-04-11 at 11:34 -0600, Eric W. Biederman wrote: >> A struct pid is very similar to a list symbol. Two of them can be compared > for >> equality just by comparing pointers. You can get different information about >> a struct pid by examining different ''slots''. A struct pid has a >> name, but unlike lisp the name is numeric instead of a string. > > I'm not sure what you mean by "list symbol". Is that a lisp concept? Yes. It was the most similar concept I could think of. > Userspace refers to processes, sessions, and process groups by numbers > which are finite and can wrap over time. Instead of using these numbers > inside the kernel, we use a 'struct pid'. Unlike a plain number, a > 'struct pid' uniquely refers to a particular process, session, or > process group, and does not suffer from any wrapping effects. Just like the plain number a 'struct pid' simultaneously refers to a process, session and a process group. Unlike a plain number it does not suffer any wrapping effects. > In general, the underlying task to which a 'struct pid' refers will not > change. It is possible such as when a non-thread-leader does an exec() > and takes over as the leader, tat the tasks to which a 'struct pid' > refers will change. There is no unique underlying task to which a 'struct pid' refers. > This effectively lets the kernel do a pid_t (process, session, or > process group) to task lookup at a particular time and keep the results > of that lookup meaningful for a long, long time, not worrying about if > userspace has re-used those values. > > --- > > I think at least part of the problem is that a 'struct pid' creates a > relationship to pid, as well as session ids and process groups. This is > a bit muddled relationship that comes out of userspace, but it would be > nice to unmuddle it somehow in the kernel. I think part of that might > be to give 'struct pid' a more neutral name that also encompasses the > pgrp and sid par…
creator: ebiederm
date: 2007-04-11T18:46:06-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: On Wed, 2007-04-11 at 12:46 -0600, Eric W. Biederman wrote: > > > These can be a bit confusing: > > > > struct pid *pid; > > struct pid *pgrp; > > struct pid *sid; > > How is it more confusing then? > > pid_t pid; > pid_t pgrp; > pid_t sid; They confuse me the same way. :) We can't do much about userspace. But, we do have quite a bit of control how we name things in the kernel, and I think there's a better way. Eric, we all know that you understand this stuff. I'm just trying to make it a bit more approachable for anyone that comes along in the future. Serge, Suka, Kirill and Pavel have all expressed some level of dissatisfaction, and I think we all agree that 'struct pid' is at the heart of the issue. That's a pretty large chunk of the people other than you that have tried to hack on it so far. I honestly don't think changing peripheral names is going to cut it, especially as the largest amount of confusion comes in the core code which doesn't use as many of the functions which could get new names. What would you think about running the _concept_ by Linus and akpm and seeing if they have any insights? -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-04-11T20:12:56-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: Dave Hansen <hansendc@us.ibm.com> writes: > On Wed, 2007-04-11 at 12:46 -0600, Eric W. Biederman wrote: >> >> > These can be a bit confusing: >> > >> > struct pid *pid; >> > struct pid *pgrp; >> > struct pid *sid; >> >> How is it more confusing then? >> >> pid_t pid; >> pid_t pgrp; >> pid_t sid; > > They confuse me the same way. :) > > We can't do much about userspace. But, we do have quite a bit of > control how we name things in the kernel, and I think there's a better > way. Maybe. The worst of those above is: pid_t pid; Am I correct? When someone mentions a pid which side of the above statement are you thinking of the left hand side or the right hand side. The type or the variable name. If you think of the variable name, I contend pids are very confusing. If you think of the type it is less so. Did that make sense? > Eric, we all know that you understand this stuff. I'm just trying to > make it a bit more approachable for anyone that comes along in the > future. Serge, Suka, Kirill and Pavel have all expressed some level of > dissatisfaction, and I think we all agree that 'struct pid' is at the > heart of the issue. That's a pretty large chunk of the people other > than you that have tried to hack on it so far. I haven't heard it universally said that 'struct pid' is at the heart of the issue. > I honestly don't think changing peripheral names is going to cut it, > especially as the largest amount of confusion comes in the core code > which doesn't use as many of the functions which could get new > names. Maybe. I guess it depends on what issue you are trying to solve. If the issue is a flood of the word pid lots of little things can help. If the issue is that you find the concept of pid_t confusing then it is much harder to sort this out. > What would you think about running the _concept_ by Linus and akpm and > seeing if they have any insights? They aren't likely to step in, if there is a reasonable chance we can resolve this ourselves. Eric __________________…
creator: ebiederm
date: 2007-04-11T20:54:50-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: On Wed, 2007-04-11 at 14:54 -0600, Eric W. Biederman wrote: > Dave Hansen <hansendc@us.ibm.com> writes: > > > On Wed, 2007-04-11 at 12:46 -0600, Eric W. Biederman wrote: > >> > >> > These can be a bit confusing: > >> > > >> > struct pid *pid; > >> > struct pid *pgrp; > >> > struct pid *sid; > >> > >> How is it more confusing then? > >> > >> pid_t pid; > >> pid_t pgrp; > >> pid_t sid; > > > > They confuse me the same way. :) > > > > We can't do much about userspace. But, we do have quite a bit of > > control how we name things in the kernel, and I think there's a better > > way. > > Maybe. > > The worst of those above is: > pid_t pid; > > Am I correct? Definitely. > When someone mentions a pid which side of the above statement are you > thinking of the left hand side or the right hand side. The type or > the variable name. Traditionally, I think of a pid as what I see in top. So, I think of the right hand side variable name. I think of it this way because the left hand side has little meaning in how the pid_t is going to be used. > If the issue is that you find the concept of pid_t confusing then it > is much harder to sort this out. I find pid_t confusing. There, I've said it. ;) In a perfect world, kill() wouldn't be multiplexed the way it is. We'd have kill_myself(), kill_pgrp(pgrp), kill_pid() and the pid_t passed into kill_pid() there would only mean 'process id', only and could _never_ mean 'process group id'. We could even have different data structures so that type safety would keep get_pgrp()'s result from being easily fed to kill_pgrp(). -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-04-11T21:28:47-00:00

Re: [RFC][PATCH] rename 'struct pid'
type: item
description: Dave Hansen <hansendc@us.ibm.com> writes: > On Wed, 2007-04-11 at 14:54 -0600, Eric W. Biederman wrote: >> When someone mentions a pid which side of the above statement are you >> thinking of the left hand side or the right hand side. The type or >> the variable name. > > Traditionally, I think of a pid as what I see in top. So, I think of > the right hand side variable name. I think of it this way because the > left hand side has little meaning in how the pid_t is going to be > used. I think if you looked a little more closely you would be surprised. >> If the issue is that you find the concept of pid_t confusing then it >> is much harder to sort this out. > > I find pid_t confusing. There, I've said it. ;) > > In a perfect world, kill() wouldn't be multiplexed the way it is. We'd > have kill_myself(), kill_pgrp(pgrp), kill_pid() and the pid_t passed > into kill_pid() there would only mean 'process id', only and could > _never_ mean 'process group id'. The multiplexing is an anomaly. kill(2) really is 4 different functions. kill_process(pid_t process, int sig); kill_process_group(pid_t group, int sig); kill_current_process_group(int sig); kill_all(int sig); And it is really an abuse of the data type to overload it quite they way it happens in kill(2). However that no way in validates the fact that the identifiers for session, process groups, and processes are and should be of the same time. To my knowledge the only kernel interface the brain damage of kill(2) propagates in a persistent way to is: "fcntl(F_GETOWN);" which is a tacked on interface. I admit there are a few others where the set of processes is interpreted the same as in kill(2) but on fcntl actually reads back that way. > We could even have different data structures so that type safety would > keep get_pgrp()'s result from being easily fed to kill_pgrp(). I assume here you mean to switch the type of process grouping, process vs process group or something like that. What tends to be instructive in the u…
creator: ebiederm
date: 2007-04-12T16:25:34-00:00