On Nov 27, 2007 7:49 PM, Guillaume Chazarain <[email protected]> wrote:
> [email protected] wrote:
>
> > We may be stuck with the current broken behavior for backwards
> > compatibility reasons but lets try fixing our ancient bug for the 2.6.25
> > time frame and see if anyone screams.
It's not broken. It's just not the feature you're looking for.
> I'm not screaming because of this change, but I screamed when I
> discovered I could not have a replacement for gettid() in Java, or any
> other high level environment.
Java is so high-level that it seems inappropriate to touch /proc.
It is allowed for Java to do N:M threading you know.
> So, instead of making /proc/self an unstable interface that changed in
> 2.6.0 and 2.6.25, I'll vote for /proc/self/task/self. A new interface
> that can trivially be detected for existence, and programs relying on
> this interface will loudly break on older kernels, unlike with the
> proposed interface change.
>
> Ccing Albert Cahalan as he made the change to /proc/self in the first
> place:
Changing /proc/self is somewhat risky, and probably
undesirable anyway. That file has always been used
to represent the process; at one time this also meant
the task. Documentation everywhere says "process".
This one is probably best:
/proc/task -> 123/task/456
(with both numbers showing)
The problem with /proc/self/task/self is that it
makes /proc/789/task/self be ill-defined when
the observer is not tgid 789. If the directory can
only show up in the observer's own task directory,
then this solution is good.
I really don't want to see anything that would encourage
more use of the gdb backdoor. For those that don't
remember, gdb broke when access to threads via the
top-level /proc directory was temporarily removed.
We need that back door, unfortunately, but having it
show up in symlink targets is quite nasty.
As for the history:
I left it out. At the time it would have been fairly useless.
Back then, glibc didn't make things painful by pulling
phony thread IDs out of its ass. Shell scripts sure didn't
deal in threads. Monitoring tools like "ps" didn't need it.
If nothing needs it, well, why have it?
Regarding some of the discusison on LKML, I don't see
how unshare matters. If you unshare to the point where
you get a new TGID, then /proc/self must reflect that.
* Albert Cahalan <[email protected]> wrote:
> On Nov 27, 2007 7:49 PM, Guillaume Chazarain <[email protected]> wrote:
> > [email protected] wrote:
> >
> > > We may be stuck with the current broken behavior for backwards
> > > compatibility reasons but lets try fixing our ancient bug for the 2.6.25
> > > time frame and see if anyone screams.
>
> It's not broken. It's just not the feature you're looking for.
well it's quite broken at the moment and we are looking for solutions
not for a blame game :-) You might have read the thread where i describe
what i had to go through to do something fairly trivial.
> > Ccing Albert Cahalan as he made the change to /proc/self in the first
> > place:
>
> Changing /proc/self is somewhat risky, and probably
> undesirable anyway. That file has always been used
> to represent the process; at one time this also meant
> the task. Documentation everywhere says "process".
in Linux we never truly had a notion of "process" when your change was
done - "process" always meant the task itself. That's why all the
task_struct parameters/variables used to be named 'p', not 't'. So when
NTPL came around this became a poorly defined notion.
> This one is probably best:
> /proc/task -> 123/task/456
> (with both numbers showing)
this sounds good to me. If it's a symlink then there's not much other
choice because the thread PIDs do not even show up under /proc anymore.
> The problem with /proc/self/task/self is that it
> makes /proc/789/task/self be ill-defined when
> the observer is not tgid 789. If the directory can
> only show up in the observer's own task directory,
> then this solution is good.
agreed.
> I really don't want to see anything that would encourage
> more use of the gdb backdoor. For those that don't
> remember, gdb broke when access to threads via the
> top-level /proc directory was temporarily removed.
> We need that back door, unfortunately, but having it
> show up in symlink targets is quite nasty.
>
> As for the history:
>
> I left it out. At the time it would have been fairly useless.
> Back then, glibc didn't make things painful by pulling
> phony thread IDs out of its ass. Shell scripts sure didn't
> deal in threads. Monitoring tools like "ps" didn't need it.
> If nothing needs it, well, why have it?
sound, future-proof API design, with a little bit of foresight? I am
faced with incidents on an almost daily basis that show how much we
kernel folks suck at defining new APIs. The only luck is that the set of
system calls is fairly complete already - but in the rare case where we
touch an API it's a catastrophy most of the time. With such an API track
record we'd probably never survive as a user-space project.
Ingo
Ingo Molnar <[email protected]> writes:
> * Albert Cahalan <[email protected]> wrote:
>
>> On Nov 27, 2007 7:49 PM, Guillaume Chazarain <[email protected]> wrote:
>> > [email protected] wrote:
>> >
>> > > We may be stuck with the current broken behavior for backwards
>> > > compatibility reasons but lets try fixing our ancient bug for the 2.6.25
>> > > time frame and see if anyone screams.
>>
>> It's not broken. It's just not the feature you're looking for.
>
> well it's quite broken at the moment and we are looking for solutions
> not for a blame game :-) You might have read the thread where i describe
> what i had to go through to do something fairly trivial.
Yes.
In a lot of ways if you access /proc/self and you get back information
that does not correspond to yourself the result is nonsense. Which
is a fairly mighty problem.
I'm still trying to understand which will break user space more,
adding /proc/task or changing /proc/self.
>> This one is probably best:
>> /proc/task -> 123/task/456
>> (with both numbers showing)
>
> this sounds good to me. If it's a symlink then there's not much other
> choice because the thread PIDs do not even show up under /proc anymore.
The name sounds good to me.
I am not certain the two components make sense as we have a possible
permission problem where it is remotely possible that a task will
have permission to access /proc/<tid> but not /proc/<tgid>.
We certainly need to think through that case before making it
to a stable kernel.
The reason I care is that we need to fix /proc/mounts. So once we
have /proc/task we can also have change /proc/mounts to
be a symlink to /proc/task/mounts.
Once we get the /proc/mounts thing sorted out. There are several
other entries in /proc that need to that need to follow in it's wake
as they also become per namespace. /proc/net and /proc/sysvipc for
starters.
Eric
[email protected] (Eric W. Biederman) writes:
> I am not certain the two components make sense as we have a possible
> permission problem where it is remotely possible that a task will
> have permission to access /proc/<tid> but not /proc/<tgid>.
Got it. I can totally avoid in permission issues by having a
follow_link method that just goes to the target directory without
checking permissions as we go.
So in the worst case with weird selinux permission rules you
might be able to access /proc/task but not /proc/self or
/proc/task/..
At least for what I care about, weird cases with unshare where the
mounts and the other namespaces may be different between threads in
someones home rolled thread package that uses CLONE_THREAD
we should be ok.
Eric
On Nov 28, 2007 6:31 AM, Eric W. Biederman <[email protected]> wrote:
> Ingo Molnar <[email protected]> writes:
> > * Albert Cahalan <[email protected]> wrote:
> >> On Nov 27, 2007 7:49 PM, Guillaume Chazarain <[email protected]> wrote:
> In a lot of ways if you access /proc/self and you get back information
> that does not correspond to yourself the result is nonsense. Which
> is a fairly mighty problem.
In general, this is not a problem we have.
/proc/self points to the process, not the task group leader.
They are different. Look at /proc/*/stat, where the
per-process info is summary data. The per-thread
stat file is not summary data. This is intended to be
true for all files in /proc; there may be some with bugs.
Some of the data can not be summed up and will not
always be shared. This is legacy crud. Don't use it,
and don't try to "fix" it. It's there so that old programs
can continute to work as long as weird threading isn't
in use.
Note that it was intended that non-legacy additions
would normally be added to either the process directory
or the thread directory, not both. I think somebody may
have ripped out the ability to do this; at the very least
there have been numerous illogical additions.
> I'm still trying to understand which will break user space more,
> adding /proc/task or changing /proc/self.
Changing /proc/self makes you get per-thread data
when you asked for per-process data. That's bad.
> >> This one is probably best:
> >> /proc/task -> 123/task/456
> >> (with both numbers showing)
> >
> > this sounds good to me. If it's a symlink then there's not much other
> > choice because the thread PIDs do not even show up under /proc anymore.
>
> The name sounds good to me.
>
> I am not certain the two components make sense as we have a possible
> permission problem where it is remotely possible that a task will
> have permission to access /proc/<tid> but not /proc/<tgid>.
If it hurts, don't do that. We allow foot shooting.
> The reason I care is that we need to fix /proc/mounts. So once we
> have /proc/task we can also have change /proc/mounts to
> be a symlink to /proc/task/mounts.
>
> Once we get the /proc/mounts thing sorted out. There are several
> other entries in /proc that need to that need to follow in it's wake
> as they also become per namespace. /proc/net and /proc/sysvipc for
> starters.
As I predicted, the container bloat would be a never-ending
source of bugs. You're discovering bugs where there were none.
You'll never run out of this sort of problem. Keeping Linux lean
and simple would be far better.
On Nov 28, 2007 5:46 AM, Ingo Molnar <[email protected]> wrote:
> * Albert Cahalan <[email protected]> wrote:
> > On Nov 27, 2007 7:49 PM, Guillaume Chazarain <[email protected]> wrote:
> > > [email protected] wrote:
> > >
> > > > We may be stuck with the current broken behavior for backwards
> > > > compatibility reasons but lets try fixing our ancient bug for the 2.6.25
> > > > time frame and see if anyone screams.
> >
> > It's not broken. It's just not the feature you're looking for.
>
> well it's quite broken at the moment and we are looking for solutions
> not for a blame game :-) You might have read the thread where i describe
> what i had to go through to do something fairly trivial.
In some ways that is NOT trivial, given that a high-level
language is free to use N:M threading.
If we assume that isn't allowed though, blaming the library
for not using native Linux thread IDs is entirely reasonable.
Linus picked sane ID numbering, not Solaris-style. Normal
app developers are unable to take advantage of Linus'
wise decision.
> > Changing /proc/self is somewhat risky, and probably
> > undesirable anyway. That file has always been used
> > to represent the process; at one time this also meant
> > the task. Documentation everywhere says "process".
>
> in Linux we never truly had a notion of "process" when your change was
> done - "process" always meant the task itself. That's why all the
> task_struct parameters/variables used to be named 'p', not 't'. So when
> NTPL came around this became a poorly defined notion.
We were sort of settling on "struct signal" as the process.
> > This one is probably best:
> > /proc/task -> 123/task/456
> > (with both numbers showing)
>
> this sounds good to me. If it's a symlink then there's not much other
> choice because the thread PIDs do not even show up under /proc anymore.
>
> > The problem with /proc/self/task/self is that it
> > makes /proc/789/task/self be ill-defined when
> > the observer is not tgid 789. If the directory can
> > only show up in the observer's own task directory,
> > then this solution is good.
>
> agreed.
>
> > I really don't want to see anything that would encourage
> > more use of the gdb backdoor. For those that don't
> > remember, gdb broke when access to threads via the
> > top-level /proc directory was temporarily removed.
> > We need that back door, unfortunately, but having it
> > show up in symlink targets is quite nasty.
> >
> > As for the history:
> >
> > I left it out. At the time it would have been fairly useless.
> > Back then, glibc didn't make things painful by pulling
> > phony thread IDs out of its ass. Shell scripts sure didn't
> > deal in threads. Monitoring tools like "ps" didn't need it.
> > If nothing needs it, well, why have it?
>
> sound, future-proof API design, with a little bit of foresight?
Yes, in a way. Adding stuff is usually easier than removing
stuff. I couldn't decide between /proc/self/task/self and /proc/task,
so I left the decision for later. I wasn't sure that I'd thought of
all the issues.
> I am
> faced with incidents on an almost daily basis that show how much we
> kernel folks suck at defining new APIs. The only luck is that the set of
> system calls is fairly complete already - but in the rare case where we
> touch an API it's a catastrophy most of the time. With such an API track
> record we'd probably never survive as a user-space project.
Most of user-space is worse.
What shocks me is that people keep designing ABIs with structs
that contain holes. (data leaks, waste, portability trouble, etc.)
This happens in kernel ABIs all the time. It ought to be blocked
by some sort of build tool. (with a whitelist for old stuff)
Another shocker is /proc/*/smaps, which should make you cry.
At the time I was working too much overtime to post about it,
and I figured that nobody would allow that into the kernel anyway...
Speaking of which, that's one that has no need to be in the task
directories. I put a maps file there to make porting old code easier,
but neither one really belongs. It's per-mm, which was in a 1:n
relationship with struct signal last I checked.
* Albert Cahalan <[email protected]> wrote:
> > I am faced with incidents on an almost daily basis that show how
> > much we kernel folks suck at defining new APIs. The only luck is
> > that the set of system calls is fairly complete already - but in the
> > rare case where we touch an API it's a catastrophy most of the time.
> > With such an API track record we'd probably never survive as a
> > user-space project.
>
> Most of user-space is worse.
most of user-space is a 'leaf' node product, not infrastructure. The
core infrastructure bits of userspace like glibc have a lot saner API
practice than the kernel. (with a 10 times larger API exposure! There
are 3000+ glibc APIs, contrast that with the few-hundred kernel APIs.)
> What shocks me is that people keep designing ABIs with structs that
> contain holes. (data leaks, waste, portability trouble, etc.) This
> happens in kernel ABIs all the time. It ought to be blocked by some
> sort of build tool. (with a whitelist for old stuff)
what shocks me more is the wide gap between talk and deed ;-) Really,
you had strong (and mostly unjust) words towards the containers stuff,
with little constructive criticism - while what started out this
discussion was your (in hindsight) shortsighted choice of mucking with
/proc/self years ago. We all make mistakes but you seem to even be proud
of it. If your goal is to bring Linux forward please become more
constructive and cut out the insults towards others. (and you'll
magically see less insults go in your general direction as well ;-)
Ingo
"Albert Cahalan" <[email protected]> writes:
> On Nov 28, 2007 6:31 AM, Eric W. Biederman <[email protected]> wrote:
>> Ingo Molnar <[email protected]> writes:
>> > * Albert Cahalan <[email protected]> wrote:
>> >> On Nov 27, 2007 7:49 PM, Guillaume Chazarain <[email protected]> wrote:
>
>> In a lot of ways if you access /proc/self and you get back information
>> that does not correspond to yourself the result is nonsense. Which
>> is a fairly mighty problem.
>
> In general, this is not a problem we have.
True in general we do not have a problem, in specific corner cases
we do have a problem, and being a system software developer corner
cases that do not work properly disturb me.
Linux does not have processes.
Linux has tasks
Linux has task groups.
Linux tasks when used in one particular way can fulfill the posix
requirements for single threaded processes.
Linux task groups when used in one particular way can fulfill the
posix requirements for processes.
Prior to 2.6 and good threading support task and process were
used interchangeably in linux.
Claiming something linux specific should be one way or another way
because the documentation said process is not an argument but rather
an indication that the documentation was ambiguous.
When the CLONE_THREAD support was added in 2.5 we left a lot of
corner cases ill-defined and not thought through and that continues
to give us trouble. In particular peoples intuition varies on what
the right set of semantics in a different situations are.
Regardless of the original intent the way the files in /proc are today
is that both /proc/<tgid>/ and /proc/<tgid>/task/<tid>/ contain all of
the same status files with the files under /proc/<tgid>/ containing
information summed across the entire thread group. To preserve
the pre thread group understanding of /proc this needs to continue
to be the case. Otherwise the /proc/<tgid>/ directory would quickly
become uninteresting to everything except tools like ps and top.
As for where /proc/self points given that procps seems to read
files like /proc/self/stat. It looks to me like we have a clear
case of a user space application that cares about the current
behavior and would break if we changed things.
> Note that it was intended that non-legacy additions
> would normally be added to either the process directory
> or the thread directory, not both. I think somebody may
> have ripped out the ability to do this; at the very least
> there have been numerous illogical additions.
The rationale was not conveyed and the policy you describe
seems like deprecating the /proc/<tgid> directory in favor
of the /proc/<tgid>/task/<pid>/. Which was a pattern
never established and it doesn't seem to make anything better
so I don't see the point there.
>> I'm still trying to understand which will break user space more,
>> adding /proc/task or changing /proc/self.
>
> Changing /proc/self makes you get per-thread data
> when you asked for per-process data. That's bad.
/proc/self used to ask for per task data. Which is why there
is some confusion.
>> >> This one is probably best:
>> >> /proc/task -> 123/task/456
>> >> (with both numbers showing)
>> >
>> > this sounds good to me. If it's a symlink then there's not much other
>> > choice because the thread PIDs do not even show up under /proc anymore.
>>
>> The name sounds good to me.
I will see about writing the patch for this in a bit and sending
it to Andrew.
>> I am not certain the two components make sense as we have a possible
>> permission problem where it is remotely possible that a task will
>> have permission to access /proc/<tid> but not /proc/<tgid>.
>
> If it hurts, don't do that. We allow foot shooting.
Well as I replied earlier I have a solution to this so it isn't
an issue.
However we don't go around pointing guns at peoples feet. If
there isn't a reason for problems we should not do it. Now
I do admit security modules in general and selinux in particular
seem to be foot shooting by definition but that is another issue
entirely. Waving too many problems away as foot shooting is a
reckless attitude can cause problems later.
> As I predicted, the container bloat would be a never-ending
> source of bugs. You're discovering bugs where there were none.
Nope. /proc/mounts was a symlink to /proc/self/mounts long before
/proc/self was modified to stop pointing at the task directory and
changed it point at the new task group directory.
Frankly from what I have seen of the code the task-group work
seems to be a larger source of bugs, and complications, because
people have a darn hard time wrapping their head around how it
is supposed to behave, and all of the corner cases were not
resolved at the time it was developed.
My favorite ongoing issue is what is needed to allow a threaded
init to actually function properly. I think enough fixes have
gone in that it might even work.
> You'll never run out of this sort of problem. Keeping Linux lean
> and simple would be far better.
Nah. The control group stuff has all kinds of corner cases because
it is a new and untested API. The namespace work after we get the
code cleanup up so it is maintainable and we can work with it
is usually just finding our globals through a pointer instead of
from a static variable. Hardly a measurable cost on the best day.
The rule is that the user space interfaces change as little as
possible. For /proc that could mean either we capture all of
the namespaces at mount time so things stay exactly the way
the are today, or that we find a way to show things that have
now become namespace dependent. Given that monitoring works
best if you can access the status files of other namespaces
it seems to make sense to follow the lead of /proc mounts.
Although I believe we should shortly reach the point where
/proc/.../mounts will have the same inode number if it is
the same mount namespace.
Eric
* Eric W. Biederman <[email protected]> wrote:
> > You'll never run out of this sort of problem. Keeping Linux lean and
> > simple would be far better.
>
> Nah. The control group stuff has all kinds of corner cases because it
> is a new and untested API. The namespace work after we get the code
> cleanup up so it is maintainable and we can work with it is usually
> just finding our globals through a pointer instead of from a static
> variable. Hardly a measurable cost on the best day.
yeah - anyone who claims that containers are 'fat' has likely not even
looked at the code. Even maintainance-wise there's very visible positive
effects: we do discover and properly map our "global resource"
dependencies and abstract them. That increases cleanliness of our code
and APIs all around.
Ingo
On Nov 29, 2007 4:40 PM, Eric W. Biederman <[email protected]> wrote:
> "Albert Cahalan" <[email protected]> writes:
>
> > On Nov 28, 2007 6:31 AM, Eric W. Biederman <[email protected]> wrote:
> >> Ingo Molnar <[email protected]> writes:
> >> > * Albert Cahalan <[email protected]> wrote:
> >> >> On Nov 27, 2007 7:49 PM, Guillaume Chazarain <[email protected]> wrote:
> Linux tasks when used in one particular way can fulfill the posix
> requirements for single threaded processes.
>
> Linux task groups when used in one particular way can fulfill the
> posix requirements for processes.
Right. Once you leave this, weirdness happens.
POSIX defines things in terms of processes and threads.
POSIX defines many of our interfaces. That includes
kernel behavior, the C library, and numerous programs.
> As for where /proc/self points given that procps seems to read
> files like /proc/self/stat. It looks to me like we have a clear
> case of a user space application that cares about the current
> behavior and would break if we changed things.
I wasn't saying procps would break, though it would if
/proc/self/task went away. I'm more concerned about
multi-threaded things that look in their own /proc/self
directory. The procps programs are single-threaded.
In procps, the self link is used:
a. to see if the wchan file exists
b. to see if the task directory exists
c. to find the tty number
(that last one: there might not be a file descriptor
for the tty, and anyway I need it with the bits in all
the same places as what I get for the other processes)
I'll bet that something reads /proc/self/stat to see
CPU usage.
> > Note that it was intended that non-legacy additions
> > would normally be added to either the process directory
> > or the thread directory, not both. I think somebody may
> > have ripped out the ability to do this; at the very least
> > there have been numerous illogical additions.
>
> The rationale was not conveyed and the policy you describe
> seems like deprecating the /proc/<tgid> directory in favor
> of the /proc/<tgid>/task/<pid>/. Which was a pattern
> never established and it doesn't seem to make anything better
> so I don't see the point there.
For the stuff that is logically per-task, yes.
For the rest, no. Oh well...
It does make things better because redundant info
is a source of confusion.
> >> I'm still trying to understand which will break user space more,
> >> adding /proc/task or changing /proc/self.
> >
> > Changing /proc/self makes you get per-thread data
> > when you asked for per-process data. That's bad.
>
> /proc/self used to ask for per task data. Which is why there
> is some confusion.
Heh. Well, /proc/self used to ask for per process data.
It was all the same. I think it matters that /proc/self was
always documented as being per-process.
> >> >> This one is probably best:
> >> >> /proc/task -> 123/task/456
> >> >> (with both numbers showing)
> >> >
> >> > this sounds good to me. If it's a symlink then there's not much other
> >> > choice because the thread PIDs do not even show up under /proc anymore.
> >>
> >> The name sounds good to me.
>
> I will see about writing the patch for this in a bit and sending
> it to Andrew.
Nice.
> Nope. /proc/mounts was a symlink to /proc/self/mounts long before
> /proc/self was modified to stop pointing at the task directory and
> changed it point at the new task group directory.
Having the filesystem namespace be per-process is wild enough.
We really don't need it to be per-thread. (and yes, I'm using the
POSIX terms on purpose)
> Frankly from what I have seen of the code the task-group work
> seems to be a larger source of bugs, and complications, because
> people have a darn hard time wrapping their head around how it
> is supposed to behave, and all of the corner cases were not
> resolved at the time it was developed.
People look at me like I have two heads when I explain to
them that the Linux kernel source uses "pid" to mean
a thread. The bad terminology probably promotes bad thinking.
It would be lovely if that could somehow get fixed.
> My favorite ongoing issue is what is needed to allow a threaded
> init to actually function properly. I think enough fixes have
> gone in that it might even work.
My "favorite" is the multi-threaded debugger. By this I
mean the debugger itself wants to be multi-threaded,
issuing ptrace commands from multiple threads.
"Albert Cahalan" <[email protected]> writes:
>> >> >> This one is probably best:
>> >> >> /proc/task -> 123/task/456
>> >> >> (with both numbers showing)
>> >> >
>> >> > this sounds good to me. If it's a symlink then there's not much other
>> >> > choice because the thread PIDs do not even show up under /proc anymore.
>> >>
>> >> The name sounds good to me.
>>
>> I will see about writing the patch for this in a bit and sending
>> it to Andrew.
>
> Nice.
Actually I think I will call it /proc/current. That names seems to convey
a little better what we are after.
Eric