Date: Sat, 19 Mar 2011 03:51:28 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Kostik Belousov <kostikbel@gmail.com> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Bruce Evans <brde@optusnet.com.au> Subject: Re: svn commit: r219712 - head/sys/ufs/ufs Message-ID: <20110319032048.H3038@besplex.bde.org> In-Reply-To: <20110318142844.GB78089@deviant.kiev.zoral.com.ua> References: <201103171123.p2HBNCGh025820@svn.freebsd.org> <20110318001402.H1537@besplex.bde.org> <20110318142844.GB78089@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 18 Mar 2011, Kostik Belousov wrote: > On Fri, Mar 18, 2011 at 12:37:41AM +1100, Bruce Evans wrote: >> On Thu, 17 Mar 2011, Konstantin Belousov wrote: >> >>> 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 >> >> 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 >> ... >> the module. opt_ffs_broken_fixme.h also became unnecessary in 2002, >> but remains to generate history lessons :-). > > I went ahead and implemented UFS_SNAPONE() and removal of > opt_ffs_broken_fixme.h. Thanks. At first I thought that UFS_SNAPGONE() was pointless, since there are lots of other layering violations and the macros don't make the spit really work. But ffs_snapgone() was the only function named ffs_*() called from ufs. There are just 2 other references to ffs_*() in ufs. Both are in comments in ufs_inode.c. But there are many other layering violations which are not so obvious because the ffs functions are not spelled ffs_*(). Many are spelled softdep_*(). Soft updates are only implemented for ffs and even their stubs were only implemented for ffs, so ufs without ffs cannot compile. The ufs files are actually selected by the ffs option in conf/files, so ufs without ffs is almost impossible anyway. The ffs module is misnamed ufs, and it links all the ffs and ufs files, so if there were a ext2fs, ifs, lfs or otherfs using ufs, especially if these were in modules there would probably be multiple instances ifs ufs. This may even be the best way -- "fork" ufs by creating separate objects from common sources with a few ifdefs, so that you don't have lots of runtime tests and function pointers and extra source code to initialize the function pointers... Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110319032048.H3038>