[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freebsd-hackers
Subject:    Re: top -d1 behavior
From:       Fernando_Apesteguía <fernando.apesteguia () gmail ! com>
Date:       2014-12-19 17:22:42
Message-ID: CAGwOe2bMka=sRz7Ug+0SsGxHaXKGUsUkb+g9MY9w6byteUyk+g () mail ! gmail ! com
[Download RAW message or body]

On Thu, Dec 18, 2014 at 8:49 PM, John Baldwin <jhb@freebsd.org> wrote:
> On Wednesday, December 17, 2014 5:27:40 pm Fernando Apesteguía wrote:
>> On Tue, Dec 16, 2014 at 6:03 PM, John Baldwin <jhb@freebsd.org> wrote:
>> > On Sunday, November 23, 2014 4:57:33 pm Fernando Apesteguía wrote:
>> >> > Neither seem like what the user would expect.
>> >>
>> >> Agreed. But this is mostly unexpected (and can lead scripts to fail):
>> >
>> > Actually, I think having it output the states since boot would be more
>> > consistent with other tools like iostat/vmstat/etc.  They report states
>> > on the first iteration that are states since boot.  For example:
>> >
>> > % iostat 1
>> >        tty            ada0             ada1              cd0             cpu
>> >  tin  tout  KB/t tps  MB/s   KB/t tps  MB/s   KB/t tps  MB/s  us ni sy in id
>> >    8   225 20.41  12  0.24  20.56  12  0.24   2.79   0  0.00   3  0  2  0 95
>> >    0  6230 60.00   6  0.35  64.80  10  0.62   0.00   0  0.00   9  0 91  0  0
>> >    0  6195 64.00   5  0.31  51.43   7  0.35   0.00   0  0.00  11  0 89  0  1
>> >
>> > Can you test this test patch to see if it gives you what you expect from
>> > top -d1?
>>
>> Yes it does.
>>
>> I thought however that the discussion was over :) so I filed a PR[1]
>> to at least warn in the man page about the special behavior for this
>> case.
>>
>> I would rather fix top and leave the man page as it is.
>
> I think fixing top makes the most sense.  I saw this thread a while ago but
> just hadn't replied until I sent the previous message.  Here's a real version
> of the patch if you would like to test it.

I don't know what I was doing wrong, but I couldn't apply your patch
cleanly, sorry (patch -p0 < /path/to/patch). Anyway, I translated the
changes to the code and it worked for me. Sample output:

last pid:  8084;  load averages:  0.36,  0.26,  0.23
                                                                    up
1+00:47:24  16:38:10
47 processes:  1 running, 46 sleeping
CPU: 32.0% user,  0.0% nice,  8.1% system,  0.1% interrupt, 59.8% idle
Mem: 361M Active, 1024M Inact, 630M Wired, 528K Cache, 361M Buf, 1931M Free
Swap: 128M Total, 40M Used, 88M Free, 31% Inuse

...
[snip]

If you are to commit this, could you please close PR 195717?


>
> Index: contrib/top/display.c
> ===================================================================
> --- contrib/top/display.c       (revision 275828)
> +++ contrib/top/display.c       (working copy)
> @@ -557,46 +557,6 @@
>  }
>  }
>
> -z_cpustates()
> -
> -{
> -    register int i = 0;
> -    register char **names;
> -    register char *thisname;
> -    register int *lp;
> -    int cpu, value;
> -
> -for (cpu = 0; cpu < num_cpus; cpu++) {
> -    names = cpustate_names;
> -
> -    /* show tag and bump lastline */
> -    if (num_cpus == 1)
> -       printf("\nCPU: ");
> -    else {
> -       value = printf("\nCPU %d: ", cpu);
> -       while (value++ <= cpustates_column)
> -               printf(" ");
> -    }
> -    lastline++;
> -
> -    while ((thisname = *names++) != NULL)
> -    {
> -       if (*thisname != '\0')
> -       {
> -           printf("%s    %% %s", (i++ % num_cpustates) == 0 ? "" : ", ", thisname);
> -       }
> -    }
> -}
> -
> -    /* fill the "last" array with all -1s, to insure correct updating */
> -    lp = lcpustates;
> -    i = num_cpustates * num_cpus;
> -    while (--i >= 0)
> -    {
> -       *lp++ = -1;
> -    }
> -}
> -
>  /*
>   *  *_memory(stats) - print "Memory: " followed by the memory summary string
>   *
> Index: contrib/top/top.c
> ===================================================================
> --- contrib/top/top.c   (revision 275828)
> +++ contrib/top/top.c   (working copy)
> @@ -176,7 +176,6 @@
>      int  preset_argc = 0;
>      char **av;
>      int  ac;
> -    char dostates = No;
>      char do_unames = Yes;
>      char interactive = Maybe;
>      char warnings = 0;
> @@ -643,23 +642,7 @@
>                         system_info.procstates);
>
>         /* display the cpu state percentage breakdown */
> -       if (dostates)   /* but not the first time */
> -       {
> -           (*d_cpustates)(system_info.cpustates);
> -       }
> -       else
> -       {
> -           /* we'll do it next time */
> -           if (smart_terminal)
> -           {
> -               z_cpustates();
> -           }
> -           else
> -           {
> -               putchar('\n');
> -           }
> -           dostates = Yes;
> -       }
> +       (*d_cpustates)(system_info.cpustates);
>
>         /* display memory stats */
>         (*d_memory)(system_info.memory);
>
> --
> John Baldwin
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"
[prev in list] [next in list] [prev in thread] [next in thread]