On Wed, 14 Feb 2024 14:05:15 +0000 Matthew Chae <matthew.c...@axis.com> wrote:
> Hi Bernhard, > > I'm sending new patch and the result of bloatcheck. Many thanks for the updated patch! function old new delta display_process_list 1406 1765 +359 .rodata 99721 99724 +3 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 2/0 up/down: 362/0) Total: 362 bytes text data bss dec hex filename 1009548 16507 1840 1027895 faf37 busybox_old 1009910 16507 1840 1028257 fb0a1 busybox_unstripped I think that's too much. For me this gives +293 bytes, still way too much. Can you please see if it helps to retain the manual formatting of PID PPID USER? PS: procps/top.c: In function ‘display_process_list’: procps/top.c:664:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] 664 | typedef struct { unsigned quot, rem; } bb_div_t; | ^~~~~~~ so please move your new #define PID_STR block down to right before /* what info of the processes is shown */ In + int lines = (lines_rem - 1); please drop the superfluous braces. It is most likely not smaller to use pid_len = strlen(make_human_readable_str(pid_max,0,0)) than to introduce this private count_digits(), i fear? Maybe we could amortize the addition of count_digits by reusing it elsewhere, as a follow-up. thanks > Can you review these and give me your thoughts? > Just let me know if you think that the code size needs to be reduced. > > Br-Matthew > ________________________________ > From: Bernhard Reutner-Fischer <rep.dot....@gmail.com> > Sent: Tuesday, February 13, 2024 7:16 PM > To: Matthew Chae <matthew.c...@axis.com> > Cc: rep.dot....@gmail.com <rep.dot....@gmail.com>; David Laight > <david.lai...@aculab.com>; 'Denys Vlasenko' <vda.li...@googlemail.com>; > busybox@busybox.net <busybox@busybox.net>; Christopher Wong > <christopher.w...@axis.com> > Subject: Re: fix large PID overflow their column in top command > > On Mon, 5 Feb 2024 09:56:20 +0000 > Matthew Chae <matthew.c...@axis.com> wrote: > > > Hi David, > > > > I'm sending an improved patch based on your comments. > > > > Not only does it not care about the PID_MAX value, > > it searches all the contents to be output to recognize the required column > > width > > and dynamically allocates the space for PID and PPID appropriately without > > creating a lot of empty space. > > > > Additionally, this patch still allows user names to be displayed up to 8 > > characters without truncation. > > > > Can you look through the attachment? > > Unfortunately the patch does not apply to current master. > How much would your patch add to the size? Can you bring it down to a > minimum? > See make baseline; apply the patch;make bloatcheck > > thanks > > > (0002-Allocate-PID-PPID-space-dynamically-in-top-command.patch) > > > > BR-Matthew Chae > > ________________________________ > > From: David Laight <david.lai...@aculab.com> > > Sent: Thursday, November 23, 2023 6:10 PM > > To: 'Denys Vlasenko' <vda.li...@googlemail.com>; Matthew Chae > > <matthew.c...@axis.com> > > Cc: busybox@busybox.net <busybox@busybox.net>; Christopher Wong > > <christopher.w...@axis.com> > > Subject: RE: fix large PID overflow their column in top command > > > > ... > > > + fp = xfopen_for_read("/proc/sys/kernel/pid_max"); > > > + if (!fgets(pid_buf, PID_DIGITS_MAX + 1, fp)) { > > > ... > > > + if (strncmp(pid_buf, "32768", 5) == 0) > > > + pid_digits_num = 5; > > > + else > > > + pid_digits_num = PID_DIGITS_MAX; > > > > > > The logic above is not sound. Even if sysctl kernel.pid_max > > > is 32768, there can be already running processes with pids > 99999. > > > > It's also probably wrong for pretty much all other values. > > > > I'd just base the column width on strlen(pid_buf) with a minimum > > value of 5. > > > > It is unlikely that pid_max has been reduced - so column overflow > > it that case probably doesn't really matter. > > > > The more interesting case is really a system with a very large pid_max > > that has never run many processes. > > You don't really want lots of blank space. > > > > I can't remember whether top reads everything before doing any output? > > Since the output is sorted it probably almost always does. > > In which case it knows the column width it will need. > > > > I did post a patch a while back that enabled 'Irix mode'. > > (100% cpu is one cpu at 100%, not all cpus at 100%) > > Maybe I should dig it out again. > > > > David > > > > - > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 > > 1PT, UK > > Registration No: 1397386 (Wales) > _______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox