Date: Fri, 18 Mar 2011 16:28:44 +0200 From: Kostik Belousov <kostikbel@gmail.com> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r219712 - head/sys/ufs/ufs Message-ID: <20110318142844.GB78089@deviant.kiev.zoral.com.ua> In-Reply-To: <20110318001402.H1537@besplex.bde.org> References: <201103171123.p2HBNCGh025820@svn.freebsd.org> <20110318001402.H1537@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--7/SBNUKqFhVQCjUu
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Fri, Mar 18, 2011 at 12:37:41AM +1100, Bruce Evans wrote:
> On Thu, 17 Mar 2011, Konstantin Belousov wrote:
>=20
> >Log:
> > Remove the #if defined(FFS) || defined(IFS) braces around the calls to
> > ffs_snapgone(). ufs.ko module is not build with FFS define, causing
> > snapshot inode number slots in superblock never be freed, as well as a
> > reference on the snapshot vnode.
> >
> > IFS was removed several years ago, and UFS/FFS separation was not
> > maintained for real.
> >
> > Reported, analyzed and tested by: Yamagi Burmeister <lists yamagi org>
> > MFC after: 3 days
>=20
> This seems to leave FFS correctly unused. Most options for file systems
> are put in opt_dontuse.h to inhibit bugs like this (but I never figured
> out a way to generate a compile time error if an option in there is
> used). The FFS option is special. It should not be special. It was
> moved from opt_dontuse.h to opt_ffs_broken_fixme.h in 2001 to avoid a
> collateral bug: the FFS/UFS split doesn't work, but was used by ext2fs,
> and it is a layering violation for UFS to call ffs_snapgone() in FFS,
> and this broke the configuration with EXT2FS (and UFS) but not FFS;
> this was hacked around by the ifdefs. ext2fs was decoupled from UFS
> in 2002, so the ifdefs became unnecessary, but they remained to break
> the module. opt_ffs_broken_fixme.h also became unnecessary in 2002,
> but remains to generate history lessons :-).
>=20
> The ifdef on IFS was even more bogus, since IFS doesn't exist and has
> no vestiges in conf/*.
I went ahead and implemented UFS_SNAPONE() and removal of
opt_ffs_broken_fixme.h.
diff --git a/sys/conf/options b/sys/conf/options
index 7f92258..8207800 100644
--- a/sys/conf/options
+++ b/sys/conf/options
@@ -200,6 +200,7 @@ CD9660 opt_dontuse.h
CODA opt_dontuse.h
EXT2FS opt_dontuse.h
FDESCFS opt_dontuse.h
+FFS opt_dontuse.h
HPFS opt_dontuse.h
MSDOSFS opt_dontuse.h
NTFS opt_dontuse.h
@@ -217,9 +218,6 @@ UNIONFS opt_dontuse.h
# Pseudofs debugging
PSEUDOFS_TRACE opt_pseudofs.h
=20
-# Broken - ffs_snapshot() dependency from ufs_lookup() :-(
-FFS opt_ffs_broken_fixme.h
-
# In-kernel GSS-API
KGSSAPI opt_kgssapi.h
KGSSAPI_DEBUG opt_kgssapi.h
diff --git a/sys/modules/ufs/Makefile b/sys/modules/ufs/Makefile
index 0fe7b4c..652856c 100644
--- a/sys/modules/ufs/Makefile
+++ b/sys/modules/ufs/Makefile
@@ -3,8 +3,7 @@
.PATH: ${.CURDIR}/../../ufs/ufs ${.CURDIR}/../../ufs/ffs
=20
KMOD=3D ufs
-SRCS=3D opt_ddb.h opt_directio.h opt_ffs.h opt_ffs_broken_fixme.h \
- opt_quota.h opt_suiddir.h opt_ufs.h \
+SRCS=3D opt_ddb.h opt_directio.h opt_ffs.h opt_quota.h opt_suiddir.h opt_u=
fs.h \
vnode_if.h ufs_acl.c ufs_bmap.c ufs_dirhash.c ufs_extattr.c \
ufs_gjournal.c ufs_inode.c ufs_lookup.c ufs_quota.c ufs_vfsops.c \
ufs_vnops.c ffs_alloc.c ffs_balloc.c ffs_inode.c ffs_snapshot.c \
diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c
index f578382..573c364 100644
--- a/sys/ufs/ffs/ffs_vfsops.c
+++ b/sys/ufs/ffs/ffs_vfsops.c
@@ -797,6 +797,7 @@ ffs_mountfs(devvp, mp, td)
ump->um_vfree =3D ffs_vfree;
ump->um_ifree =3D ffs_ifree;
ump->um_rdonly =3D ffs_rdonly;
+ ump->um_snapgone =3D ffs_snapgone;
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)
diff --git a/sys/ufs/ufs/ufs_lookup.c b/sys/ufs/ufs/ufs_lookup.c
index d819f69..45ebea1 100644
--- a/sys/ufs/ufs/ufs_lookup.c
+++ b/sys/ufs/ufs/ufs_lookup.c
@@ -37,7 +37,6 @@
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
=20
-#include "opt_ffs_broken_fixme.h"
#include "opt_ufs.h"
#include "opt_quota.h"
=20
@@ -1253,7 +1252,7 @@ out:
* when last open reference goes away.
*/
if (ip !=3D 0 && (ip->i_flags & SF_SNAPSHOT) !=3D 0 && ip->i_effnlink =3D=
=3D 0)
- ffs_snapgone(ip);
+ UFS_SNAPGONE(ip);
return (error);
}
=20
@@ -1316,7 +1315,7 @@ ufs_dirrewrite(dp, oip, newinum, newtype, isrmdir)
* when last open reference goes away.
*/
if ((oip->i_flags & SF_SNAPSHOT) !=3D 0 && oip->i_effnlink =3D=3D 0)
- ffs_snapgone(oip);
+ UFS_SNAPGONE(oip);
return (error);
}
=20
diff --git a/sys/ufs/ufs/ufsmount.h b/sys/ufs/ufs/ufsmount.h
index b13db40..c2cfcfb 100644
--- a/sys/ufs/ufs/ufsmount.h
+++ b/sys/ufs/ufs/ufsmount.h
@@ -104,6 +104,7 @@ struct ufsmount {
int (*um_vfree)(struct vnode *, ino_t, int);
void (*um_ifree)(struct ufsmount *, struct inode *);
int (*um_rdonly)(struct inode *);
+ void (*um_snapgone)(struct inode *);
};
=20
#define UFS_BALLOC(aa, bb, cc, dd, ee, ff) VFSTOUFS((aa)->v_mount)->um_bal=
loc(aa, bb, cc, dd, ee, ff)
@@ -114,6 +115,7 @@ struct ufsmount {
#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_RDONLY(aa) ((aa)->i_ump->um_rdonly(aa))
+#define UFS_SNAPGONE(aa) ((aa)->i_ump->um_snapgone(aa))
=20
#define UFS_LOCK(aa) mtx_lock(&(aa)->um_lock)
#define UFS_UNLOCK(aa) mtx_unlock(&(aa)->um_lock)
--7/SBNUKqFhVQCjUu
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (FreeBSD)
iEYEARECAAYFAk2DbBwACgkQC3+MBN1Mb4j4JQCfQc+9ytDaqrH22pRBOYIgYq19
EWAAn1J2gfvx6dtPRWNt9UcUuaR4GRsr
=+VDp
-----END PGP SIGNATURE-----
--7/SBNUKqFhVQCjUu--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110318142844.GB78089>