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