Re: bin/51116: resize_ffs has problems with non-zero filled expansion of an ffsv2 filesystem

[David Holland <dholland-bugs%netbsd.org@localhost>]

The following reply was made to PR bin/51116; it has been noted by GNATS.

From: David Holland <dholland-bugs%netbsd.org@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: bin/51116: resize_ffs has problems with non-zero filled
 expansion of an ffsv2 filesystem
Date: Sun, 14 Aug 2016 00:31:10 +0000

 On Thu, May 05, 2016 at 08:25:00PM +0000, brad%anduin.eldar.org@localhost wrote:
  > I narrowed the issue down to ffsv2 filesystems that are expanded in a
  > non zero-filled manor.
 
 I think the problem is that resize_ffs is not handling ffsv2's
 deferred inode initialization correctly.
 
 It does
 
 	cg->cg_initediblk = newsb->fs_ipg < 2 * FFS_INOPB(newsb) ?
 	    newsb->fs_ipg : 2 * FFS_INOPB(newsb);
 
 (line 497 in initcg) which declares that two blocks' worth of inodes
 have been written out zeroed, but then later (at line 619) it doesn't
 write the inodes out at all if ffsv2.
 
 If this is the problem the following 100% untested patch might improve
 the situation:
 
 Index: resize_ffs.c
 ===================================================================
 RCS file: /cvsroot/src/sbin/resize_ffs/resize_ffs.c,v
 retrieving revision 1.46
 diff -u -p -r1.46 resize_ffs.c
 --- resize_ffs.c	17 Mar 2016 01:41:54 -0000	1.46
 +++ resize_ffs.c	14 Aug 2016 00:29:11 -0000
 @@ -105,7 +105,8 @@ union dinode {
  	} while (0)
  
  /* a cg's worth of brand new squeaky-clean inodes */
 -static struct ufs1_dinode *zinodes;
 +static struct ufs1_dinode *zinodes1;
 +static struct ufs2_dinode *zinodes2;
  
  /* pointers to the in-core cgs, read off disk and possibly modified */
  static struct cg **cgs;
 @@ -615,10 +616,15 @@ initcg(int cgn)
  	newsb->fs_cstotal.cs_nffree += cg->cg_cs.cs_nffree;
  	newsb->fs_cstotal.cs_nbfree += cg->cg_cs.cs_nbfree;
  	newsb->fs_cstotal.cs_nifree += cg->cg_cs.cs_nifree;
 -	if (is_ufs2 == 0)
 +	if (is_ufs2) {
 +		/* Write out the cleared inodes. */
 +		writeat(FFS_FSBTODB(newsb, cgimin(newsb, cgn)), zinodes2,
 +		    cg->cg_initediblk * sizeof(*zinodes2));
 +	} else {
  		/* Write out the cleared inodes. */
 -		writeat(FFS_FSBTODB(newsb, cgimin(newsb, cgn)), zinodes,
 -		    newsb->fs_ipg * sizeof(*zinodes));
 +		writeat(FFS_FSBTODB(newsb, cgimin(newsb, cgn)), zinodes1,
 +		    newsb->fs_ipg * sizeof(*zinodes1));
 +	}
  	/* Dirty the cg. */
  	cgflags[cgn] |= CGF_DIRTY;
  }
 @@ -993,8 +999,15 @@ grow(void)
  	/* Update the timestamp. */
  	newsb->fs_time = timestamp();
  	/* Allocate and clear the new-inode area, in case we add any cgs. */
 -	zinodes = alloconce(newsb->fs_ipg * sizeof(*zinodes), "zeroed inodes");
 -	memset(zinodes, 0, newsb->fs_ipg * sizeof(*zinodes));
 +	if (is_ufs2) {
 +		zinodes2 = alloconce(newsb->fs_ipg * sizeof(*zinodes2),
 +			"zeroed inodes");
 +		memset(zinodes2, 0, newsb->fs_ipg * sizeof(*zinodes2));
 +	} else {
 +		zinodes1 = alloconce(newsb->fs_ipg * sizeof(*zinodes1),
 +			"zeroed inodes");
 +		memset(zinodes1, 0, newsb->fs_ipg * sizeof(*zinodes1));
 +	}
  	
  	/* Check that the new last sector (frag, actually) is writable.  Since
  	 * it's at least one frag larger than it used to be, we know we aren't
 
 
 -- 
 David A. Holland
 dholland%netbsd.org@localhost