libxfs: stop caching inode structures

PATCH In review Christoph Hellwig 2 participants 3 messages Feb 7, 2012 – Feb 9, 2012

Share & tools

Christoph Hellwig

author

2012-02-07 18:22 UTC #0 lore raw

PATCH 1/1 libxfs: stop caching inode structures

Currently libxfs has a cache for xfs_inode structures. Unlike in kernelspace where the inode cache, and the associated page cache for file data is used for all filesystem operations the libxfs inode cache is only used in few places:

- the libxfs init code reads the root and realtime inodes when called from xfs_db using a special flag, but these inode structure are never referenced again
- mkfs uses namespace and bmap routines that take the xfs_inode structure to create the root and realtime inodes, as well as any additional files specified in the proto file
- the xfs_db attr code uses xfs_inode-based attr routines in the attrset and attrget commands
- phase6 of xfs_repair uses xfs_inode-based routines for rebuilding directories and moving files to the lost+found directory.
- phase7 of xfs_repair uses struct xfs_inode to modify the nlink count of inodes.

So except in repair we never ever reuse a cached inode, and in repair we can easily read the information from the more compact cached buffers (or even better rewrite phase7 to operate on the raw on-disk inodes). Given these facts stop caching the inodes to reduce memory usage especially in xfs_repair.

With this we probably could increase the memory available to the buffer cache in xfs_repair, but trying to do so I got a bit lost - the current formula seems to magic to me to make any sense, and simply doubling the buffer cache size causes us to run out of memory given that the data cached in the buffer cache (typically lots of 8k inode buffers and few 4k other metadata buffers) are much bigger than the inodes cached in the inode cache. We probably need a sizing scheme that takes the actual amount of memory allocated to the buffer cache into account to solve this better.

Signed-off-by Christoph Hellwig <[email protected]>

include/libxfs.h +0 -5

