<div dir="auto">I'm talking about the pid_max problem. See the patch inlined. </div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 14, 2019, 10:21 Dmitry Shmidt <<a href="mailto:dimitrysh@google.com">dimitrysh@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div dir="ltr">On Thu, Nov 14, 2019 at 8:32 AM enh <<a href="mailto:enh@google.com" target="_blank" rel="noreferrer">enh@google.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, Feb 6, 2019 at 3:44 PM enh <<a href="mailto:enh@google.com" target="_blank" rel="noreferrer">enh@google.com</a>> wrote:<br>
><br>
> On Wed, Feb 6, 2019 at 2:34 PM Rob Landley <<a href="mailto:rob@landley.net" target="_blank" rel="noreferrer">rob@landley.net</a>> wrote:<br>
> ><br>
> > On 2/6/19 12:56 PM, Dmitry Shmidt wrote:<br>
> > > Hi Rob,<br>
> > ><br>
> > > I am using your top tool implementation for Android from<br>
> > > toybox project.<br>
> > > I am wondering if in the mode where it shows cpu usage<br>
> > > per thread, the total usage per task (process) is included in initial<br>
> > > ("main") thread?<br>
> ><br>
> > I don't really use threads much, Elliott provided most of the use cases for that.<br>
> ><br>
> > > For example: <a href="http://com.google.android.youtube.tv" rel="noreferrer noreferrer" target="_blank">com.google.android.youtube.tv</a><br>
> > > <<a href="http://com.google.android.youtube.tv" rel="noreferrer noreferrer" target="_blank">http://com.google.android.youtube.tv</a>> shows 153% usage<br>
> > > for main thread and some more for other threads, like MainWebView - 31%.<br>
> > > Is this 31% included in 153% report or not?<br>
> ><br>
> > It should never report more than 100%, so it sounds like it is combining CPU<br>
> > usage from threads into the parent, yes.<br>
> ><br>
> > Hmmm... Top -H isn't showing TID by default,<br>
><br>
> yeah, that seemed a bit weird to me, but it matches what the<br>
> traditional implementation did. (though threads aren't as common on<br>
> the desktop.)<br>
><br>
> interestingly, i notice that our numbers don't add up. on the desktop,<br>
> total == running + sleeping, but our sleeping count is a lot lower<br>
> than it should be. (the desktop also says "Tasks:" or "Threads:"<br>
> depending on whether you supplied -H, and we don't.)<br>
><br>
> we also don't do a good job of sizing the PID field on machines with a<br>
> large pid_max. this fixes both of those minor issues, but between the<br>
> removal of the `const` on the array and the floating point math i<br>
> assume you'll want to do this differently :-)<br>
<br>
any thoughts on how you'd like to fix this so i can send a patch you'd<br>
accept? (the bug bankruptcy bot is asking whether i'm actually going<br>
to do anything about this bug, which reminded me...)<br></blockquote><div><br></div><div>I am ok with current solution. After we merged:</div>commit 168bfe5382c5a5034b7e208b3253f292b24999ec<br>Author: Rob Landley <<a href="mailto:rob@landley.net" target="_blank" rel="noreferrer">rob@landley.net</a>><br>Date:   Sat Mar 2 22:05:00 2019 -0600<br><br>    Make top -H show TID instead of PID, not collate %CPU into parent thread<br>    (resulting in 400% CPU with 4 threads), and add a couple comments.<br><div><br></div><div>It works as we think it should. </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> diff --git a/toys/posix/ps.c b/toys/posix/ps.c<br>
> index 079bdbd6..50f52b41 100644<br>
> --- a/toys/posix/ps.c<br>
> +++ b/toys/posix/ps.c<br>
> @@ -314,9 +314,9 @@ struct procpid {<br>
>  struct typography {<br>
>    char *name, *help;<br>
>    signed char width, slot;<br>
> -} static const typos[] = TAGGED_ARRAY(PS,<br>
> +} static /*const*/ typos[] = TAGGED_ARRAY(PS,<br>
>    // Numbers. (What's in slot[] is what's displayed, sorted numerically.)<br>
> -  {"PID", "Process ID", 5, SLOT_pid},<br>
> +  {"PID", "Process ID", 2, SLOT_pid},<br>
>    {"PPID", "Parent Process ID", 5, SLOT_ppid},<br>
>    {"PRI", "Priority (dynamic 0 to 139)", 3, SLOT_priority},<br>
>    {"NI", "Niceness (static 19 to -20)", 3, SLOT_nice},<br>
> @@ -1262,6 +1262,16 @@ static void default_ko(char *s, void *fields, char *err,<br>
> struct arg_list *arg)<br>
>    if (x) help_help();<br>
>  }<br>
><br>
> +static void init_pid_width(void)<br>
> +{<br>
> +  FILE *fp = xfopen("/proc/sys/kernel/pid_max", "re");<br>
> +  int pid_max;<br>
> +<br>
> +  fscanf(fp, "%d", &pid_max);<br>
> +  fclose(fp);<br>
> +  typos[0].width = ceil(log10(pid_max));<br>
> +}<br>
> +<br>
>  void ps_main(void)<br>
>  {<br>
>    char **arg;<br>
> @@ -1270,6 +1280,7 @@ void ps_main(void)<br>
>    int i;<br>
><br>
>    TT.ticks = sysconf(_SC_CLK_TCK); // units for starttime/uptime<br>
> +  init_pid_width();<br>
><br>
>    if (-1 != (i = tty_fd())) {<br>
>      struct stat st;<br>
> @@ -1546,8 +1557,9 @@ static void top_common(<br>
>            for (i = 0; i<mix.count; i++)<br>
>              run[1+stridx("RSTZ", *string_field(mix.tb[i], &field))]++;<br>
>            sprintf(toybuf,<br>
> -            "Tasks: %d total,%4ld running,%4ld sleeping,%4ld stopped,"<br>
> -            "%4ld zombie", mix.count, run[1], run[2], run[3], run[4]);<br>
> +            "%ss: %d total, %3ld running, %3ld sleeping, %3ld stopped, "<br>
> +            "%3ld zombie", FLAG(H)?"Thread":"Task", mix.count,<br>
> +            run[1], run[2], run[3], run[4]);<br>
>            lines = header_line(lines, 0);<br>
><br>
>            if (readfile("/proc/meminfo", toybuf, sizeof(toybuf))) {<br>
> @@ -1697,6 +1709,7 @@ static void top_setup(char *defo, char *defk)<br>
>  {<br>
>    TT.ticks = sysconf(_SC_CLK_TCK); // units for starttime/uptime<br>
>    TT.tty = tty_fd() != -1;<br>
> +  init_pid_width();<br>
><br>
>    // Are we doing "batch" output or interactive?<br>
>    if (FLAG(b)) TT.width = TT.height = 99999;<br>
><br>
><br>
> > and top -H -O TID is never showing<br>
> > more than one instance of the same PID... until I sort by TID, and then I get a<br>
> > bunch of chrome threads under the same PID, each with 1.5% of the CPU. So yeah,<br>
> > CPU usage is per process here, not per thread.<br>
> ><br>
> > I'm trying to cut a release, but let me add that to the todo list for next<br>
> > release. (I should try to come up with a better test case because y system's way<br>
> > too loaded normally...)<br>
> ><br>
> > Rob<br>
> > _______________________________________________<br>
> > Toybox mailing list<br>
> > <a href="mailto:Toybox@lists.landley.net" target="_blank" rel="noreferrer">Toybox@lists.landley.net</a><br>
> > <a href="http://lists.landley.net/listinfo.cgi/toybox-landley.net" rel="noreferrer noreferrer" target="_blank">http://lists.landley.net/listinfo.cgi/toybox-landley.net</a><br>
</blockquote></div></div>
</blockquote></div>