On Tue, Nov 20, 2012 at 6:20 AM, Bruce Evans <b...@optusnet.com.au> wrote: > On Mon, 19 Nov 2012, Attilio Rao wrote: > >> Log: >> r16312 is not any longer real since many years (likely since when VFS >> received granular locking) but the comment present in UFS has been >> copied all over other filesystems code incorrectly for several times. >> >> Removes comments that makes no sense now. > > > It still made sense (except for bitrot in the function name), but might not > be true). The code made sense with it. Now the code makes no sense. > > >> Modified: head/sys/ufs/ffs/ffs_vfsops.c >> >> ============================================================================== >> --- head/sys/ufs/ffs/ffs_vfsops.c Mon Nov 19 21:58:14 2012 >> (r243310) >> +++ head/sys/ufs/ffs/ffs_vfsops.c Mon Nov 19 22:43:45 2012 >> (r243311) >> @@ -1676,14 +1676,6 @@ ffs_vgetf(mp, ino, flags, vpp, ffs_flags >> ump = VFSTOUFS(mp); >> dev = ump->um_dev; >> fs = ump->um_fs; >> - >> - /* >> - * If this malloc() is performed after the getnewvnode() > > > This malloc() didn't match the code, which uses uma_zalloc(). Old > versions used MALLOC() in both the comment and the code. ffs's comment > was updated to say malloc() when the code was changed to use malloc(), > then rotted when the code was changed to use uma_zalloc(). In some > other file systems, the comment still said MALLOC(). > > >> - * it might block, leaving a vnode with a NULL v_data to be >> - * found by ffs_sync() if a sync happens to fire right then, >> - * which will cause a panic because ffs_sync() blindly >> - * dereferences vp->v_data (as well it should). >> - */ >> ip = uma_zalloc(uma_inode, M_WAITOK | M_ZERO); >> >> /* Allocate a new vnode/inode. */ >> > > The code makes no sense now. The comment explains why ip is allocated > before vp, instead of in the natural, opposite order like it used to > be. Allocating things in an unnatural order requires extra code to > free ip when the allocation of vp fails.
"Used to be" is very arguably. The code has been like its current form many more years than the opposite (16 against 3 I think). And the code makes perfectly sense if you know the history. So I don't agree with you. Thanks for the other comments, I will try to squeeze a patch about it. Attilio -- Peace can only be achieved by understanding - A. Einstein _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