Index: xfsprogs-dev/include/libxfs.h
===================================================================
@@ -207,7 +207,6 @@ typedef struct xfs_mount {
 #define LIBXFS_MOUNT_COMPAT_ATTR	0x0010
 #define LIBXFS_MOUNT_ATTR2		0x0020
 
-#define LIBXFS_IHASHSIZE(sbp)		(1<<10)
 #define LIBXFS_BHASHSIZE(sbp) 		(1<<10)
 
 extern xfs_mount_t	*libxfs_mount (xfs_mount_t *, xfs_sb_t *,

@@ -335,7 +334,6 @@ extern int	libxfs_writebuf_int(xfs_buf_t
 extern int	libxfs_readbufr(dev_t, xfs_daddr_t, xfs_buf_t *, int, int);
 
 extern int libxfs_bhash_size;
-extern int libxfs_ihash_size;
 
 #define LIBXFS_BREAD	0x1
 #define LIBXFS_BWRITE	0x2

@@ -455,9 +453,6 @@ extern int	libxfs_iread (xfs_mount_t *,
 				xfs_inode_t *, xfs_daddr_t);
 
 /* Inode Cache Interfaces */
-extern struct cache	*libxfs_icache;
-extern struct cache_operations	libxfs_icache_operations;
-extern void	libxfs_icache_purge (void);
 extern int	libxfs_iget (xfs_mount_t *, xfs_trans_t *, xfs_ino_t,
 				uint, xfs_inode_t **, xfs_daddr_t);
 extern void	libxfs_iput (xfs_inode_t *, uint);

libxfs/init.c +0 -9

Index: xfsprogs-dev/libxfs/init.c
===================================================================
@@ -22,9 +22,6 @@
 
 char *progname = "libxfs";	/* default, changed by each tool */
 
-struct cache *libxfs_icache;	/* global inode cache */
-int libxfs_ihash_size;		/* #buckets in icache */
-
 struct cache *libxfs_bcache;	/* global buffer cache */
 int libxfs_bhash_size;		/* #buckets in bcache */

@@ -333,9 +330,6 @@ libxfs_init(libxfs_init_t *a)
 	}
 	if (needcd)
 		chdir(curdir);
-	if (!libxfs_ihash_size)
-		libxfs_ihash_size = LIBXFS_IHASHSIZE(sbp);
-	libxfs_icache = cache_init(libxfs_ihash_size, &libxfs_icache_operations);
 	if (!libxfs_bhash_size)
 		libxfs_bhash_size = LIBXFS_BHASHSIZE(sbp);
 	libxfs_bcache = cache_init(libxfs_bhash_size, &libxfs_bcache_operations);

@@ -817,7 +811,6 @@ libxfs_umount(xfs_mount_t *mp)
 	int			agno;
 
 	libxfs_rtmount_destroy(mp);
-	libxfs_icache_purge();
 	libxfs_bcache_purge();
 
 	for (agno = 0; agno < mp->m_maxagi; agno++) {

@@ -833,7 +826,6 @@ void
 libxfs_destroy(void)
 {
 	manage_zones(1);
-	cache_destroy(libxfs_icache);
 	cache_destroy(libxfs_bcache);
 }

@@ -849,7 +841,6 @@ libxfs_report(FILE *fp)
 	time_t t;
 	char *c;
 
-	cache_report(fp, "libxfs_icache", libxfs_icache);
 	cache_report(fp, "libxfs_bcache", libxfs_bcache);
 
 	t = time(NULL);

libxfs/rdwr.c +16 ~~-65~~

106 lines not on this page · raw message

mkfs/xfs_mkfs.c +0 -1

Index: xfsprogs-dev/mkfs/xfs_mkfs.c
===================================================================
@@ -2698,7 +2698,6 @@ an AG size that is one stripe unit small
 	 * Need to drop references to inodes we still hold, first.
 	 */
 	libxfs_rtmount_destroy(mp);
-	libxfs_icache_purge();
 	libxfs_bcache_purge();
 
 	/*

repair/xfs_repair.c +3 ~~-11~~

Index: xfsprogs-dev/repair/xfs_repair.c
===================================================================
@@ -69,7 +69,6 @@ static char *c_opts[] = {
 };
 
 
-static int	ihash_option_used;
 static int	bhash_option_used;
 static long	max_mem_specified;	/* in megabytes */
 static int	phase2_threads = 32;

@@ -239,13 +238,13 @@ process_args(int argc, char **argv)
 					pre_65_beta = 1;
 					break;
 				case IHASH_SIZE:
-					libxfs_ihash_size = (int)strtol(val, NULL, 0);
-					ihash_option_used = 1;
+					do_warn(
+		_("-o ihash option has been removed and will be ignored\n"));
 					break;
 				case BHASH_SIZE:
 					if (max_mem_specified)
 						do_abort(
-			_("-o bhash option cannot be used with -m option\n"));
+		_("-o bhash option cannot be used with -m option\n"));
 					libxfs_bhash_size = (int)strtol(val, NULL, 0);
 					bhash_option_used = 1;
 					break;

@@ -646,9 +645,7 @@ main(int argc, char **argv)
 		unsigned long	max_mem;
 		struct rlimit	rlim;
 
-		libxfs_icache_purge();
 		libxfs_bcache_purge();
-		cache_destroy(libxfs_icache);
 		cache_destroy(libxfs_bcache);
 
 		mem_used = (mp->m_sb.sb_icount >> (10 - 2)) +

@@ -707,11 +704,6 @@ main(int argc, char **argv)
 			do_log(_("        - block cache size set to %d entries\n"),
 				libxfs_bhash_size * HASH_CACHE_RATIO);
 
-		if (!ihash_option_used)
-			libxfs_ihash_size = libxfs_bhash_size;
-
-		libxfs_icache = cache_init(libxfs_ihash_size,
-						&libxfs_icache_operations);
 		libxfs_bcache = cache_init(libxfs_bhash_size,
 						&libxfs_bcache_operations);
 	}

man/man8/xfs_repair.8 +0 -6

Index: xfsprogs-dev/man/man8/xfs_repair.8
===================================================================
@@ -130,12 +130,6 @@ The
 supported are:
 .RS 1.0i
 .TP
-.BI ihash= ihashsize
-overrides the default inode cache hash size. The total number of
-inode cache entries are limited to 8 times this amount. The default
-.I ihashsize
-is 1024 (for a total of 8192 entries).
-.TP
 .BI bhash= bhashsize
 overrides the default buffer cache hash size. The total number of
 buffer cache entries are limited to 8 times this amount. The default

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Dave Chinner

2012-02-08 05:11 UTC #1 lore raw

Quoted 18 lines · Christoph Hellwig

Currently libxfs has a cache for xfs_inode structures. Unlike in kernelspace where the inode cache, and the associated page cache for file data is used for all filesystem operations the libxfs inode cache is only used in few places:
- the libxfs init code reads the root and realtime inodes when called from xfs_db using a special flag, but these inode structure are never referenced again
- mkfs uses namespace and bmap routines that take the xfs_inode structure to create the root and realtime inodes, as well as any additional files specified in the proto file
- the xfs_db attr code uses xfs_inode-based attr routines in the attrset and attrget commands
- phase6 of xfs_repair uses xfs_inode-based routines for rebuilding directories and moving files to the lost+found directory.
- phase7 of xfs_repair uses struct xfs_inode to modify the nlink count of inodes.

Quoted 4 lines

So except in repair we never ever reuse a cached inode, and in repair we can easily read the information from the more compact cached buffers (or even better rewrite phase7 to operate on the raw on-disk inodes). Given these facts stop caching the inodes to reduce memory usage especially in xfs_repair.

Ok, so what does it do to the speed of phase6 and phase7 of repair? How much CPU overhead does this add to every inode lookup done in these phases?

Indeed, there are cases where individual inode caching is much more memory efficient than keeping the buffers around (think sparse inode chunks on disk where only a few of the 64 inodes are actually allocated). Tracking them in buffers (esp. if the inode size is large) could use a lot more memory than just caching the active inodes in a struct xfs_inode. Hence I'm not so sure this is clear cut win for memory usage.

Do you have any numbers for memory usage or performance?

The code changes are simple enough, so if it is actually a win then I see no problems with doing this. But that's what I need more information about to be convinced on....

Quoted 4 lines

With this we probably could increase the memory available to the buffer cache in xfs_repair, but trying to do so I got a bit lost - the current formula seems to magic to me to make any sense, and simply doubling the buffer cache size causes us to run out of memory given that the data cached

IIRC, that's because the current formula sets the buffer cache size to 75% of physical RAM on the machine. Doubling it will definitely cause problems ;)

Cheers,

Dave.

Signature · 7 lines

Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Christoph Hellwig

author

2012-02-09 18:01 UTC #2 lore raw

Quoted 3 lines · Dave Chinner

Ok, so what does it do to the speed of phase6 and phase7 of repair? How much CPU overhead does this add to every inode lookup done in these phases?

I'm away from my test system, but on the tons of inodes filesystems it actually slightly improved their speed, probably because the box was swapping less, or we spent less time in inode cache doing cache misses as we'd never actually have the inode we care about cached.

The reason why the individual inode cache here doesn't work is because we only every touched inodes in phase7 if we are going to modify them and write them out, so we absolutely need the backing buffer anyway.

I can't see how phase6 benefits from the logical inode cache either, given it's structure:

- in phase 6a we iterate over each inode in the incore inode tree, and if it's a directory check/rebuild it
- phase6b then updates the "." and ".." entries for directories that need, which means we require the backing buffers.
- phase6c moves disconnected inodes to lost_found, which again needs the backing buffer to actually do anything.

In short there is no code in repair that benefits from doing logical inode caching.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs