Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 17 May 2002 17:40:30 -0600
From:      Ian <freebsd@damnhippie.dyndns.org>
To:        Terry Lambert <tlambert2@mindspring.com>
Cc:        freebsd-current <freebsd-current@freebsd.org>
Subject:   Re: df
Message-ID:  <B90AF10E.D17D%freebsd@damnhippie.dyndns.org>
In-Reply-To: <3CE56361.67EB4549@mindspring.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On 05/17/02 14:09, Terry Lambert wrote:

> Ian wrote:
>> On 05/17/02 10:27, Riccardo Torrini wrote:
>>> I manually mount nfs fs after a reboot (current -> stable) and
>>> if I do a "df" I got two different output.  First time it get
>>> garbled, all other times it is aligned.  I'm alone?
>> 
>> I looked at the df code because I was intensely curious how this could
>> happen.  It obtains the list of mounts using a NOWAIT flag, and NFS mount
>> info may not be available immediately so it doesn't get included in the
>> field width calculations.  Then later it re-gets the list of mounts using a
>> WAIT flag (unless you used -n on the command line) so this time it has the
>> info from NFS filesystems, but it doesn't recalculate the field widths.  The
>> following patch should fix it without breaking other behaviors, I belive.
> 
> I guess the real question is why the NOWAIT flag results in the
> NFS FS's not being included, since the purpose of NOWAIT is to
> keep it from hanging indefinitely, not to keep high latency FS's
> out of consideration.

I leave troublesome questions like that to people who like mucking around in
kernel and filesystem internals.  I was just looking for something that
could cause apparently-stateful behavior across runs of the program and
decided I'd found a candidate cause.

> Also, one would expect that the FS's that failed "NOWAIT" would
> have their details left out, and be marked as "could cause hang
> if examined".
> 
>> -        if (vfslist != NULL) {
>> +        if (!nflag) {
> 
> 
> Uh, think this wants to be:
> 
>> +        if (!nflag || vfslist != NULL) {
> 
> The reason is that an explicit list of FS's on the command line
> still means "look these up, come hell or high water, and hang, if
> you have to".

vfslist != NULL means the stats array could have had some entries filtered
out of it.  Only if nflag is false could the array have grown to have new
entries.  Still, if the array got filtered, then potentially some fields
could be displayed narrower than the first-cut calcs set up, so your
suggestion is more-correct.  (Naming filesystems on the command line leads
to a completely different path thru the code where this change doesn't come
into play.)

Actually, I now think a more-correct fix would be to have no if statement at
all, and just always recalculate the field widths after calling the routine
to re-get the stats.  The if statement strikes me as a troublesome
micro-optimization that smears info about the internal state of the re-get
routine's behavior to another outside routine.  All to save a couple
microseconds.  (Consider that this whole problem happened because already
the code was assuming the re-get routine wouldn't have changed the stats
array if vfslist was NULL, which incorrectly reflects the behavior of the
re-get routine.)

If it's really important to save these cycles, the recalc of the field
widths should move closer to the point at which the raw data is
(re-)obtained.  But then you start asking yourself questions like "why does
it bother to get the list with a NOWAIT then later get the list again with a
WAIT instead of just getting it once using the method 'nflag' implies?" and
before you know it you're rewriting half the code.  :-)

-- Ian


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B90AF10E.D17D%freebsd>