Skip to contentSkip to conversation

New xfs_repair handling for inode nlink counts

PATCH In review Barry Naujok 2 participants 4 messages Mar 9, 2007 – Mar 17, 2007

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 New xfs_repair handling for inode nlink counts

The attached patch has 3 parts to it:
- optimised phase 7 (inode nlink count) speed
- improved memory usage for inode nlink counts
- memory usage tracking
- other speed improvements

Overall, phase 7 is almost instant, and phases 6/7 use less memory than current versions of xfs_repair.

The optimised phase 7 involved the patches to:

dino_chunks.c
This stores the on-disk nlink count for inodes into the inode tree that is created in phase 3.

phase7.c
This compares the on-disk nlink counts read in phase 3 to the actual count it should be generated in phase 6. If they are different, creates a transaction and updates the inode on disk. No other disk I/O is generated.

incore.h
Added disk_nlinks to ino_tree_node_t structure and renamed nlinks to counted_nlinks in the backptrs_t structure. Also created set/get_inode_disk_nlinks inline functions.

Due to the massive increase in memory required to store these counts for each inode in the filesystem, I have implemented memory optimisation using a dynamically sized elements for each inode cluster. Initially, they start at 8 bits each and double in bits as required by inodes with large nlink counts. This implementation uses an "nlinkops" function pointers to keep CPU usage to a minimum. This is entirely implemented in incore.h and incore_ino.c.

To measure memory used by various parts xfs_repair, I implemented memory tracking in global.h and global.c. Default is not to compile this in, but can enabled by defining TRACK_MEMORY when compiling these two files.

Finally, a small enhancement was made in xfs_repair.c. For filesystems that fit within the libxfs block cache, phase 6 6 is now significantly faster by flushing dirty blocks to disk rather than purging them from memory and then re-reading again them during phase 6. The flush is required as the libxfs block and inode cache is not unified.

repair/globals.c +180 -0
Index: xfsprogs/repair/globals.c
===================================================================
@@ -20,3 +20,183 @@
 
 #define EXTERN
 #include "globals.h"
