diff options
author | green <green@FreeBSD.org> | 2005-06-10 23:50:41 +0000 |
---|---|---|
committer | green <green@FreeBSD.org> | 2005-06-10 23:50:41 +0000 |
commit | ff904ffb643a4538cfbfba4658052e1b36e2bfe5 (patch) | |
tree | 4eeddcd51bbe2b1f2ab59415946715327f429a80 /sys/nfsclient/nfs_bio.c | |
parent | 613f0c4df2e951f81d91d8550f1ce05c8aa55f2d (diff) | |
download | FreeBSD-src-ff904ffb643a4538cfbfba4658052e1b36e2bfe5.zip FreeBSD-src-ff904ffb643a4538cfbfba4658052e1b36e2bfe5.tar.gz |
Fix a serious deadlock with the NFS client. Given a large enough
atomic write request, it can fill the buffer cache with the entirety
of that write in order to handle retries. However, it never drops
the vnode lock, or else it wouldn't be atomic, so it ends up waiting
indefinitely for more buf memory that cannot be gotten as it has it
all, and it waits in an uncancellable state.
To fix this, hibufspace is exported and scaled to a reasonable
fraction. This is used as the limit of how much of an atomic write
request by the NFS client will be handled asynchronously. If the
request is larger than this, it will be turned into a synchronous
request which won't deadlock the system. It's possible this value is
far off from what is required by some, so it shall be tunable as soon
as mount_nfs(8) learns of the new field.
The slowdown between an asynchronous and a synchronous write on NFS
appears to be on the order of 2x-4x.
General nod by: gad
MFC after: 2 weeks
More testing: wes
PR: kern/79208
Diffstat (limited to 'sys/nfsclient/nfs_bio.c')
-rw-r--r-- | sys/nfsclient/nfs_bio.c | 65 |
1 files changed, 65 insertions, 0 deletions
diff --git a/sys/nfsclient/nfs_bio.c b/sys/nfsclient/nfs_bio.c index 17c3c3d..4eb461d 100644 --- a/sys/nfsclient/nfs_bio.c +++ b/sys/nfsclient/nfs_bio.c @@ -873,6 +873,14 @@ nfs_write(struct vop_write_args *ap) */ if (ioflag & (IO_APPEND | IO_SYNC)) { if (np->n_flag & NMODIFIED) { + /* + * Require non-blocking, synchronous writes to + * dirty files to inform the program it needs + * to fsync(2) explicitly. + */ + if (ioflag & IO_NDELAY) + return (EAGAIN); +flush_and_restart: np->n_attrstamp = 0; error = nfs_vinvalbuf(vp, V_SAVE, td, 1); if (error) @@ -953,6 +961,63 @@ restart: } biosize = vp->v_mount->mnt_stat.f_iosize; + /* + * Find all of this file's B_NEEDCOMMIT buffers. If our writes + * would exceed the local maximum per-file write commit size when + * combined with those, we must decide whether to flush, + * go synchronous, or return error. We don't bother checking + * IO_UNIT -- we just make all writes atomic anyway, as there's + * no point optimizing for something that really won't ever happen. + */ + if (!(ioflag & IO_SYNC)) { + int needrestart = 0; + if (nmp->nm_wcommitsize < uio->uio_resid) { + /* + * If this request could not possibly be completed + * without exceeding the maximum outstanding write + * commit size, see if we can convert it into a + * synchronous write operation. + */ + if (ioflag & IO_NDELAY) + return (EAGAIN); + ioflag |= IO_SYNC; + if (np->n_flag & NMODIFIED) + needrestart = 1; + } else if (np->n_flag & NMODIFIED) { + int wouldcommit = 0; + BO_LOCK(&vp->v_bufobj); + if (vp->v_bufobj.bo_dirty.bv_cnt != 0) { + TAILQ_FOREACH(bp, &vp->v_bufobj.bo_dirty.bv_hd, + b_bobufs) { + if (bp->b_flags & B_NEEDCOMMIT) + wouldcommit += bp->b_bcount; + } + } + BO_UNLOCK(&vp->v_bufobj); + /* + * Since we're not operating synchronously and + * bypassing the buffer cache, we are in a commit + * and holding all of these buffers whether + * transmitted or not. If not limited, this + * will lead to the buffer cache deadlocking, + * as no one else can flush our uncommitted buffers. + */ + wouldcommit += uio->uio_resid; + /* + * If we would initially exceed the maximum + * outstanding write commit size, flush and restart. + */ + if (wouldcommit > nmp->nm_wcommitsize) + needrestart = 1; + } + if (needrestart) { + if (haverslock) { + nfs_rsunlock(np, td); + haverslock = 0; + } + goto flush_and_restart; + } + } do { nfsstats.biocache_writes++; |