From owner-svn-src-all@FreeBSD.ORG Wed May 21 09:28:36 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id C3D3F801; Wed, 21 May 2014 09:28:36 +0000 (UTC) Received: from mail107.syd.optusnet.com.au (mail107.syd.optusnet.com.au [211.29.132.53]) by mx1.freebsd.org (Postfix) with ESMTP id 6FBAE23A0; Wed, 21 May 2014 09:28:35 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail107.syd.optusnet.com.au (Postfix) with ESMTPS id 275F8D46A6A; Wed, 21 May 2014 19:03:45 +1000 (EST) Date: Wed, 21 May 2014 19:03:44 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Jack Vogel Subject: Re: svn commit: r266423 - in head/sys: conf dev/i40e modules/i40e In-Reply-To: Message-ID: <20140521164516.I974@besplex.bde.org> References: <201405190121.s4J1L3qA068339@svn.freebsd.org> <53796149.8060000@freebsd.org> <537B714A.5080500@freebsd.org> <537B726C.1080000@freebsd.org> <20140521014355.V3433@besplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=eojmkOZX c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=JD0ISiim65UA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=bcTnmpMzrbkznxxsaCkA:9 a=a3Fqpw3XbYTMFsea:21 a=n-AMvmRKszQcaNyp:21 a=CjuIK1q_8ugA:10 Cc: "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" , Rui Paulo , Bruce Evans , Jack F Vogel , Julian Elischer X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 May 2014 09:28:36 -0000 On Tue, 20 May 2014, Jack Vogel wrote: > If you don't like the name there's this wonderful feature of ifconfig > > ifconfig i40e0 name eth0 (or whatever pleases you...) Ah, another bug :-). If you use this to make a non-null change to the name, then the name becomes inconsistent with the interrupt name, since the latter is set at driver initialization time and is not updated if the if name is changed. > Oh and Bruce, I did run into the string length issue, so with this driver > the queues > are all named 'q%d', I might go back and change the earlier drivers. Also, there is no way for the user to changes this. setproctitle() only works on non-kernel processes Also, setproctitle() is deficient for renaming even the process name. It renames something from p_comm/ki_comm and doesn't go near ki_tdname. ps and top -a merge ki_comm with the proctitle in a bad way to end up with a bad combination "foo: bar (foo)". More lossage from combining ki_comm with ki_tdname: on freefall now, there are 206 non-kernel threads with more than 1 per process. After removing duplicates, there are just 4 combined names in COLUMNS=1000 top -H output: auditdistd{auditdistd} irssi{irssi} nfsd{nfsd: master} nfsd{nfsd: service} In all cases, the ki_comm name is uselessly repeated in ki_tdname, and in almost all cases ki_td_name is useless for distinguishing the threads. It distinguish the 1 nfsd "master" from the 191 nfsd "services". The "master" is in the same process as the "services". There is also a separate nfsd process with no threads. The names are confusing. According to logic and nfsd(8), all these threads are servers, not services. nfsd(8) says to kill the "master" to kill all the children, but I think the "master" is the independent process and not the thread named "nfsd: master", since killing applies to processes so it makes no sense to have a special thread for killing the others. Indeed, the documentation matches old versions of FreeBSD when the nfsd's were separate processes: FreeBSD-5.2 (now ps laxwH output): % 0 556 1 392 4 0 704 456 accept Is ?? 0:00.01 nfsd: master (nfsd) % 0 558 556 319 4 0 576 324 - I ?? 0:00.00 nfsd: server (nfsd) % 0 559 556 319 4 0 576 324 - I ?? 0:00.00 nfsd: server (nfsd) % ... This also shows the bad combination of ki_comm proctitle, and the name "server" not regressed to "service". -current (ps laxwH): % 0 707 1 0 52 0 20408 1064 select Is - 0:00.03 nfsd: master (nfsd) % 0 709 707 0 52 0 12216 4104 rpcsvc S - 0:01.59 nfsd: server (nfsd) % 0 709 707 0 52 0 12216 4104 rpcsvc I - 0:00.00 nfsd: server (nfsd) Oops, everything is correct for ps output (the only change is to use threads. proctitle(3) is used correctly to add master/server to the name, except it is missing numbering of the servers in both. Numbering is much more needed with the disfustingly bloated number of servers on freefall. It is top that has the bad names. These are only in ki_tdname. ps is broken in another way -- it doesn't show the broken ki_tdname since it doesn't even display ki_tdname for nfsd. (It does display ki_tdname for kernel threads. The bug there is primarily that it uses a different format to top. It also has a keyword "tdnam" for displaying ki_tdname. Bugs in this start with its name not being spelled with an "e" and being undocumented. It is unclear if it is used in standard formats. It is used in some cases in -wH. The editing problems for combining names are especially large for using this keyword. Adding -o any to a standard format makes a mess generally, and adding -o tdnam may or may not duplicate details in the standard format. In practice, ps laxwH -o tdnam gives: ... % 0 0 0 0 -92 0 0 5856 - DLs - 0:00.08 [kernel/igb0 que igb0 que % 0 12 0 0 -92 0 0 1120 - WL - 9:37.90 [intr/irq256: ig irq256: igb0:que % 0 707 1 0 52 0 20408 1064 select Is - 0:00.03 nfsd: master (nf % 0 709 707 0 52 0 12216 4104 rpcsvc S - 0:01.59 nfsd: server (nf nfsd: master % 0 709 707 0 52 0 12216 4104 rpcsvc I - 0:00.00 nfsd: server (nf nfsd: service Various bugs are now more evident: - laxwH contains ki_tdname precisely for kernel threads. So adding -o tdnam gives duplication for igb0 bug not for nfsd* - truncation causes various messes: - the igb0 proctitle and tdnam (sic) get truncated after "que". Even the "]" delimiter is truncated away for igb0 - the nfsd proctitle gets truncated after "nf". The nfsd tdnam doesn't get truncated. FreeBSD-5.2 (top -Ha output): % 556 root 4 0 704K 456K accept 0:00 0.00% nfsd: master (nfsd) % 558 root 4 0 576K 324K - 0:00 0.00% nfsd: server (nfsd) % 561 root 4 0 576K 324K - 0:00 0.00% nfsd: server (nfsd) % ... Nothing changed since there are no threads. I now see that "nfsd: " is part of the proctitle, so the verboseness is not all ps's fault. nfsd didn't ask setproctitle() to not keep the original name, so the extension "master" or "server" was added in an undocumented format (after a colon and a space). Then ps adds the original name in parentheses. It shouldn't add this when the original name is still in the proctitle. Combining these names has similar problem to combining ki_comm with ki_tdnam. With standard parts of the format undocumented, it is difficult to edit away unintentional redundancies. FreeBSD-5.2 (top -Ha output): % 707 root 52 0 20408K 1064K select 6 0:00 0.00% nfsd: master (nfsd) % 709 root 52 0 12216K 4104K rpcsvc 10 0:02 0.00% nfsd: server (nfsd){nfsd: master} % 709 root 52 0 12216K 4104K rpcsvc 13 0:00 0.00% nfsd: server (nfsd){nfsd: service} % ... This is now similar to ps laxwH -o tdnam. The truncation is not quite as bad. For igb0, "intr" and irq261:" are not lost (ps is apparently using a completely different naming scheme for kernel threads. I added tdnam to get the thread name for nfsd*, but it didn't give the thread name for igb0. The thread name for igb0 is "irq261:...". This isn't in the plain laxwH output either. Adding tdnam doubled something other than the thread name for igb0). For nfsd, there is no truncation after "nf". Even more excessive delimiters "[]" are used for kernel threads, but at least they don't get truncated (in this best case with COLUMNS=1000; curses output is of course truncated at the window margin). The following naming regressions are now evident for nfsd: - the proctitle part of the name is unchanged and correct - the ki_tdnam part of the name (delimited by {}) is inconsistent with the proctitle part, and wrong: - it misnames first server thread as "master" - it misnames all the other server threads as "service". ki_tdname is supposed to be the thread name, not fine details of what is in the proctitle. Bruce