diff options
author | dillon <dillon@FreeBSD.org> | 1999-12-12 06:09:57 +0000 |
---|---|---|
committer | dillon <dillon@FreeBSD.org> | 1999-12-12 06:09:57 +0000 |
commit | 1da218306142327914ba163494fe5569fc587d6d (patch) | |
tree | ee9e75232133b77df56df727656abe5a80829194 /sys/nfs/nfs_common.c | |
parent | af2fc77e5c1d88a74bf1b5f6def6f7c8209bdcad (diff) | |
download | FreeBSD-src-1da218306142327914ba163494fe5569fc587d6d.zip FreeBSD-src-1da218306142327914ba163494fe5569fc587d6d.tar.gz |
Synopsis of problem being fixed: Dan Nelson originally reported that
blocks of zeros could wind up in a file written to over NFS by a client.
The problem only occurs a few times per several gigabytes of data. This
problem turned out to be bug #3 below.
bug #1:
B_CLUSTEROK must be cleared when an NFS buffer is reverted from
stage 2 (ready for commit rpc) to stage 1 (ready for write).
Reversions can occur when a dirty NFS buffer is redirtied with new
data.
Otherwise the VFS/BIO system may end up thinking that a stage 1
NFS buffer is clusterable. Stage 1 NFS buffers are not clusterable.
bug #2:
B_CLUSTEROK was inappropriately set for a 'short' NFS buffer (short
buffers only occur near the EOF of the file). Change to only set
when the buffer is a full biosize (usually 8K). This bug has no
effect but should be fixed in -current anyway. It need not be
backported.
bug #3:
B_NEEDCOMMIT was inappropriately set in nfs_flush() (which is
typically only called by the update daemon). nfs_flush()
does a multi-pass loop but due to the lack of vnode locking it
is possible for new buffers to be added to the dirtyblkhd list
while a flush operation is going on. This may result in nfs_flush()
setting B_NEEDCOMMIT on a buffer which has *NOT* yet gone through its
stage 1 write, causing only the commit rpc to be made and thus
causing the contents of the buffer to be thrown away (never sent to
the server).
The patch also contains some cleanup, which only applies to the commit
into -current.
Reviewed by: dg, julian
Originally Reported by: Dan Nelson <dnelson@emsphone.com>
Diffstat (limited to 'sys/nfs/nfs_common.c')
-rw-r--r-- | sys/nfs/nfs_common.c | 20 |
1 files changed, 14 insertions, 6 deletions
diff --git a/sys/nfs/nfs_common.c b/sys/nfs/nfs_common.c index b62b21d..0da996b 100644 --- a/sys/nfs/nfs_common.c +++ b/sys/nfs/nfs_common.c @@ -1357,11 +1357,13 @@ nfs_loadattrcache(vpp, mdp, dposp, vaper) vap->va_size = np->n_size; else np->n_size = vap->va_size; - } else + } else { np->n_size = vap->va_size; + } vnode_pager_setsize(vp, np->n_size); - } else + } else { np->n_size = vap->va_size; + } } np->n_attrstamp = time_second; if (vaper != NULL) { @@ -1444,11 +1446,13 @@ nfs_getattrcache(vp, vaper) vap->va_size = np->n_size; else np->n_size = vap->va_size; - } else + } else { np->n_size = vap->va_size; + } vnode_pager_setsize(vp, np->n_size); - } else + } else { np->n_size = vap->va_size; + } } bcopy((caddr_t)vap, (caddr_t)vaper, sizeof(struct vattr)); if (np->n_flag & NCHG) { @@ -2150,7 +2154,11 @@ nfs_invaldir(vp) * The write verifier has changed (probably due to a server reboot), so all * B_NEEDCOMMIT blocks will have to be written again. Since they are on the * dirty block list as B_DELWRI, all this takes is clearing the B_NEEDCOMMIT - * flag. Once done the new write verifier can be set for the mount point. + * and B_CLUSTEROK flags. Once done the new write verifier can be set for the + * mount point. + * + * B_CLUSTEROK must be cleared along with B_NEEDCOMMIT because stage 1 data + * writes are not clusterable. */ void nfs_clearcommit(mp) @@ -2171,7 +2179,7 @@ loop: if (BUF_REFCNT(bp) == 0 && (bp->b_flags & (B_DELWRI | B_NEEDCOMMIT)) == (B_DELWRI | B_NEEDCOMMIT)) - bp->b_flags &= ~B_NEEDCOMMIT; + bp->b_flags &= ~(B_NEEDCOMMIT | B_CLUSTEROK); } } splx(s); |