Christian Brauner [off-list ref] writes:
On Mon, Nov 19, 2018 at 07:59:24AM -0800, Daniel Colascione wrote:
quoted
You never addressed my comment on the previous patch about your use of
Sorry, that thread exploded so quickly that I might have missed it.
quoted
private_data here. Why can't you use the struct pid reference that's
already in the inode?
If that's what people prefer we can probably use that. There was
precedent for stashing away such data in fs/proc/base.c already for
various other things including user namespaces and struct mm so I
followed this model. A prior version of my patch (I didn't send out) did
retrive the inode via proc_pid() in .open() took an additional reference
via get_pid() and dropped it in .release(). Do we prefer that?
If you are using proc/<pid>/ directories as your file descriptors, you
don't need to add an open or a release method at all. The existing file
descriptors hold a reference to the inode which holds a reference the
the struct pid.
The only time you need to get a reference is when you need a task
and kill_pid_info already performs that work for you.
So using proc_pid is all you need to do to get the pid from the existing
file descriptors.
Eric
On Mon, Nov 19, 2018 at 01:02:06PM -0600, Eric W. Biederman wrote:
Christian Brauner [off-list ref] writes:
quoted
On Mon, Nov 19, 2018 at 07:59:24AM -0800, Daniel Colascione wrote:
quoted
You never addressed my comment on the previous patch about your use of
Sorry, that thread exploded so quickly that I might have missed it.
quoted
private_data here. Why can't you use the struct pid reference that's
already in the inode?
If that's what people prefer we can probably use that. There was
precedent for stashing away such data in fs/proc/base.c already for
various other things including user namespaces and struct mm so I
followed this model. A prior version of my patch (I didn't send out) did
retrive the inode via proc_pid() in .open() took an additional reference
via get_pid() and dropped it in .release(). Do we prefer that?
If you are using proc/<pid>/ directories as your file descriptors, you
don't need to add an open or a release method at all. The existing file
descriptors hold a reference to the inode which holds a reference the
the struct pid.
The only time you need to get a reference is when you need a task
and kill_pid_info already performs that work for you.
Oh, I see what you and Andy are saying now. Sweet, that means we can
trim down the patch even more. Less code, less headache.
Thanks!
So using proc_pid is all you need to do to get the pid from the existing
file descriptors.
Eric
Yep. That's also what I was talking about, FWIW.
On Mon, Nov 19, 2018 at 11:31 AM, Christian Brauner
[off-list ref] wrote:
On Mon, Nov 19, 2018 at 01:02:06PM -0600, Eric W. Biederman wrote:
quoted
Christian Brauner [off-list ref] writes:
quoted
On Mon, Nov 19, 2018 at 07:59:24AM -0800, Daniel Colascione wrote:
quoted
You never addressed my comment on the previous patch about your use of
Sorry, that thread exploded so quickly that I might have missed it.
quoted
private_data here. Why can't you use the struct pid reference that's
already in the inode?
If that's what people prefer we can probably use that. There was
precedent for stashing away such data in fs/proc/base.c already for
various other things including user namespaces and struct mm so I
followed this model. A prior version of my patch (I didn't send out) did
retrive the inode via proc_pid() in .open() took an additional reference
via get_pid() and dropped it in .release(). Do we prefer that?
If you are using proc/<pid>/ directories as your file descriptors, you
don't need to add an open or a release method at all. The existing file
descriptors hold a reference to the inode which holds a reference the
the struct pid.
The only time you need to get a reference is when you need a task
and kill_pid_info already performs that work for you.
Oh, I see what you and Andy are saying now. Sweet, that means we can
trim down the patch even more. Less code, less headache.
Thanks!
quoted
So using proc_pid is all you need to do to get the pid from the existing
file descriptors.
Eric