+
+#ifdef TRACK_MEMORY
+
+#undef calloc
+#undef malloc
+#undef memalign
+#undef realloc
+#undef free
+
+/*
+ * Track by file name pointer and also by return pointer
+ */
+
+typedef struct func {
+	const char	*file;
+	int		line;
+	int64_t		acount;
+	int64_t		fcount;
+	int64_t		rcount;
+	int64_t		current;
+	int64_t		peak;
+} func_t;
+
+typedef struct entry {
+	struct entry	*next;
+	func_t		*fileline;
+	size_t		size;
+	void		*ptr;
+} entry_t;
+
+static int	caller_count = 0;
+static int	caller_size = 0;
+static func_t	*callers = NULL;
+
+static entry_t	*ptrhash[256];
+
+static
+void track_alloc(const char *file, int line, size_t size, void *p)
+{
+	int		i;
+	entry_t		*e;
+
+	/* find an existing func call from file/line */
+	for (i = 0; i < caller_count; i++) {
+		if ((callers[i].file == file) && (callers[i].line == line))
+			break;
+	}
+	if (i == caller_count) {	/* add new func if not found */
+		if (caller_count == caller_size) {
+			caller_size += 64;
+			callers = realloc(callers, sizeof(func_t) * caller_size);
+		}
+		memset(&callers[i], 0, sizeof(func_t));
+		callers[i].file = file;
+		callers[i].line = line;
+		caller_count++;
+	}
+
+	e = malloc(sizeof(entry_t));
+	e->size = size;
+	e->ptr = p;
+	e->fileline = &callers[i];
+
+	callers[i].acount++;
+	callers[i].current += size;
+	if (callers[i].current > callers[i].peak)
+		callers[i].peak = callers[i].current;
+
+	/* add pointer to hash list, very basic simple hash function */
+	i = (((size_t)p) >> 8) & 0xff;
+
+	e->next = ptrhash[i];
+	ptrhash[i] = e;
+}
+
+void *track_calloc(const char *file, int line, size_t num, size_t size)
+{
+	void *retval = calloc(num, size);
+
+	if (retval != NULL)
+		track_alloc(file, line, num * size, retval);
+
+	return retval;
+}
+
+void *track_malloc(const char *file, int line, size_t size)
+{
+	void *retval = malloc(size);
+
+	if (retval != NULL)
+		track_alloc(file, line, size, retval);
+
+	return retval;
+}
+
+void *track_memalign(const char *file, int line, size_t boundary, size_t size)
+{
+	void *retval = memalign(boundary, size);
+
+	if (retval != NULL)
+		track_alloc(file, line, size, retval);
+
+	return retval;
+}
+
+void *track_realloc(const char *file, int line, void *ptr, size_t size)
+{
+	int		i;
+	entry_t		*e, *prev;
+	void 		*newptr = realloc(ptr, size);
+
+	if (ptr == NULL && newptr != NULL) {
+		track_alloc(file, line, size, newptr);
+		return newptr;
+	}
+
+	i = (((size_t)ptr) >> 8) & 0xff;
+
+	prev = NULL;
+	for (e = ptrhash[i]; e; e = e->next) {
+		if (e->ptr == ptr)
+			break;
+		prev = e;
+	}
+	if (!e)
+		return newptr;
+
+	e->fileline->rcount++;
+	e->fileline->current = e->fileline->current + size - e->size;
+	if (e->fileline->current > e->fileline->peak)
+		e->fileline->peak = e->fileline->current;
+	e->size = size;
+	e->ptr = newptr;
+
+	return newptr;
+}
+
+void track_free(const char *file, int line, void *ptr)
+{
+	int		i;
+	entry_t	*e, *prev;
+
+	free(ptr);
+
+	/* find associated entry */
+	i = (((size_t)ptr) >> 8) & 0xff;
+
+	prev = NULL;
+	for (e = ptrhash[i]; e; e = e->next) {
+		if (e->ptr == ptr)
+			break;
+		prev = e;
+	}
+	if (!e)
+		return;
+
+	e->fileline->fcount++;
+	e->fileline->current -= e->size;
+
+	if (prev)
+		prev->next = e->next;
+	else
+		ptrhash[i] = e->next;
+	free(e);
+}
+
+void print_memory_usage(void)
+{
+	int i;
+
+	printf("%20s:line \ta_cnt\tf_cnt\tr_cnt\tremain\tpeak\n", "file");
+	for (i = 0; i < caller_count; i++) {
+		printf("%20s:%-5d\t%lld\t%lld\t%lld\t%lld\t%lld\n",
+			callers[i].file, callers[i].line,
+			callers[i].acount, callers[i].fcount, callers[i].rcount,
+			callers[i].current, callers[i].peak);
+	}
+}
+
+#endif /* TRACK_MEMORY */
\ No newline at end of file
repair/globals.h +25 -0
Index: xfsprogs/repair/globals.h
===================================================================
@@ -16,6 +16,16 @@
  * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#ifdef TRACK_MEMORY
+
+#define calloc(n,s)	track_calloc(__FILE__, __LINE__, (n), (s))
+#define malloc(s)	track_malloc(__FILE__, __LINE__, (s))
+#define memalign(b,s)	track_memalign(__FILE__, __LINE__, (b), (s))
+#define realloc(p,s)	track_realloc(__FILE__, __LINE__, (p), (s))
+#define free(p)		track_free(__FILE__, __LINE__, (p))
+
+#endif
+
 #ifndef _XFS_REPAIR_GLOBAL_H
 #define _XFS_REPAIR_GLOBAL_H
 
@@ -23,6 +33,21 @@
 #define EXTERN extern
 #endif
 
+#ifdef TRACK_MEMORY
+
+void print_memory_usage(void);
+void *track_calloc(const char *file, int line, size_t num, size_t size);
+void *track_malloc(const char *file, int line, size_t size);
+void *track_memalign(const char *file, int line, size_t boundary, size_t size);
+void *track_realloc(const char *file, int line, void *ptr, size_t size);
+void track_free(const char *file, int line, void *ptr);
+
+#else
+
+#define print_memory_usage()	do { } while(0)
+
+#endif
+
 /* useful macros */
 
 #define rounddown(x, y) (((x)/(y))*(y))
repair/incore.h +34 -7

97 lines not on this page · raw message

repair/incore_ino.c +237 -48

328 lines not on this page · raw message

repair/xfs_repair.c +7 -2
Index: xfsprogs/repair/xfs_repair.c
===================================================================
@@ -277,7 +277,7 @@
 		case 't':
 			report_interval = (int) strtol(optarg, 0, 0);
 			break;
-			
+
 		case '?':
 			usage();
 		}
@@ -563,7 +563,7 @@
 
 	/* XXX: nathans - something in phase4 ain't playing by */
 	/* the buffer cache rules.. why doesn't IRIX hit this? */
-	libxfs_bcache_purge();
+	libxfs_bcache_flush();
 
 	if (no_modify)
 		printf(_("No modify flag set, skipping phase 5\n"));
