Date: Tue, 20 May 2014 23:40:01 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Rui Paulo <rpaulo@felyko.com> Cc: Jack F Vogel <jfv@freebsd.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Julian Elischer <julian@freebsd.org> Subject: Re: svn commit: r266423 - in head/sys: conf dev/i40e modules/i40e Message-ID: <20140520223516.R2836@besplex.bde.org> In-Reply-To: <AF83F052-00D1-40E1-A427-58EDE0853D42@felyko.com> References: <201405190121.s4J1L3qA068339@svn.freebsd.org> <53796149.8060000@freebsd.org> <AF83F052-00D1-40E1-A427-58EDE0853D42@felyko.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 19 May 2014, Rui Paulo wrote: > On 18 May 2014, at 18:41, Julian Elischer <julian@freebsd.org> wrote: > >> On 5/19/14, 9:21 AM, Jack F Vogel wrote: >>> Author: jfv >>> Date: Mon May 19 01:21:02 2014 >>> New Revision: 266423 >>> URL: http://svnweb.freebsd.org/changeset/base/266423 >>> >>> Log: >>> This is the beta release of the driver for the new >>> Intel 40G Ethernet Controller XL710 Family. This is >>> the core driver, a VF driver called i40evf, will be >>> following soon. Questions or comments to myself or >>> my co-developer Eric Joyner. Cheers! >> love the name.. > > Aesthetics aside, I think the name should be changed. Network drivers always used [a-z] for name and [0-9] for unit. Can you find an example where this is not true? Also, verbose names break formatting. E.g., netstat -r has 5 columns available under Netif for the driver name and device number. netstat -i has about the same under Name (possibly 1 or 2 not directly under Name, but reserved for the Name column). systat has 3 columns available, but with a more flexible format that truncates other info. All driver name+numbers are broken now on freefall: 2 users Load 0.07 0.04 0.01 May 20 11:46 Mem:KB REAL VIRTUAL VN PAGER SWAP PAGER Tot Share Tot Share Free in out in out Act 27100 5768 566228 9656 2578420 count All 11821k 6964 1075549k 22040 pages Proc: Interrupts r p d s w Csw Trp Sys Int Sof Flt cow 130 total 25 252 3 35 9 65 zfod uart0 4 ozfod 44 cpu0:timer 0.2%Sys 0.0%Intr 0.0%User 0.0%Nice 99.8%Idle %ozfod 2 igb0:que 0 | | | | | | | | | | | daefr 1 igb0:que 1 prcfr 1 igb0:que 2 dtbuf totfr 1 igb0:que 3 Namei Name-cache Dir-cache 484467 desvn react 2 igb0:que 4 Calls hits % hits % 309999 numvn pdwak 1 igb0:que 5 3 3 100 120414 frevn pdpgs 1 igb0:que 6 intrn 1 igb0:que 7 Disks ada0 ada1 ada2 ada3 pass0 pass1 pass2 20279528 wire igb0:link KB/t 0.00 0.00 0.00 0.00 0.00 0.00 0.00 179464 act ahci0 274 tps 0 0 0 0 0 0 0 1323416 inact 48 cpu1:timer MB/s 0.00 0.00 0.00 0.00 0.00 0.00 0.00 532604 cache 1 cpu9:timer %busy 0 0 0 0 0 0 0 2045816 free 2 cpu23:time 1694240 buf 1 cpu11:time 1 cpu22:time systat is hard-coded for 80 columns (it can handle more than 25 rows but only uses then to display more interrupts). igb0 is already 1 too long. "igb0: que N" is 8 too long. This destroys all the space meant for showing the interrupt number. "que N" is almost as good. The multitude of igb queues (handling a whole 1 interrupt/second each) also defeats sysstat's vertical formatting. Fortunately there are almost no other peripheral devices that ever interrupted on freefall, (just uart and ahci0), so their interrupts aren't pushed off the display. uart and ahci have even more verbose names than igb. Only most of the cpuN:timer interrupts are pushed off. cpuN:timer is an even more verbose name than uart and ahci. top is considerably more broken with 80x25 but is not completely hard-coded for 80 columns. -SH output is now almost useless on freefall: last pid: 23715; load averages: 0.02, 0.05, 0.01 up 15+06:28:30 12:05:06 436 processes: 25 running, 356 sleeping, 55 waiting CPU: 0.1% user, 0.0% nice, 0.1% system, 0.0% interrupt, 99.9% idle Mem: 176M Active, 1293M Inact, 19G Wired, 520M Cache, 1655M Buf, 1996M Free ARC: 15G Total, 7008M MFU, 4317M MRU, 18K Anon, 536M Header, 3240M Other Swap: 8192M Total, 10M Used, 8182M Free PID USERNAME PRI NICE SIZE RES STATE C TIME WCPU COMMAND 11 root 155 ki31 0K 384K CPU2 2 363.7H 100.00% idle{idle: 11 root 155 ki31 0K 384K CPU3 3 363.7H 100.00% idle{idle: ... Whenever there are more CPUs than rows, top -SH can't even display all the idle threads. Without -H, it doesn't display any interrupts, and without -S it doesn't display any detail about interrupts. 47 rows is just enough for all the CPUs and a couple of igb interrupts. Non-curses top output (top -SH <number> >file) displays any number of rows in a not so useful way of course. Now I use it to show other problems: - It truncates the bad verbose description "idle{idle:" as above - there is a formatting error for "100.00%" that uses 1 more column than allocated and thus steals one from the COMMAND column. 100.00% used to be unusual for any process, but is now normal for idle threads - too much space is wasted for the USERNAME column. In some previous version, even more space was wasted. - top uses the tty's number of columns for file output. This is bug for bug compatible with ps. The default for a file should be 80 columns, but changeable using something like the COLUMNS environment variable (not the tty's attribute). - COLUMNS does work for top and ps. ROWS works for systat. - with COLUMNS=80, igb0 is truncated out of existence in top -SH 100 >file. Not igb's fault. - with COLUMNS=114, igb0 is finally visible in top output (top -SH 100 >file): 12 root -92 - 0K 880K WAIT 0 1:03 0.00% intr{irq256: igb0:que} 12 root -92 - 0K 880K WAIT 6 0:47 0.00% intr{irq262: igb0:que} 12 root -92 - 0K 880K WAIT 2 0:34 0.00% intr{irq258: igb0:que} Something truncated "que N" to just the useless "que". It is collateral with the ABI breakage for tdname. (Process names were expanded from length 16 to 19, and the old space was reserved for ABI compatibility of struct kinfo_proc. ki_comm is the new name and ki_ocomm was the old name. Interrupt names use length 19. The reserved space was abused for tdname. Later, ki_ocomm was renamed to ki_tdname. The tdname length needs to be at least as large as the command name so as to hold copies of the command name, since due to compatibility hacks related to the abuse, ki_tdname is used to return what should be in ki_comm and (?) vice versa. Here we see the results of truncation from this. The full interrupt name is "irq256:igb0 que N". This has length 18, so it would fit in ki_ocomm, but it is truncated to length 16, giving "irq256:igb0 que" when it is returned in ki_tdname. When showing threads, top and ps just increase the mess by truncating ki_comm and ki_tdname and adding braces to waste even more space. ki_comm provides no extra info for interrupts and some other threads. It just says "intr" and doesn't need 19 characters for this. "intr" is repeated as "irq" in ki_tdname. ki_tdname needs more space for details but has less.) Previous regressions changed interrupt command names from something like "igb0 que 3 irq256" on i386 to "irq256:igb0 que 3" on all arches (move irqN: from last to first and add punctuation). This unimproved truncated cases since it is better to truncate the irq number than anything else.) systat -v works around some of these bugs as follows: - it doesn't use kinfo_proc or ki_tdname. It fetches the interrupt name by a different mechanism, so it is not truncated - it edits the interrupt name to put the irq number at the end, to remove punctuation that would waste space. Then if truncation would still occur, it removes "irq" Much more complicated editing would be needed to parse ki_comm and ki_tdname and discard redundant and space-wasting bits. This is too hard. The kernel should be more careful to make the strings directly usable. Interrupt names for top and ps could be fixed by expanding ki_tdname (breaking the ABI again :-() and putting the full interrupt name in it, and putting nothing instead of "intr" in ki_comm. Or maybe do any concatenation in the kernel, depending on a syscall parameter, and never using ki_tdname. Other wasteful redundancies in ki_comm{ki_tdname}: % idle{idle: cpuN} % geom{g_down} Not too bad, but g_ is an improved spelling of geom. % intr{swi4: clock} Lots of these, with the important info truncated in several steps. Old versions of top displayed something like "swi4: clock clk:cy+" for the non-threads case and "swi4: clock clk" for the threads case. This was from when kernel threads were separate processes. Several drivers are attached to "swi4: clock" (clk, cy, sio at least). 19 characters in the interrupt name is too short, so the name is truncated. (This is a regression from previous version written by me. That used a string space so that the length was unlimited but combined length was several times smaller since most names are short.) At least it writes "+" to indicate the truncation. The "+" is then lost by blind truncation on copying to ki_tdname, so the truncation is not obvious in top and ps. systat and vmstat never supported SWIs, so the usual method of seeing the non-truncated non-edited names doesn't work (it is to use vmstat -i). systat would run out of vertical space showing SWIs. % kernel{swapper} % kernel{zio_null_issue} "kernel" for kernel threads is almost as useless as "intr". The full thread name in ki_tdname says more. % zfskern{arc_reclaim_thre} This and other zfskern names are the only examples of good ki_comm/ki_tdname decompositions. "zfs*" is not repeated in ki_tdname, so only "kern" in ki_comm becomes redundant when ki_kdname is appended. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140520223516.R2836>