From owner-freebsd-fs@FreeBSD.ORG Sat Dec 1 08:07:19 2007 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9361716A417 for ; Sat, 1 Dec 2007 08:07:19 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from relay02.kiev.sovam.com (relay02.kiev.sovam.com [62.64.120.197]) by mx1.freebsd.org (Postfix) with ESMTP id 3620113C45B for ; Sat, 1 Dec 2007 08:07:19 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from [212.82.216.226] (helo=deviant.kiev.zoral.com.ua) by relay02.kiev.sovam.com with esmtps (TLSv1:AES256-SHA:256) (Exim 4.67) (envelope-from ) id 1IyNNZ-00031y-9z; Sat, 01 Dec 2007 10:07:17 +0200 Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.1/8.14.1) with ESMTP id lB187A75031536; Sat, 1 Dec 2007 10:07:10 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.2/8.14.2/Submit) id lB1879VC031535; Sat, 1 Dec 2007 10:07:09 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 1 Dec 2007 10:07:09 +0200 From: Kostik Belousov To: Don Lewis Message-ID: <20071201080709.GO83121@deviant.kiev.zoral.com.ua> References: <20071130052840.GH83121@deviant.kiev.zoral.com.ua> <200712010715.lB17FlZw011929@gw.catspoiler.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="f4HxWLVbzokH9yio" Content-Disposition: inline In-Reply-To: <200712010715.lB17FlZw011929@gw.catspoiler.org> User-Agent: Mutt/1.4.2.3i X-Scanner-Signature: cba338bb38be4578e9980d7a4db64caa X-DrWeb-checked: yes X-SpamTest-Envelope-From: kostikbel@gmail.com X-SpamTest-Group-ID: 00000000 X-SpamTest-Info: Profiles 1836 [Nov 30 2007] X-SpamTest-Info: helo_type=3 X-SpamTest-Info: {received from trusted relay: not dialup} X-SpamTest-Method: none X-SpamTest-Method: Local Lists X-SpamTest-Rate: 0 X-SpamTest-Status: Not detected X-SpamTest-Status-Extended: not_detected X-SpamTest-Version: SMTP-Filter Version 3.0.0 [0255], KAS30/Release Cc: freebsd-fs@freebsd.org Subject: Re: File remove problem X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 01 Dec 2007 08:07:19 -0000 --f4HxWLVbzokH9yio Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 30, 2007 at 11:15:47PM -0800, Don Lewis wrote: > On 30 Nov, Kostik Belousov wrote: > > On Fri, Nov 30, 2007 at 03:58:55PM +1100, Bruce Evans wrote: > >> On Fri, 30 Nov 2007, David Cecil wrote: > >>=20 > >> >Thanks Bruce. > >> > > >> >Actually, I had found the same problem, and I came up with the first = line=20 > >> >of your patch (adding IN_MODIFIED) myself, but I still saw the proble= m. I > >>=20 > >> Yes, it's not that. Testing reminded me that there is normally a > >> VOP_INACTIVE() after unlink so the IN_CHANGE mark doesn't live very lo= ng > >> for unlink (it can only live long for open files). > >>=20 > >> Testing shows that the problem is easy to reproduce and often partially > >> detected before it becomes fatal. I saw something like the following: > >>=20 > >> after touch a; ln a b; rm a; unmount -- no problem with 1 link rem= aining > >> after touch a; rm a; unmount -- no problem with unmount > >> after touch a; ln a b; rm a; mount -u o ro -- no problem with 1 li= nk... > >> after touch a; ; rm a; mount -u o ro -- worked once without = soft > >> updates but seemed to be responsible for a soft update panic la= ter > >> after touch a; ; rm a; mount -u o ro -- usually fails with s= oft > >> updates; the error is detected in various ways: > >> under ~5.2, mount -u prints "/f: update error: blocks 0 file= s 1" > >> but succeeds > >> under -current, mount -u fails and a subroutine prints > >> "softdep_waitidle: Failed to flush worklist for 0xc3e1a29c" > >> However, mount -u apparently cannot afford to fail at this > >> poing since it has committed to succeeding -- further > >> mount -u's and unmounts fail and it takes a reboot to reach > >> an fsck that can fix the problem. > >>=20 > >> mount -u seems to do some things right: at least under -current: > >> - it calls ffs_sync() and thus ffs_update() with waitfor !=3D 0. > >> - IN_MODIFIED is usually already set in ffs_update(). > >> - softdep_update_inode_inodeblock() in ffs_update() seems to > >> make null changes. That doesn't seem right -- shouldn't it > >> update the link count and finish removing the file?... I > >> just noticed that ufs_inactive() handles some of this. > >> - it calls softdep_flushfiles() after doing the sync. This > >> doesn't seem to touch the inode. > >> - apparently, softdep_flushfiles() fails in -current, while in > >> ~5.2 it bogusly succeeds and then code just after it is called > >> detects a problem but doesn't handle it. > >>=20 > >> >didn't pick up on the need for the second line (else if (DOINGASYNC(d= vp))=20 > >> >{) though. It's a default mount, so I don't understand how that will= =20 > >> >help, i.e. it won't be an async mount, right? > >>=20 > >> Ignore that. It is for async mounts, to make them unconditionally asy= nc. > >>=20 > >> >One more point to address Julian's question, the partition is not mou= nted=20 > >> >with soft updates. > >>=20 > >> Interesting. I saw no sign of the problem without soft updates except= a > >> panic later after enabling soft updates. I was running fsck a lot but > >> may have forgotten one since no error was detected. The problem should > >> be easier to understand if it affects non-soft-updates. > >>=20 > >> [Context lost to top posting] > >>=20 > >=20 > > As a speculation, it might be that ufs_inactive() should conditionalize= on > > fs_ronly instead of MNT_RDONLY. Then, VOP_INACTIVE() would set up the > > IN_CHANGE|IN_UPDATE and finally call the ffs_update() ? >=20 > That sounds reasonable to me. I see that ffs_update(), which is called > by ufs_inactive(), looks at fs_ronly. The patch (compile-tested only) is below. diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c index cbccc62..687011d 100644 --- a/sys/ufs/ffs/ffs_vfsops.c +++ b/sys/ufs/ffs/ffs_vfsops.c @@ -79,6 +79,7 @@ static void ffs_oldfscompat_read(struct fs *, struct ufsm= ount *, ufs2_daddr_t); static void ffs_oldfscompat_write(struct fs *, struct ufsmount *); static void ffs_ifree(struct ufsmount *ump, struct inode *ip); +static int ffs_isronly(struct ufsmount *ump); static vfs_init_t ffs_init; static vfs_uninit_t ffs_uninit; static vfs_extattrctl_t ffs_extattrctl; @@ -748,6 +749,7 @@ ffs_mountfs(devvp, mp, td) ump->um_valloc =3D ffs_valloc; ump->um_vfree =3D ffs_vfree; ump->um_ifree =3D ffs_ifree; + ump->um_isronly =3D ffs_isronly; mtx_init(UFS_MTX(ump), "FFS", "FFS Lock", MTX_DEF); bcopy(bp->b_data, ump->um_fs, (u_int)fs->fs_sbsize); if (fs->fs_sbsize < SBLOCKSIZE) @@ -1862,3 +1864,12 @@ ffs_geom_strategy(struct bufobj *bo, struct buf *bp) } g_vfs_strategy(bo, bp); } + +static int +ffs_isronly(struct ufsmount *ump) +{ + struct fs *fs =3D ump->um_fs; + + return (fs->fs_ronly); +} + diff --git a/sys/ufs/ufs/ufs_inode.c b/sys/ufs/ufs/ufs_inode.c index 448f436..a9abbeb 100644 --- a/sys/ufs/ufs/ufs_inode.c +++ b/sys/ufs/ufs/ufs_inode.c @@ -90,8 +90,7 @@ ufs_inactive(ap) ufs_gjournal_close(vp); #endif if ((ip->i_effnlink =3D=3D 0 && DOINGSOFTDEP(vp)) || - (ip->i_nlink <=3D 0 && - (vp->v_mount->mnt_flag & MNT_RDONLY) =3D=3D 0)) { + (ip->i_nlink <=3D 0 && !UFS_ISRONLY(ip->i_ump))) { loop: if (vn_start_secondary_write(vp, &mp, V_NOWAIT) !=3D 0) { /* Cannot delete file while file system is suspended */ @@ -121,7 +120,7 @@ ufs_inactive(ap) } if (ip->i_effnlink =3D=3D 0 && DOINGSOFTDEP(vp)) softdep_releasefile(ip); - if (ip->i_nlink <=3D 0 && (vp->v_mount->mnt_flag & MNT_RDONLY) =3D=3D 0) { + if (ip->i_nlink <=3D 0 && !UFS_ISRONLY(ip->i_ump)) { #ifdef QUOTA if (!getinoquota(ip)) (void)chkiq(ip, -1, NOCRED, FORCE); diff --git a/sys/ufs/ufs/ufsmount.h b/sys/ufs/ufs/ufsmount.h index 80fe3fa..57e7dd0 100644 --- a/sys/ufs/ufs/ufsmount.h +++ b/sys/ufs/ufs/ufsmount.h @@ -93,6 +93,7 @@ struct ufsmount { int (*um_valloc)(struct vnode *, int, struct ucred *, struct vnode **); int (*um_vfree)(struct vnode *, ino_t, int); void (*um_ifree)(struct ufsmount *, struct inode *); + int (*um_isronly)(struct ufsmount *); }; =20 #define UFS_BALLOC(aa, bb, cc, dd, ee, ff) VFSTOUFS((aa)->v_mount)->um_bal= loc(aa, bb, cc, dd, ee, ff) @@ -102,6 +103,7 @@ struct ufsmount { #define UFS_VALLOC(aa, bb, cc, dd) VFSTOUFS((aa)->v_mount)->um_valloc(aa, = bb, cc, dd) #define UFS_VFREE(aa, bb, cc) VFSTOUFS((aa)->v_mount)->um_vfree(aa, bb, cc) #define UFS_IFREE(aa, bb) ((aa)->um_ifree(aa, bb)) +#define UFS_ISRONLY(aa) ((aa)->um_isronly(aa)) =20 #define UFS_LOCK(aa) mtx_lock(&(aa)->um_lock) #define UFS_UNLOCK(aa) mtx_unlock(&(aa)->um_lock) --f4HxWLVbzokH9yio Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.7 (FreeBSD) iD8DBQFHURYtC3+MBN1Mb4gRAuz7AJ402T07oC29X4x7cl3OPoh/EylrqQCcDvoK 82kvkH2TYZEMHmlkfIoZ8Gc= =r8eH -----END PGP SIGNATURE----- --f4HxWLVbzokH9yio--