@@ -576,6 +576,8 @@
 		phase6(mp);
 		timestamp(PHASE_END, 6, NULL);
 
+		libxfs_bcache_flush();
+
 		phase7(mp);
 		timestamp(PHASE_END, 7, NULL);
 	} else  {
@@ -640,6 +642,9 @@
 	if (do_parallel && report_interval)
 		stop_progress_rpt();
 
+	if (verbose > 1)
+		print_memory_usage();
+
 	if (no_modify)  {
 		do_log(
 	_("No modify flag set, skipping filesystem flush and exiting.\n"));
repair/dino_chunks.c +7 -0
Index: xfsprogs/repair/dino_chunks.c
===================================================================
@@ -779,6 +779,13 @@
 					do_warn(_("would correct imap\n"));
 			}
 			set_inode_used(ino_rec, irec_offset);
+			/*
+			 * store on-disk nlink count for comparing in phase 7
+			 */
+			set_inode_disk_nlinks(ino_rec, irec_offset,
+				dino->di_core.di_version > XFS_DINODE_VERSION_1
+					? be32_to_cpu(dino->di_core.di_nlink)
+					: be16_to_cpu(dino->di_core.di_onlink));
 		} else  {
 			set_inode_free(ino_rec, irec_offset);
 		}
repair/phase7.c +96 -232

404 lines not on this page · raw message

#1 lore raw

+#ifdef TRACK_MEMORY
+
+#undef calloc
+#undef malloc
+#undef memalign
+#undef realloc
+#undef free

	Can you put all thise into a memory_tracking.h file that
	gets include with:

	#ifdef TRACK_MEMORY
	#include "track_memory.h"
	#endif

	Instead of polluting the implementation file?

+ /* add pointer to hash list, very basic simple hash function */
+ i = (((size_t)p) >> 8) & 0xff;

+ i = (((size_t)ptr) >> 8) & 0xff;

Note that there is not guarantee that size_t and pointers have the same lenght, and there are system where it's not (win64?), better cast things use uintptr_t here.

repair/globals.h +10 -0
@@ -16,6 +16,16 @@
  * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#ifdef TRACK_MEMORY
+
+#define calloc(n,s)	track_calloc(__FILE__, __LINE__, (n), (s))
+#define malloc(s)	track_malloc(__FILE__, __LINE__, (s))
+#define memalign(b,s)	track_memalign(__FILE__, __LINE__, (b), (s))
+#define realloc(p,s)	track_realloc(__FILE__, __LINE__, (p), (s))
+#define free(p)		track_free(__FILE__, __LINE__, (p))
+
+#endif
+
 #ifndef _XFS_REPAIR_GLOBAL_H
 #define _XFS_REPAIR_GLOBAL_H
 

The memory tracking should probably come after the inclusion guards.

#2 lore raw

Hi Christoph,

Thanks for the feedback. I've attached an update to the trackmem stuff for review. globals.c is now unmodified.

Regards,
Barry.

Quoted 58 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Christoph Hellwig
Sent: Friday, 9 March 2007 6:34 PM
To: Barry Naujok
Cc: [email protected]; [email protected]
Subject: Re: [PATCH] New xfs_repair handling for inode nlink counts

(quoted diff) +7 -0
+#ifdef TRACK_MEMORY
+
+#undef calloc
+#undef malloc
+#undef memalign
+#undef realloc
+#undef free

	Can you put all thise into a memory_tracking.h file that
	gets include with:

	#ifdef TRACK_MEMORY
	#include "track_memory.h"
	#endif

	Instead of polluting the implementation file?
(quoted diff) +3 -0
+	/* add pointer to hash list, very basic simple hash function */
+	i = (((size_t)p) >> 8) & 0xff;
 
+	i = (((size_t)ptr) >> 8) & 0xff;

Note that there is not guarantee that size_t and
pointers have the
same lenght, and there are system where it's not
(win64?), better
cast things use uintptr_t here.

repair/globals.h +10 -0
@@ -16,6 +16,16 @@
  * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#ifdef TRACK_MEMORY
+
+#define calloc(n,s)	track_calloc(__FILE__, __LINE__, (n), (s))
+#define malloc(s)	track_malloc(__FILE__, __LINE__, (s))
+#define memalign(b,s)	track_memalign(__FILE__, 
 __LINE__, (b), (s))
+#define realloc(p,s)	track_realloc(__FILE__, __LINE__, (p), (s))
+#define free(p)		track_free(__FILE__, __LINE__, (p))
+
+#endif
+
 #ifndef _XFS_REPAIR_GLOBAL_H
 #define _XFS_REPAIR_GLOBAL_H
