svn commit: r266423 - in head/sys: conf dev/i40e modules/i40e

[Bruce Evans]

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