[Toybox] [PATCH] Revert "Fix "ps -T 1234" to show thread belonging to that PID."

[enh]

On Sat, Feb 10, 2018 at 10:40 AM, Rob Landley <rob at landley.net> wrote:
> Ok, trying again at ps -T.
>
> On 01/24/2018 10:42 AM, enh wrote:
>> random example on my laptop:
>>
>> /tmp/toybox$ ./toybox ps -AT
>> ...
>> 244083 244083 ?      00:17:32 chrome
>> 244083 244084 ?      00:14:38 chrome
>> 244083 244085 ?      00:14:38 chrome
>> 244083 244086 ?      00:14:38 chrome
> $ ps -T 2273
>   PID  SPID TTY      STAT   TIME COMMAND
>  2273  2273 ?        SLl  645:52
> /usr/lib/chromium-browser/chromium-browser --e
>  2273  2300 ?        SLl    7:58
> /usr/lib/chromium-browser/chromium-browser --e
>  2273  2305 ?        SLl    0:00
> /usr/lib/chromium-browser/chromium-browser --e
>  2273  2309 ?        SLl    0:01
> /usr/lib/chromium-browser/chromium-browser --e
>
> I didn't notice because that's what the host ps -T was doing. (And
> obviously it's not showing the _same_ structure or the TID wouldn't vary
> either.)

yeah, i should have been clearer that "all the threads are getting the
same CPU time in the toybox case".

>> /tmp/toybox$ git revert 416397e14858c75a9bf20d05f7729595e03943df
>> [master 0e3dbdb] Revert "Fix "ps -T 1234" to show thread belonging to that PID."
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> (rebuild)
>>
>> /tmp/toybox$ ./toybox ps -AT
>> ...
>> 244083 244083 ?      00:17:32 chrome
>> 244083 244084 ?      00:00:00 TaskSchedulerSe
>> 244083 244085 ?      00:00:01 TaskSchedulerRe
>> 244083 244086 ?      00:00:01 TaskSchedulerRe
>
> What columns is that displaying? (You cut that part out... /me diverges
> into android plumbing a bit until I figure out "right, you're showing me
> the ubuntu version, not the android version"....)
>
> Anyway, it's with -T it's -o PID,TID,TIME,CMD and I get:
>
> $ ./ps -AT | grep -w 2273
>  2273  2273 ?        15:53:23 chromium-browse
>  2273  2300 ?        00:07:58 sandbox_ipc_thr
>  2273  2305 ?        00:00:00 chromium-browse
>  2273  2309 ?        00:00:01 gdbus
>
> The patch was:
>
> -  if (TT.threadparent && TT.threadparent->extra)
> -    if (*slot == *(((struct carveup *)TT.threadparent->extra)->slot))
> return 0;
> +  if (TT.threadparent && TT.threadparent->extra) {
> +    *slot = *(((struct carveup *)TT.threadparent->extra)->slot);
> +    // Parent also shows up as a thread, discard duplicate
> +    if (*slot == tb->slot[SLOT_tid]) return 0;
> +  }
>
> (Dear thunderbird developers, despite having three tabs under
> preferences->composition, letting me specify what column to wrap at
> never occurred to you. You are _operatically_ bad at user interface stuff.)
>
> Old logic: if this is a thread, if this thread's PID matches the
> parent's PID, return 0.
>
> New logic: if this is a thread, copy the parent's PID into this thread's
> PID field. (Which is not the same as the TID field.) Then if that PID is
> equal to the TID, bail because it's a duplicate (process also showing up
> as first thread).
>
> So the only differences afterwards are either we returned early (which
> would skip the field) or we assigned a different value to *slot... Which
> is then used in filenames for opening all the _other_ files under
> /proc/$PID/task and will now all be opening the main thread's files.
>
> Sigh. Ok, switch those to opening tid, not pid. (They're the same for
> processes so it should still work in the normal case.)

yeah, the new patch seems to work for the cases i tried. merged
yesterday. no shouting yet :-)

> Should get_sched_policy(*slot, (void *)&slot[SLOT_pcy]) be using the PID
> or the TID? I'm not sure if threads have their own scheduling policy or
> what, but I'm erring on the side of "what -AT does now", so switching
> that to tid too.

yes, it's a tid:

lib/portability.h:static inline int get_sched_policy(int tid, void
*policy) {return 0;}

> $ ./ps -AT 2273
>   PID   TID TTY          TIME CMD
>  2273  2273 ?        15:53:34 chromium-browse
>  2273  2300 ?        00:07:58 sandbox_ipc_thr
>  2273  2305 ?        00:00:00 chromium-browse
>  2273  2309 ?        00:00:01 gdbus
>  2273  2310 ?        00:00:01 NetworkChangeNo
>
> Rob