Bug 153584 - [ext2fs] [patch] Performance fix and cleanups for BSD licensed ext2fs
Summary: [ext2fs] [patch] Performance fix and cleanups for BSD licensed ext2fs
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 8.2-BETA1
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-fs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-31 16:00 UTC by Pedro F. Giffuni
Modified: 2011-02-01 19:38 UTC (History)
0 users

See Also:


Attachments
file.diff (9.04 KB, patch)
2010-12-31 16:00 UTC, Pedro F. Giffuni
no flags Details | Diff
patch-ext2fs.txt (10.10 KB, patch)
2011-01-03 03:50 UTC, Pedro F. Giffuni
no flags Details | Diff
patch-ext2fs.txt (8.71 KB, patch)
2011-01-18 21:24 UTC, Pedro F. Giffuni
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pedro F. Giffuni 2010-12-31 16:00:24 UTC
Since the adoption of the BSD licensed ext2fs implementation, a performance decrease was observed.
Bruce Evans found the source for some of the problems:
http://lists.freebsd.org/pipermail/freebsd-fs/2010-September/009589.html

In addition to these, some merges from UFS1 have been backported and some style issues have been fixed. See PR 142924 and 143345 for the details.

This patch include bde's fixes and applies the basic cleanups from the previous PRs.

It also adds back the async mount option as it is operational after the initial BSD licensed backporting: this has been tested as part of the recent GSoC project.

Fix: Patch follows.

Patch attached with submission follows:
Comment 1 Pedro F. Giffuni 2011-01-03 03:50:08 UTC
A small update ...
This includes a small patch for the #ifdef KDB case
in ext2_subr.c.

Thanks to Doug Barton for reporting the issue.


      
Comment 2 Mark Linimon freebsd_committer freebsd_triage 2011-01-03 20:58:15 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-fs

Over to maintainer(s).
Comment 3 Pedro F. Giffuni 2011-01-18 21:24:51 UTC
Oops.. cleanup some comments that were not supposed to be there.

While here, I should mention that Bruce Evans' patch has been giving some great results with blogbench:

http://blogs.freebsdish.org/liuzheng/2011/01/10/ext2fs-performance-benchmark-blogbench/

and bonnie++
http://blogs.freebsdish.org/liuzheng/2011/01/18/ext2fs-performance-benchmark-bonnie/


      
Comment 4 dfilter service freebsd_committer freebsd_triage 2011-01-21 21:33:52 UTC
Author: jhb
Date: Fri Jan 21 21:33:46 2011
New Revision: 217702
URL: http://svn.freebsd.org/changeset/base/217702

Log:
  Restore support for the 'async' and 'sync' mount options lost when
  switching to nmount(2).  While here, sort the options.
  
  PR:		kern/153584
  Submitted by:	Pedro F. Giffuni  giffunip at yahoo
  MFC after:	1 week

Modified:
  head/sys/fs/ext2fs/ext2_vfsops.c

