Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 2 Apr 2021 18:55:38 GMT
From:      Kirk McKusick <mckusick@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: fc56fd262d0b - main - Ensure that all allocated data structures in fsck_ffs are freed.
Message-ID:  <202104021855.132ItcaP077937@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by mckusick:

URL: https://cgit.FreeBSD.org/src/commit/?id=fc56fd262d0bc8ee523f6c8e6a65c0ff5417af6e

commit fc56fd262d0bc8ee523f6c8e6a65c0ff5417af6e
Author:     Kirk McKusick <mckusick@FreeBSD.org>
AuthorDate: 2021-04-02 18:57:34 +0000
Commit:     Kirk McKusick <mckusick@FreeBSD.org>
CommitDate: 2021-04-02 18:58:49 +0000

    Ensure that all allocated data structures in fsck_ffs are freed.
    
    Several large data structures are allocated by fsck_ffs to track
    resource usage. Most but not all were deallocated at the end of
    checking each filesystem. This commit consolidates the freeing
    of all data structures in one place and adds one that had previously
    been missing.
    
    It is important to clean up these data structures as they can be
    large. If the previous allocations have not been freed, fsck_ffs
    can run out of address space when many large filesystems are being
    checked. An alternative would be to fork a new instance of fsck_ffs
    for each filesystem to be checked, but we choose to free the small
    set of large structures to save the fork overhead.
    
    Reported by:  Chuck Silvers
    Tested by:    Chuck Silvers
    MFC after:    7 days
    Sponsored by: Netflix
---
 sbin/fsck_ffs/fsck.h   |  4 +++-
 sbin/fsck_ffs/fsutil.c | 47 +++++++++++++++++++++++++++++++++++++----------
 sbin/fsck_ffs/globs.c  |  2 --
 sbin/fsck_ffs/main.c   |  6 ------
 4 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/sbin/fsck_ffs/fsck.h b/sbin/fsck_ffs/fsck.h
index 676350b75767..9ecc5793e644 100644
--- a/sbin/fsck_ffs/fsck.h
+++ b/sbin/fsck_ffs/fsck.h
@@ -236,10 +236,12 @@ extern int sujrecovery;			/* 1 => doing check using the journal */
 } while (0)
 #define	initbarea(bp, type) do { \
 	(bp)->b_bno = (ufs2_daddr_t)-1; \
+	(bp)->b_size = 0; \
+	(bp)->b_errs = 0; \
 	(bp)->b_flags = 0; \
+	(bp)->b_type = type; \
 	(bp)->b_refcnt = 0; \
 	(bp)->b_index = 0; \
-	(bp)->b_type = type; \
 } while (0)
 
 #define	sbdirty()	dirty(&sblk)
diff --git a/sbin/fsck_ffs/fsutil.c b/sbin/fsck_ffs/fsutil.c
index 127884400651..ca19f6726af5 100644
--- a/sbin/fsck_ffs/fsutil.c
+++ b/sbin/fsck_ffs/fsutil.c
@@ -84,7 +84,6 @@ static LIST_HEAD(bufhash, bufarea) bufhashhd[HASHSIZE]; /* buffer hash list */
 static int numbufs;				/* size of buffer cache */
 static int cachelookups;			/* number of cache lookups */
 static int cachereads;				/* number of cache reads */
-static struct bufarea *cgbufs;	/* header for cylinder group cache */
 static int flushtries;		/* number of tries to reclaim memory */
 
 char *buftype[BT_NUMBUFTYPES] = BT_NAMES;