Quoted 4 lines

The memory tracking should probably come after the inclusion guards.

===========================================================================
xfsprogs/repair/Makefile
===========================================================================

xfsprogs/repair/Makefile +3 -2
@@ -9,13 +9,14 @@ LTCOMMAND = xfs_repair
 
 HFILES = agheader.h attr_repair.h avl.h avl64.h bmap.h dinode.h dir.h \
 	dir2.h dir_stack.h err_protos.h globals.h incore.h protos.h rt.h \
-	progress.h scan.h versions.h prefetch.h threads.h
+	progress.h scan.h versions.h prefetch.h threads.h trackmem.h
 
 CFILES = agheader.c attr_repair.c avl.c avl64.c bmap.c dino_chunks.c \
 	dinode.c dir.c dir2.c dir_stack.c globals.c incore.c \
 	incore_bmc.c init.c incore_ext.c incore_ino.c phase1.c \
 	phase2.c phase3.c phase4.c phase5.c phase6.c phase7.c rt.c sb.c \
-	progress.c prefetch.c scan.c versions.c xfs_repair.c threads.c
+	progress.c prefetch.c scan.c versions.c xfs_repair.c threads.c \
+	trackmem.c
 
 LLDLIBS = $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBPTHREAD) $(LIBRT)
 LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG)

===========================================================================
xfsprogs/repair/globals.h
===========================================================================

xfsprogs/repair/globals.h +4 -0
@@ -23,6 +23,10 @@
 #define EXTERN extern
 #endif
 
+#ifdef TRACK_MEMORY
+#include "trackmem.h"
+#endif
+
 /* useful macros */
 
 #define rounddown(x, y) (((x)/(y))*(y))

===========================================================================
xfsprogs/repair/trackmem.c
===========================================================================

xfsprogs/repair/trackmem.c +195 -0

195 lines not on this page · raw message

===========================================================================
xfsprogs/repair/trackmem.h
===========================================================================

xfsprogs/repair/trackmem.h +35 -0
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2007 Silicon Graphics, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef _XFS_REPAIR_TRACK_MEM_H
+#define _XFS_REPAIR_TRACK_MEM_H
+
+#define calloc(n,s)	track_calloc(__FILE__, __LINE__, (n), (s))
+#define malloc(s)	track_malloc(__FILE__, __LINE__, (s))
+#define memalign(b,s)	track_memalign(__FILE__, __LINE__, (b), (s))
+#define realloc(p,s)	track_realloc(__FILE__, __LINE__, (p), (s))
+#define free(p)		track_free(__FILE__, __LINE__, (p))
+
+void print_memory_usage(void);
+void *track_calloc(const char *file, int line, size_t num, size_t size);
+void *track_malloc(const char *file, int line, size_t size);
+void *track_memalign(const char *file, int line, size_t boundary, size_t size);
+void *track_realloc(const char *file, int line, void *ptr, size_t size);
+void track_free(const char *file, int line, void *ptr);
+
+#endif /* _XFS_REPAIR_TRACK_MEM_H */

===========================================================================
xfsprogs/repair/xfs_repair.c
===========================================================================

xfsprogs/repair/xfs_repair.c +7 -1
@@ -563,7 +563,7 @@ main(int argc, char **argv)
 
 	/* XXX: nathans - something in phase4 ain't playing by */
 	/* the buffer cache rules.. why doesn't IRIX hit this? */
-	libxfs_bcache_purge();
+	libxfs_bcache_flush();
 
 	if (no_modify)
 		printf(_("No modify flag set, skipping phase 5\n"));
@@ -576,6 +576,8 @@ main(int argc, char **argv)
 		phase6(mp);
 		timestamp(PHASE_END, 6, NULL);
 
+		libxfs_bcache_flush();
+
 		phase7(mp);
 		timestamp(PHASE_END, 7, NULL);
 	} else  {
@@ -640,6 +642,10 @@ _("Warning:  project quota information w
 	if (do_parallel && report_interval)
 		stop_progress_rpt();
 
+#ifdef TRACK_MEMORY
+	print_memory_usage();
+#endif
+
 	if (no_modify)  {
 		do_log(
 	_("No modify flag set, skipping filesystem flush and exiting.\n"));
#3 lore raw
Quoted 4 lines · Barry Naujok

Hi Christoph,

Thanks for the feedback. I've attached an update to the trackmem stuff for review. globals.c is now unmodified.

Looks good to me.