Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 5 Jun 2012 01:05:53 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Gleb Smirnoff <glebius@freebsd.org>
Cc:        svn-src-projects@freebsd.org, src-committers@freebsd.org, Bruce Evans <brde@optusnet.com.au>
Subject:   Re: svn commit: r236554 - projects/pf/head/sys/contrib/pf/net
Message-ID:  <20120605000211.R1992@besplex.bde.org>
In-Reply-To: <20120604133358.GO44607@FreeBSD.org>
References:  <201206041009.q54A9v4A019437@svn.freebsd.org> <20120604210548.Y1459@besplex.bde.org> <20120604133358.GO44607@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 4 Jun 2012, Gleb Smirnoff wrote:

> On Mon, Jun 04, 2012 at 09:29:46PM +1000, Bruce Evans wrote:
> B> > Log:
> B> >  Consistent names for pf(4) malloc(9) types.
> B>
> B> Many still have spaces in them.  This breaks at least simple parsing
> B> of "vmstat -m" output using columns in awk (and postprocessing of
> B> vmstat -m output is more needed than orginally, since vmstat is now
> B> too stupid to even print the totals).  A few bugs in this area have
> B> been fixed relatively recently.  For example:
> B>
> B>    file desc -> filedesc   (matches original naming scheme -- no underscore)
> B>    BIO buffer -> biobuf
> B>    UFS mount -> ufs_mount  (now just a style bug -- underscore)
> B>    VM pgdata -> vm_pgdata
> B>    * ihash -> [went away]
> B>    struct cdev * -> [went away]
> B>    cluster_save buffer -> [went away]  (was also too long)
> B>    [too many] -> [vmstat -z, with worse names and formatting]
>
> Should only the shortdesc not contain spaces? Well, there is conflict between
> parseability and understandability :)

Of course the longdesc should contain spaces.  Especially since it is not
used, so it only bloats the source code :-).

The shortdesc could even use M_FOO (if the longdesc could be looked
up more easily).  But in 4.4BSD, there was a separate (hard-coded)
string table with names, with lots of spaces in it too, but apparently
only for newer (in 1993) names, since entries 1-21 had no spaces, no
underscores, many abbreviations and no upper case, while entries 22-61
are quite different.

> B> Now broken on ref10-i386:
> B>
> B>    NFSCL diroffd

Actually "NFSCL diroffdiroff".  This is a strange name, apart from being
too long (see below).  NFS names are poor.  Another one bad one is
"newnfsclient_req".  This one is apparently the current extension of the
first one with a space in it in 4.4BSD (#22 was "NFS req").  It was still
"NFS req" in FreeBSD-5.  But the space in it is fixed in the old nfs
client -- there it is now "nfsclient_req":

% RCS file: /home/ncvs/src/sys/nfsclient/nfs_vfsops.c,v
% Working file: nfs_vfsops.c
% head: 1.259
% ----------------------------
% revision 1.178
% date: 2005/10/31 15:41:27;  author: rwatson;  state: Exp;  lines: +5 -5
% Normalize a significant number of kernel malloc type names:
% 
% - Prefer '_' to ' ', as it results in more easily parsed results in
%   memory monitoring tools such as vmstat.
% 
% - Remove punctuation that is incompatible with using memory type names
%   as file names, such as '/' characters.

So your "(4)" shouldn't be used.  It works in file names but has shell
metacharacters.

% 
% - Disambiguate some collisions by adding subsystem prefixes to some
%   memory types.
% 
% - Generally prefer lower case to upper case.
% 
% - If the same type is defined in multiple architecture directories,
%   attempt to use the same name in additional cases.
% 
% Not all instances were caught in this change, so more work is required to
% finish this conversion.  Similar changes are required for UMA zone names.
% ----------------------------

I don't like verbosifying "NFS" to "nfsclient" or the underscore.  In
general there is not enough space for such verbose names.  In vmstat -m
output, there are 13 columns for the name.  That is a larger than
normal number for a tabular format, but "nfsclient_" uses 9 for just the
subsystem name, leaving only 4 for the details.  The details remain
abbreviated to "req".

> B>    NFS fh

This is older-style, and abbreviates everything (fh is less well known
than NFS).  It is for the new fs client and/or server (it is mostly
used by the client but it is defined in a common part).  This was
renamed to nfsclient_bigfh in the old nfsclient, and its macro name
has a BIG in it too, and its verbose description says "version 3"
instead of "big".  In FreeBSD-5, its short description was "NFSV3
bigfh" and its other details were the same as now.

"NFSCL diroffdiroff" seems to be just an editing error in the new nfs
client.  In the old nfs client, this only ever had 1 diroff, and it
was renamed from "NFSV3 diroff" in 4.4BSD..FreeBSD-5 to
"nfsclient_diroff" in the old nfs client now.  The verbose prefix in
it makes it too long even with only one diroff in it.

The new and old nfs clients can be compiled into the same kernel and
give many potential naming conflicts and bad names to avoid the conflicts
and worse names in the new version because the best names are taken.
"CL" is better than "client" for anti-verboseness, but you might still
need new/old and V2/V3/V4 or shorter.

> B>
> B> only.
>
> Also a lot of "CAM foo".

These didn't show up for me.  Nor did most nfs ones.  Apparently vmstat -m
doesn't show everything, and I couldn't find an option to change this.
Grepping in cam showed:
- no subsystem prefix for ctl* or ramdisk
- scsi subsystem mostly uses scsi_foo, but some places use "SCSI FOO"
- use of private malloc types exploded from 1 in 2004 to 22 now.  There
   were about 5 at the time of rwatson's sweep described above.  These were
   mostly "SCSI FOO" and were not touched by the sweep.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120605000211.R1992>