Date: Mon, 5 Aug 2002 21:19:47 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Pierre Beyssac <beyssac@enst.fr> Cc: freebsd-current@FreeBSD.ORG Subject: Re: deadlock in fs/msdosfs/msdosfs_vnops.c:msdosfs_fsync() Message-ID: <20020805203323.M17317-100000@gamplex.bde.org> In-Reply-To: <20020805104153.A37733@bofh.enst.fr>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 5 Aug 2002, Pierre Beyssac wrote:
> I'm having a problem with the fsync() call on the MSDOS filesystem.
>
> I have a reproductible way (using a specific application under Wine)
> to get it to deadlock in the msleep() call. If I disable msdos_fsync()
> entirely by adding a return 0 at the top, everything runs smoothly,
> no deadlock.
msdosfs_fsync() doesn't honor the MNT_NOWAIT flag. This might explain
your problem. msdosfs (and other filesystems, and even ffs in RELENG_4?)
is missing fixes for endless looping on write errors. This is
unlikely to be the problem here.
> Since AFAIK the problem doesn't occur with ufs fsync(), should I
> suppose this may have to do (and should I start looking) with the
> way msdosfs handles vp->v_numoutput, can it be a deadlock with some
> other vnode locking in the msdosfs code, or can it be an unexpected
> side-effet of recent SMP/KSE changes (not likely considering my
> machine is UP) ?
The following patch makes msdosfs_fsync() as identical as possible with
an old version of ffs_fsync(). Summary of the changes:
- in the main loop, optimize bwrite() to vfs_bio_awrite() in the MNT_NOWAIT
case. A more up to date merge from ffs would do this in the !MNT_WAIT
case (ffs once had more complications involving the MNT_LAZY case
and some of these complications made it into this patch and into
some other filesystems). A less agressive optimization would use
bawrite() for the !MNT_WAIT case and bwrite() for the MNT_WAIT case.
- in the main loop, exit early if bwrite() fails. This is from rev.1.57
of ffs_vnops.c. IIRC, in ffs this fixes endless looping on write
errors in some but not all cases. However, it seems to be moot here
because the other change in the main loop makes the bwrite() unreachable
(the Debugger() call is to detect it being reached).
- don't wait for v_numoutput in the !MNT_WAIT case.
- don't use a too-long or poorly abbreviated string for the sleep message.
%%%
Index: msdosfs_vnops.c
===================================================================
RCS file: /home/ncvs/src/sys/fs/msdosfs/msdosfs_vnops.c,v
retrieving revision 1.121
diff -u -2 -r1.121 msdosfs_vnops.c
--- msdosfs_vnops.c 16 May 2002 21:25:37 -0000 1.121
+++ msdosfs_vnops.c 21 May 2002 21:07:15 -0000
@@ -808,12 +821,12 @@
{
struct vnode *vp = ap->a_vp;
- int s;
struct buf *bp, *nbp;
+ int error, s;
/*
* Flush all dirty buffers associated with a vnode.
*/
-loop:
s = splbio();
+loop:
for (bp = TAILQ_FIRST(&vp->v_dirtyblkhd); bp; bp = nbp) {
nbp = TAILQ_NEXT(bp, b_vnbufs);
@@ -822,19 +835,35 @@
if ((bp->b_flags & B_DELWRI) == 0)
panic("msdosfs_fsync: not dirty");
- bremfree(bp);
- splx(s);
- (void) bwrite(bp);
+ if (bp->b_vp == vp || ap->a_waitfor == MNT_NOWAIT) {
+ BUF_UNLOCK(bp);
+ vfs_bio_awrite(bp);
+ } else {
+ /*
+ * Wait for I/O associated with metadata blocks to
+ * complete, since there is no way to quickly wait
+ * for them below.
+ */
+ Debugger("msdosfs_fsync: metadata");
+ bremfree(bp);
+ splx(s);
+ error = bwrite(bp);
+ if (error != 0)
+ return (error);
+ s = splbio();
+ }
goto loop;
}
- while (vp->v_numoutput) {
- vp->v_flag |= VBWAIT;
- (void) tsleep((caddr_t)&vp->v_numoutput, PRIBIO + 1, "msdosfsn", 0);
- }
+ if (ap->a_waitfor == MNT_WAIT) {
+ while (vp->v_numoutput) {
+ vp->v_flag |= VBWAIT;
+ tsleep(&vp->v_numoutput, PRIBIO + 1, "msfsyn", 0);
+ }
#ifdef DIAGNOSTIC
- if (!TAILQ_EMPTY(&vp->v_dirtyblkhd)) {
- vprint("msdosfs_fsync: dirty", vp);
- goto loop;
- }
+ if (!TAILQ_EMPTY(&vp->v_dirtyblkhd)) {
+ vprint("msdosfs_fsync: dirty", vp);
+ goto loop;
+ }
#endif
+ }
splx(s);
return (deupdat(VTODE(vp), ap->a_waitfor == MNT_WAIT));
%%%
Bruce
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?20020805203323.M17317-100000>