@@ -187,13 +186,9 @@ bufinit(void)
 {
 	int i;
 
-	pdirbp = (struct bufarea *)0;
-	bzero(&cgblk, sizeof(struct bufarea));
-	cgblk.b_un.b_buf = Malloc((unsigned int)sblock.fs_bsize);
-	if (cgblk.b_un.b_buf == NULL)
+	if ((cgblk.b_un.b_buf = Malloc((unsigned int)sblock.fs_bsize)) == NULL)
 		errx(EEXIT, "Initial malloc(%d) failed", sblock.fs_bsize);
 	initbarea(&cgblk, BT_CYLGRP);
-	cgbufs = NULL;
 	numbufs = cachelookups = cachereads = 0;
 	TAILQ_INIT(&bufqueuehd);
 	for (i = 0; i < HASHSIZE; i++)
@@ -559,7 +554,8 @@ void
 ckfini(int markclean)
 {
 	struct bufarea *bp, *nbp;
-	int ofsmodified, cnt;
+	struct inoinfo *inp, *ninp;
+	int ofsmodified, cnt, cg, i;
 
 	if (bkgrdflag) {
 		unlink(snapname);
@@ -609,16 +605,20 @@ ckfini(int markclean)
 			free(cgbufs[cnt].b_un.b_cg);
 		}
 		free(cgbufs);
+		cgbufs = NULL;
 	}
 	flush(fswritefd, &cgblk);
 	free(cgblk.b_un.b_buf);
+	cgblk.b_un.b_buf = NULL;
 	cnt = 0;
 	/* Step 2: indirect, directory, external attribute, and data blocks */
 	if (debug)
 		printf("Flush indirect, directory, external attribute, "
 		    "and data blocks\n");
-	if (pdirbp != NULL)
+	if (pdirbp != NULL) {
 		brelse(pdirbp);
+		pdirbp = NULL;
+	}
 	TAILQ_FOREACH_REVERSE_SAFE(bp, &bufqueuehd, bufqueue, b_list, nbp) {
 		switch (bp->b_type) {
 		/* These should not be in the buffer cache list */
@@ -658,8 +658,10 @@ ckfini(int markclean)
 	/* Step 3: inode blocks */
 	if (debug)
 		printf("Flush inode blocks\n");
-	if (icachebp != NULL)
+	if (icachebp != NULL) {
 		brelse(icachebp);
+		icachebp = NULL;
+	}
 	TAILQ_FOREACH_REVERSE_SAFE(bp, &bufqueuehd, bufqueue, b_list, nbp) {
 		if (debug && bp->b_refcnt != 0) {
 			prtbuf("ckfini: clearing in-use buffer", bp);
@@ -686,7 +688,6 @@ ckfini(int markclean)
 		sbdirty();
 		flush(fswritefd, &sblk);
 	}
-	pdirbp = (struct bufarea *)0;
 	if (cursnapshot == 0 && sblock.fs_clean != markclean) {
 		if ((sblock.fs_clean = markclean) != 0) {
 			sblock.fs_flags &= ~(FS_UNCLEAN | FS_NEEDSFSCK);
@@ -711,6 +712,32 @@ ckfini(int markclean)
 			rerun = 1;
 		}
 	}
+	/*
+	 * Free allocated tracking structures.
+	 */
+	if (blockmap != NULL)
+		free(blockmap);
+	blockmap = NULL;
+	if (inostathead != NULL) {
+		for (cg = 0; cg < sblock.fs_ncg; cg++)
+			if (inostathead[cg].il_stat != NULL)
+				free((char *)inostathead[cg].il_stat);
+		free(inostathead);
+	}
+	inostathead = NULL;
+	if (inpsort != NULL)
+		free(inpsort);
+	inpsort = NULL;
+	if (inphead != NULL) {
+		for (i = 0; i < dirhash; i++) {
+			for (inp = inphead[i]; inp != NULL; inp = ninp) {
+				ninp = inp->i_nexthash;
+				free(inp);
+			}
+		}
+		free(inphead);
+	}
+	inphead = NULL;
 	finalIOstats();
 	(void)close(fsreadfd);
 	(void)close(fswritefd);
diff --git a/sbin/fsck_ffs/globs.c b/sbin/fsck_ffs/globs.c
index 45d6b80d8fe8..be4434ce38ca 100644
--- a/sbin/fsck_ffs/globs.c
+++ b/sbin/fsck_ffs/globs.c
@@ -128,7 +128,6 @@ fsckinit(void)
 	bzero(totalreadtime, sizeof(struct timespec) * BT_NUMBUFTYPES);
 	bzero(&startprog, sizeof(struct timespec));
 	bzero(&sblk, sizeof(struct bufarea));
-	pdirbp = NULL;
 	cursnapshot = 0;
 	listmax = numdirs = dirhash = inplast = 0;
 	countdirs = 0;
@@ -159,7 +158,6 @@ fsckinit(void)
 	fsreadfd = 0;
 	fswritefd = 0;
 	maxfsblock = 0;
-	blockmap = NULL;
 	maxino = 0;
 	lfdir = 0;
 	lfname = "lost+found";
diff --git a/sbin/fsck_ffs/main.c b/sbin/fsck_ffs/main.c
index 65cee9b7b8c6..401ee10f9be3 100644
--- a/sbin/fsck_ffs/main.c
+++ b/sbin/fsck_ffs/main.c
@@ -243,7 +243,6 @@ checkfilesys(char *filesys)
 	char errmsg[255];
 	int ofsmodified;
 	int iovlen;
-	int cylno;
 	intmax_t blks, files;
 	size_t size;
 
@@ -627,11 +626,6 @@ checkfilesys(char *filesys)
 		resolved = 0;
 	ckfini(resolved);
 
-	for (cylno = 0; cylno < sblock.fs_ncg; cylno++)
-		if (inostathead[cylno].il_stat != NULL)
-			free((char *)inostathead[cylno].il_stat);
-	free((char *)inostathead);
-	inostathead = NULL;
 	if (fsmodified && !preen)
 		printf("\n***** FILE SYSTEM WAS MODIFIED *****\n");
 	if (rerun) {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202104021855.132ItcaP077937>