Modified: head/sys/fs/ext2fs/ext2_vfsops.c
==============================================================================
--- head/sys/fs/ext2fs/ext2_vfsops.c	Fri Jan 21 18:32:29 2011	(r217701)
+++ head/sys/fs/ext2fs/ext2_vfsops.c	Fri Jan 21 21:33:46 2011	(r217702)
@@ -95,9 +95,9 @@ static int	ext2_check_sb_compat(struct e
 static int	compute_sb_data(struct vnode * devvp,
 		    struct ext2fs * es, struct m_ext2fs * fs);
 
-static const char *ext2_opts[] = { "from", "export", "acls", "noexec",
-    "noatime", "union", "suiddir", "multilabel", "nosymfollow",
-    "noclusterr", "noclusterw", "force", NULL };
+static const char *ext2_opts[] = { "acls", "async", "noatime", "noclusterr", 
+    "noclusterw", "noexec", "export", "force", "from", "multilabel",
+    "suiddir", "nosymfollow", "sync", "union", NULL };
 
 /*
  * VFS Operations.
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 5 dfilter service freebsd_committer freebsd_triage 2011-01-21 22:00:47 UTC
Author: jhb
Date: Fri Jan 21 22:00:40 2011
New Revision: 217703
URL: http://svn.freebsd.org/changeset/base/217703

Log:
  - Move special inode constants to ext2_dinode.h and rename them to match
    NetBSD.
  - Add a constant for the HASJOURNAL compat flag.
  
  PR:		kern/153584
  Submitted by:	Pedro F. Giffuni  giffunip at yahoo

Modified:
  head/sys/fs/ext2fs/ext2_dinode.h
  head/sys/fs/ext2fs/ext2fs.h

Modified: head/sys/fs/ext2fs/ext2_dinode.h
==============================================================================
--- head/sys/fs/ext2fs/ext2_dinode.h	Fri Jan 21 21:33:46 2011	(r217702)
+++ head/sys/fs/ext2fs/ext2_dinode.h	Fri Jan 21 22:00:40 2011	(r217703)
@@ -32,6 +32,23 @@
 #define e2di_size_high	e2di_dacl
 
 /*
+ * Special inode numbers
+ * The root inode is the root of the file system.  Inode 0 can't be used for
+ * normal purposes and bad blocks are normally linked to inode 1, thus
+ * the root inode is 2.
+ * Inode 3 to 10 are reserved in ext2fs.
+ */
+#define	EXT2_BADBLKINO		((ino_t)1)
+#define	EXT2_ROOTINO		((ino_t)2)
+#define	EXT2_ACLIDXINO		((ino_t)3)
+#define	EXT2_ACLDATAINO		((ino_t)4)
+#define	EXT2_BOOTLOADERINO	((ino_t)5)
+#define	EXT2_UNDELDIRINO	((ino_t)6)
+#define	EXT2_RESIZEINO		((ino_t)7)
+#define	EXT2_JOURNALINO		((ino_t)8)
+#define	EXT2_FIRSTINO		((ino_t)11)
+
+/*
  * Inode flags
  * The current implementation uses only EXT2_IMMUTABLE and EXT2_APPEND flags
  */

Modified: head/sys/fs/ext2fs/ext2fs.h
==============================================================================
--- head/sys/fs/ext2fs/ext2fs.h	Fri Jan 21 21:33:46 2011	(r217702)
+++ head/sys/fs/ext2fs/ext2fs.h	Fri Jan 21 22:00:40 2011	(r217703)
@@ -39,22 +39,6 @@
 
 #include <sys/types.h>
 
-/*
- * Special inode numbers
- */
-#define	EXT2_BAD_INO		 1	/* Bad blocks inode */
-#define EXT2_ROOT_INO		 2	/* Root inode */
-#define EXT2_BOOT_LOADER_INO	 5	/* Boot loader inode */
-#define EXT2_UNDEL_DIR_INO	 6	/* Undelete directory inode */
-
-/* First non-reserved inode for old ext2 filesystems */
-#define E2FS_REV0_FIRST_INO	11
-
-/*
- * The second extended file system magic number
- */
-#define E2FS_MAGIC		0xEF53
-
 #if defined(_KERNEL)
 /*
  * FreeBSD passes the pointer to the in-core struct with relevant
@@ -170,7 +154,7 @@ struct m_ext2fs {
 	uint32_t e2fs_mount_opt;
 	uint32_t e2fs_blocksize_bits;
 	uint32_t e2fs_total_dir;  /* Total number of directories */
-	uint8_t	*e2fs_contigdirs;
+	uint8_t	*e2fs_contigdirs; /* (u) # of contig. allocated dirs */
 	char e2fs_wasvalid;       /* valid at mount time */
 	off_t e2fs_maxfilesize;
 	struct ext2_gd *e2fs_gd; /* Group Descriptors */
@@ -182,6 +166,14 @@ struct m_ext2fs {
 #define E2FS_DATE		"95/08/09"
 #define E2FS_VERSION		"0.5b"
 
+/* First non-reserved inode for old ext2 filesystems */
+#define E2FS_REV0_FIRST_INO	11
+
+/*
+ * The second extended file system magic number
+ */
+#define E2FS_MAGIC		0xEF53
+
 /*
  * Revision levels
  */
@@ -197,6 +189,7 @@ struct m_ext2fs {
  * compatible/incompatible features
  */
 #define EXT2F_COMPAT_PREALLOC		0x0001
+#define EXT2F_COMPAT_HASJOURNAL		0x0004
 #define EXT2F_COMPAT_RESIZE		0x0010
 
 #define EXT2F_ROCOMPAT_SPARSESUPER	0x0001
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 6 dfilter service freebsd_committer freebsd_triage 2011-02-01 18:21:52 UTC
Author: jhb
Date: Tue Feb  1 18:21:45 2011
New Revision: 218175
URL: http://svn.freebsd.org/changeset/base/218175

Log:
  - Set the next_alloc fields for an i-node after allocating a new block
    so that future allocations start with most recently allocated block
    rather than the beginning of the filesystem.
  - Fix ext2_alloccg() to properly scan for 8 block chunks that are not
    aligned on 8-bit boundaries.  Previously this was causing new blocks
    to be allocated in a highly fragmented fashion (block 0 of a file at
    lbn N, block 1 at lbn N + 8, block 2 at lbn N + 16, etc.).
  - Cosmetic tweaks to the currently-disabled fancy realloc sysctls.
  
  PR:		kern/153584
  Discussed with:	bde
  Tested by:	Pedro F. Giffuni  giffunip at yahoo, Zheng Liu (lz)

Modified:
  head/sys/fs/ext2fs/ext2_alloc.c

Modified: head/sys/fs/ext2fs/ext2_alloc.c
==============================================================================
--- head/sys/fs/ext2fs/ext2_alloc.c	Tue Feb  1 17:42:57 2011	(r218174)
+++ head/sys/fs/ext2fs/ext2_alloc.c	Tue Feb  1 18:21:45 2011	(r218175)
@@ -59,6 +59,10 @@ static u_long	ext2_hashalloc(struct inod
 						int));
 static daddr_t	ext2_nodealloccg(struct inode *, int, daddr_t, int);
 static daddr_t  ext2_mapsearch(struct m_ext2fs *, char *, daddr_t);
+#ifdef FANCY_REALLOC
+static int	ext2_reallocblks(struct vop_reallocblks_args *);
+#endif
+
 /*
  * Allocate a block in the file system.
  *
@@ -108,13 +112,17 @@ ext2_alloc(ip, lbn, bpref, size, cred, b
 		goto nospace;
 	if (bpref >= fs->e2fs->e2fs_bcount)
 		bpref = 0;
-	 if (bpref == 0)
+	if (bpref == 0)
                 cg = ino_to_cg(fs, ip->i_number);
         else
                 cg = dtog(fs, bpref);
         bno = (daddr_t)ext2_hashalloc(ip, cg, bpref, fs->e2fs_bsize,
                                                  ext2_alloccg);
         if (bno > 0) {
+		/* set next_alloc fields as done in block_getblk */
+		ip->i_next_alloc_block = lbn;
+		ip->i_next_alloc_goal = bno;
+
                 ip->i_blocks += btodb(fs->e2fs_bsize);
                 ip->i_flag |= IN_CHANGE | IN_UPDATE;
                 *bnp = bno;
@@ -143,13 +151,14 @@ nospace:
  */
 
 #ifdef FANCY_REALLOC
-#include <sys/sysctl.h>
+SYSCTL_NODE(_vfs, OID_AUTO, ext2fs, CTLFLAG_RW, 0, "EXT2FS filesystem");
+
 static int doasyncfree = 1;
-static int doreallocblks = 1;
+SYSCTL_INT(_vfs_ext2fs, OID_AUTO, doasyncfree, CTLFLAG_RW, &doasyncfree, 0,
+    "Use asychronous writes to update block pointers when freeing blocks");
 
-#ifdef	OPT_DEBUG
-SYSCTL_INT(_debug, 14, doasyncfree, CTLFLAG_RW, &doasyncfree, 0, "");
-#endif	/* OPT_DEBUG */
+static int doreallocblks = 1;
+SYSCTL_INT(_vfs_ext2fs, OID_AUTO, doreallocblks, CTLFLAG_RW, &doreallocblks, 0, "");
 #endif
 
 int
@@ -624,7 +633,8 @@ ext2_alloccg(struct inode *ip, int cg, d
 	struct m_ext2fs *fs;
 	struct buf *bp;
 	struct ext2mount *ump;
-	int error, bno, start, end, loc;
+	daddr_t bno, runstart, runlen;
+	int bit, loc, end, error, start;
 	char *bbp;
 	/* XXX ondisk32 */
 	fs = ip->i_e2fs;
@@ -665,18 +675,52 @@ ext2_alloccg(struct inode *ip, int cg, d
 	else
 		start = 0;
 	end = howmany(fs->e2fs->e2fs_fpg, NBBY) - start;
+retry:
+	runlen = 0;
+	runstart = 0;
 	for (loc = start; loc < end; loc++) {
-		if (bbp[loc] == 0) {
-			bno = loc * NBBY;
-			goto gotit;
+		if (bbp[loc] == (char)0xff) {
+			runlen = 0;
+			continue;
 		}
-	}
-	for (loc = 0; loc < start; loc++) {
-		if (bbp[loc] == 0) {
-			bno = loc * NBBY;
+
+		/* Start of a run, find the number of high clear bits. */
+		if (runlen == 0) {
+			bit = fls(bbp[loc]);
+			runlen = NBBY - bit;
+			runstart = loc * NBBY + bit;
+		} else if (bbp[loc] == 0) {
+			/* Continue a run. */
+			runlen += NBBY;
+		} else {
+			/*
+			 * Finish the current run.  If it isn't long
+			 * enough, start a new one.
+			 */
+			bit = ffs(bbp[loc]) - 1;
+			runlen += bit;
+			if (runlen >= 8) {
+				bno = runstart;
+				goto gotit;
+			}
+
+			/* Run was too short, start a new one. */
+			bit = fls(bbp[loc]);
+			runlen = NBBY - bit;
+			runstart = loc * NBBY + bit;
+		}
+
+		/* If the current run is long enough, use it. */
+		if (runlen >= 8) {
+			bno = runstart;
 			goto gotit;
 		}
 	}
+	if (start != 0) {
+		end = start;
+		start = 0;
+		goto retry;
+	}
 
 	bno = ext2_mapsearch(fs, bbp, bpref);
 	if (bno < 0){
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 7 John Baldwin freebsd_committer freebsd_triage 2011-02-01 19:37:42 UTC
State Changed
From-To: open->closed

Various patches applied to HEAD.  Closed at submitter's request.