[Toybox] ps down, top to go

[Rob Landley]

On 05/20/2016 04:57 PM, enh wrote:
> On Fri, May 20, 2016 at 2:30 PM, Rob Landley <rob at landley.net> wrote:
>> Threads have nothing in /proc/$$/cmdline, so command and friends have
>> nothing to show and fall back to showing kernel thread, yes.
>>
>> It sounds like the behavior you _want_ is for one of them to show the
>> $PID command line for this $TID? I.E. show some OTHER process's command
>> line because threads have magic relationships that ps needs to learn
>> about. Most likely this should be CMDLINE doing it.
> 
> yes.
> 
> for us, where everything's threaded, it's getpid() == gettid() that's
> the weird special case.

I'm pretty sure every thread group should have one of those, so it's not
THAT much of a special case.

Digging into the code get_ps takes a struct dirtree * which has a
->parent pointing to the base process for the threads, so I actually
_do_ have access to that info and can just snapshot it at get time.

The "replace slot -1" thing is easiest to implement, and since the zero
length command line stored the same information as pid != tid (I.E.
Admiral Ackbar telling us "it's a thread!"), no actual data is lost.

I think if I do that, I still want COMM to stick [brackets] around
thread names because it makes 'em stand out more, but I can add a tid !=
pid test for that. (And keep the old one for kernel threads, which have
no parent.)

>> So, what we gotta do is snapshot the data into toybuf. I can add another
>> entry to the fetch[] array at the start of get_ps() and have that be a
>> zero length string for non-threads but a copy of the parent process's
>> command line for threads, and then have CMDLINE print that if it's
>> non-null, otherwise fall back to previous behavior. Actually I can be
>> slimy about initializing struct carveup offset[6] so it only points to
>> the new entry if there is one, and points to
> 
> assuming you were going to finish that sentence more reasonably than
> "yo momma", yes, that sounds like what toolbox did.

I think I figured out a simpler way to do it. :)

>> The question is, which -o fields should show slot -7 and which should
>> show slot -1? I.E. when do I do the current [thread] behavior, and when
>> do I lie and show the parent's command line instead?
> 
> i think that your existing heuristics are fine for ps. i think it's
> only top (where you lose the relationship between pid and tids) where
> it matters.

Top defaults to showing args, not cmd/comm, so under the new way of
doing stuff threads would get command lines there by default.

> toolbox top wasn't trying to be consistent with ps, and
> neither supported all the traditional CMD/COMM/CMDLINE/ARGS variants,
> so effectively just went with a new field that's explicitly the
> process name. so if you don't mind adding yet another field name, i
> think that's the simplest option (and simplest to explain in the
> --help output too).

Or I can do that, but when you say "process name" do you mean:

1) stat[2]
2) argv[0] with path
3) argv[0] minus path
4) argv[]
5) something else?

That's it, I'm breaking the 6 we've already got out into their own
section in the help text...

> the primary use case is that in a bug report we include a top, and the
> last two columns will be the thread name and process name. i guess the
> interesting question is "what should interactive top -H show?". looks
> like desktop top uses thread name, so if you change nothing, you'll be
> consistent with existing expectations :-)

*blink* *blink*

I just tried Ubuntu's top and scrolled right because in mine that
changes the sort fields, and instead it scrolled the display to the side
but didn't make the output longer.

Right. (And they're highlighting the "R" processes. I wonder if I should
do that?)

Grrr. Their "top -H" takes the same cpu as my "top", but MY top -H is
taking 10% of the cpu on my netbook. And it's all the data collection
phase, not display. (At least now that I did buffered versions of
getpwuid and getgrgid anyway.) I can tell because cursor
left/right/left/right doesn't hugely increase the CPU usage even though
I'm doing 5 redisplays per refresh.

(What would be NICE here is if %cpu could split into separate system and
user percentages. I haven't got display fields for those. Would it be
cheating to add them to profile top itself? Later, maybe... :)

Anyway, yes, it's showing Chrome_IOTh+ as one of the persistent cpu hogs
on my system, and that's not an executable name.

Right, lemme wait for you to answer my list question in the previous
reply hunk before trying to figure out what "correct behavior" looks
like in this complicated specless program suite. :P

>>>>>> even though you hate them, this is one of the nice things about long
>>>>>> options. they're easier to remember, and no one cares that you've
>>>>>> already taken --list-fields because they're not likely to want
>>>>>> --list-fields to mean anything else.
>>>>>
>>>>> I just like there to be a short option corresponding to each long option.
>>>>
>>>> many long options just aren't worth a short option.
>>
>> If they aren't worth a short option, are they worth _having_?
> 
> i don't use a toilet plunger often, but when i do i'm usually pretty
> glad it was there :-)

I agree --bare-longopts and toilet plungers are analogous, yes. Neither
is really my first choice of user interface for a given problem, and
avoiding the need for them is probably worth a little effort.

Sometimes they're unavoidable, and I _did_ make infrastructure in
lib/args.c to handle them...

Rob