The addition of 64k block capability in the rec_len_from_disk
and rec_len_to_disk functions added a bit of math overhead which
slows down file create workloads needlessly when the architecture
cannot even support 64k blocks, thanks to page size limits.
The directory entry checking can also be optimized a bit
by sprinkling in some unlikely() conditions to move the
error handling out of line.
These 2 patches speed up a bonnie++ file creation workload for
me by several percent on ext2 & ext3.
Thanks,
-Eric
The addition of 64k block capability in the rec_len_from_disk
and rec_len_to_disk functions added a bit of math overhead which
slows down file create workloads needlessly when the architecture
cannot even support 64k blocks, thanks to page size limits.
The directory entry checking can also be optimized a bit
by sprinkling in some unlikely() conditions to move the
error handling out of line.
bonnie++ sequential file creates on a 512MB ramdisk speeds up
from about 2200/s to about 2500/s, about a 14% improvement.
Signed-off-by: Eric Sandeen <[email protected]>
---
diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index 2709b34..47cda41 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -28,21 +28,30 @@
typedef struct ext2_dir_entry_2 ext2_dirent;
+/*
+ * Tests against MAX_REC_LEN etc were put in place for 64k block
+ * sizes; if that is not possible on this arch, we can skip
+ * those tests and speed things up.
+ */
static inline unsigned ext2_rec_len_from_disk(__le16 dlen)
{
unsigned len = le16_to_cpu(dlen);
+#if (PAGE_CACHE_SIZE >= 65536)
if (len == EXT2_MAX_REC_LEN)
return 1 << 16;
+#endif
return len;
}
static inline __le16 ext2_rec_len_to_disk(unsigned len)
{
+#if (PAGE_CACHE_SIZE >= 65536)
if (len == (1 << 16))
return cpu_to_le16(EXT2_MAX_REC_LEN);
else
BUG_ON(len > (1 << 16));
+#endif
return cpu_to_le16(len);
}
@@ -129,15 +138,15 @@ static void ext2_check_page(struct page *page, int quiet)
p = (ext2_dirent *)(kaddr + offs);
rec_len = ext2_rec_len_from_disk(p->rec_len);
- if (rec_len < EXT2_DIR_REC_LEN(1))
+ if (unlikely(rec_len < EXT2_DIR_REC_LEN(1)))
goto Eshort;
- if (rec_len & 3)
+ if (unlikely(rec_len & 3))
goto Ealign;
- if (rec_len < EXT2_DIR_REC_LEN(p->name_len))
+ if (unlikely(rec_len < EXT2_DIR_REC_LEN(p->name_len)))
goto Enamelen;
- if (((offs + rec_len - 1) ^ offs) & ~(chunk_size-1))
+ if (unlikely(((offs + rec_len - 1) ^ offs) & ~(chunk_size-1)))
goto Espan;
- if (le32_to_cpu(p->inode) > max_inumber)
+ if (unlikely(le32_to_cpu(p->inode) > max_inumber))
goto Einumber;
}
if (offs != limit)
The addition of 64k block capability in the rec_len_from_disk
and rec_len_to_disk functions added a bit of math overhead which
slows down file create workloads needlessly when the architecture
cannot even support 64k blocks, thanks to page size limits.
Similar changes already exist in the ext4 codebase.
The directory entry checking can also be optimized a bit
by sprinkling in some unlikely() conditions to move the
error handling out of line.
bonnie++ sequential file creates on a 512MB ramdisk speeds up
from about 77,000/s to about 82,000/s, about a 6% improvement.
Signed-off-by: Eric Sandeen <[email protected]>
---
diff --git a/fs/ext3/dir.c b/fs/ext3/dir.c
index e2e72c3..34f0a07 100644
--- a/fs/ext3/dir.c
+++ b/fs/ext3/dir.c
@@ -69,25 +69,26 @@ int ext3_check_dir_entry (const char * function, struct inode * dir,
const char * error_msg = NULL;
const int rlen = ext3_rec_len_from_disk(de->rec_len);
- if (rlen < EXT3_DIR_REC_LEN(1))
+ if (unlikely(rlen < EXT3_DIR_REC_LEN(1)))
error_msg = "rec_len is smaller than minimal";
- else if (rlen % 4 != 0)
+ else if (unlikely(rlen % 4 != 0))
error_msg = "rec_len % 4 != 0";
- else if (rlen < EXT3_DIR_REC_LEN(de->name_len))
+ else if (unlikely(rlen < EXT3_DIR_REC_LEN(de->name_len)))
error_msg = "rec_len is too small for name_len";
- else if (((char *) de - bh->b_data) + rlen > dir->i_sb->s_blocksize)
+ else if (unlikely((((char *) de - bh->b_data) + rlen > dir->i_sb->s_blocksize)))
error_msg = "directory entry across blocks";
- else if (le32_to_cpu(de->inode) >
- le32_to_cpu(EXT3_SB(dir->i_sb)->s_es->s_inodes_count))
+ else if (unlikely(le32_to_cpu(de->inode) >
+ le32_to_cpu(EXT3_SB(dir->i_sb)->s_es->s_inodes_count)))
error_msg = "inode out of bounds";
- if (error_msg != NULL)
+ if (unlikely(error_msg != NULL))
ext3_error (dir->i_sb, function,
"bad entry in directory #%lu: %s - "
"offset=%lu, inode=%lu, rec_len=%d, name_len=%d",
dir->i_ino, error_msg, offset,
(unsigned long) le32_to_cpu(de->inode),
rlen, de->name_len);
+
return error_msg == NULL ? 1 : 0;
}
diff --git a/include/linux/ext3_fs.h b/include/linux/ext3_fs.h
index 6ce1bca..e5ad445 100644
--- a/include/linux/ext3_fs.h
+++ b/include/linux/ext3_fs.h
@@ -724,21 +724,30 @@ struct ext3_dir_entry_2 {
~EXT3_DIR_ROUND)
#define EXT3_MAX_REC_LEN ((1<<16)-1)
+/*
+ * Tests against MAX_REC_LEN etc were put in place for 64k block
+ * sizes; if that is not possible on this arch, we can skip
+ * those tests and speed things up.
+ */
static inline unsigned ext3_rec_len_from_disk(__le16 dlen)
{
unsigned len = le16_to_cpu(dlen);
+#if (PAGE_CACHE_SIZE >= 65536)
if (len == EXT3_MAX_REC_LEN)
return 1 << 16;
+#endif
return len;
}
static inline __le16 ext3_rec_len_to_disk(unsigned len)
{
+#if (PAGE_CACHE_SIZE >= 65536)
if (len == (1 << 16))
return cpu_to_le16(EXT3_MAX_REC_LEN);
else if (len > (1 << 16))
BUG();
+#endif
return cpu_to_le16(len);
}
On Tue, 07 Dec 2010 11:51:05 -0600
Eric Sandeen <[email protected]> wrote:
> The addition of 64k block capability in the rec_len_from_disk
> and rec_len_to_disk functions added a bit of math overhead which
> slows down file create workloads needlessly when the architecture
> cannot even support 64k blocks, thanks to page size limits.
>
> The directory entry checking can also be optimized a bit
> by sprinkling in some unlikely() conditions to move the
> error handling out of line.
>
> bonnie++ sequential file creates on a 512MB ramdisk speeds up
> from about 2200/s to about 2500/s, about a 14% improvement.
>
hrm, that's an improbably-large sounding improvement from eliminating
just a few test-n-branches from a pretty heavyweight operation.
On 12/7/10 3:07 PM, Andrew Morton wrote:
> On Tue, 07 Dec 2010 11:51:05 -0600
> Eric Sandeen <[email protected]> wrote:
>
>> The addition of 64k block capability in the rec_len_from_disk
>> and rec_len_to_disk functions added a bit of math overhead which
>> slows down file create workloads needlessly when the architecture
>> cannot even support 64k blocks, thanks to page size limits.
>>
>> The directory entry checking can also be optimized a bit
>> by sprinkling in some unlikely() conditions to move the
>> error handling out of line.
>>
>> bonnie++ sequential file creates on a 512MB ramdisk speeds up
>> from about 2200/s to about 2500/s, about a 14% improvement.
>>
>
> hrm, that's an improbably-large sounding improvement from eliminating
> just a few test-n-branches from a pretty heavyweight operation.
And yet ...
Yeah, I dunno. Part of it is that ext2_add_link does a linear
search, so when you do rec_len_from_disk 50,000 times on a dir,
that little bit adds up quite badly I suppose.
Retesting at a bunch of different number-of-files in bonnie
(with a small sample size so probably a little noise)
|files per sec|
files stock patched delta
10,000 12300 14700 +19%
20,000 6300 7600 +20%
30,000 4200 5000 +20%
40,000 3150 3700 +17%
50,000 2500 3000 +20%
(again all on a 512MB ramdisk)
*shrug* I'll believe my lyin' eyes, I guess. :)
-Eric
On Tue, 07 Dec 2010 15:22:55 -0600
Eric Sandeen <[email protected]> wrote:
> On 12/7/10 3:07 PM, Andrew Morton wrote:
> > On Tue, 07 Dec 2010 11:51:05 -0600
> > Eric Sandeen <[email protected]> wrote:
> >
> >> The addition of 64k block capability in the rec_len_from_disk
> >> and rec_len_to_disk functions added a bit of math overhead which
> >> slows down file create workloads needlessly when the architecture
> >> cannot even support 64k blocks, thanks to page size limits.
> >>
> >> The directory entry checking can also be optimized a bit
> >> by sprinkling in some unlikely() conditions to move the
> >> error handling out of line.
> >>
> >> bonnie++ sequential file creates on a 512MB ramdisk speeds up
> >> from about 2200/s to about 2500/s, about a 14% improvement.
> >>
> >
> > hrm, that's an improbably-large sounding improvement from eliminating
> > just a few test-n-branches from a pretty heavyweight operation.
>
> And yet ...
>
> Yeah, I dunno. Part of it is that ext2_add_link does a linear
> search, so when you do rec_len_from_disk 50,000 times on a dir,
> that little bit adds up quite badly I suppose.
oh yeah, that would do it.
> Retesting at a bunch of different number-of-files in bonnie
> (with a small sample size so probably a little noise)
>
> |files per sec|
> files stock patched delta
> 10,000 12300 14700 +19%
> 20,000 6300 7600 +20%
> 30,000 4200 5000 +20%
> 40,000 3150 3700 +17%
> 50,000 2500 3000 +20%
>
> (again all on a 512MB ramdisk)
>
> *shrug* I'll believe my lyin' eyes, I guess. :)
>
I bet other tweaks in there would yield similar goodliness.
On 12/7/10 3:33 PM, Andrew Morton wrote:
> I bet other tweaks in there would yield similar goodliness.
Yep, probably so ...
We could also speed up ext3/4 a bit by not doing the extN_check_dir_entry
call on every single access, but only when we read it from disk.
Given that I've talked about it for 4 years and never done it, maybe
I'll try to find a volunteer ;)
-Eric
On 2010-12-07, at 14:33, Andrew Morton wrote:
> On Tue, 07 Dec 2010 15:22:55 -0600
> Eric Sandeen <[email protected]> wrote:
>> Retesting at a bunch of different number-of-files in bonnie
>> (with a small sample size so probably a little noise)
>>
>> |files per sec|
>> files stock patched delta
>> 10,000 12300 14700 +19%
>> 20,000 6300 7600 +20%
>> 30,000 4200 5000 +20%
>> 40,000 3150 3700 +17%
>> 50,000 2500 3000 +20%
>>
>> (again all on a 512MB ramdisk)
>>
>> *shrug* I'll believe my lyin' eyes, I guess. :)
>
> I bet other tweaks in there would yield similar goodliness.
I think an important factor here is that this is being tested on a ramdisk, and is likely CPU bound, so any CPU reduction will directly be measured as a performance improvement. Probably oprofile is in order to see where other major CPU users are.
In the past, ext3_mark_inode_dirty() was a major offender, and I recall that we discussed on the ext4 concall a mechanism to only copy the inode once per transaction so that overhead could be removed.
Cheers, Andreas
On 12/08/2010 01:01 PM, Andreas Dilger wrote:
> On 2010-12-07, at 14:33, Andrew Morton wrote:
>> On Tue, 07 Dec 2010 15:22:55 -0600 Eric Sandeen
>> <[email protected]> wrote:
>>> Retesting at a bunch of different number-of-files in bonnie (with
>>> a small sample size so probably a little noise)
>>>
>>> |files per sec| files stock patched delta 10,000 12300 14700
>>> +19% 20,000 6300 7600 +20% 30,000 4200 5000 +20% 40,000 3150
>>> 3700 +17% 50,000 2500 3000 +20%
>>>
>>> (again all on a 512MB ramdisk)
>>>
>>> *shrug* I'll believe my lyin' eyes, I guess. :)
>>
>> I bet other tweaks in there would yield similar goodliness.
>
> I think an important factor here is that this is being tested on a
> ramdisk, and is likely CPU bound, so any CPU reduction will directly
> be measured as a performance improvement. Probably oprofile is in
> order to see where other major CPU users are.
Yep, I ran oprofile.
samples % app name symbol name
1140046 41.8702 ext2.ko ext2_find_entry
1052117 38.6408 ext2.ko ext2_add_link
98424 3.6148 vmlinux native_safe_halt
40461 1.4860 vmlinux wait_on_page_read
29084 1.0682 vmlinux find_get_page
pretty slammed on those 2 ext2 functions! I think it's pretty
overwhelmed by the linear search.
-Eric
> In the past, ext3_mark_inode_dirty() was a major offender, and I
> recall that we discussed on the ext4 concall a mechanism to only copy
> the inode once per transaction so that overhead could be removed.
>
> Cheers, Andreas
>
>
>
>
>
On 2010-12-08, at 14:07, Eric Sandeen wrote:
> On 12/08/2010 01:01 PM, Andreas Dilger wrote:
>> I think an important factor here is that this is being tested on a
>> ramdisk, and is likely CPU bound, so any CPU reduction will directly
>> be measured as a performance improvement. Probably oprofile is in
>> order to see where other major CPU users are.
>
> Yep, I ran oprofile.
>
> samples % app name symbol name
> 1140046 41.8702 ext2.ko ext2_find_entry
> 1052117 38.6408 ext2.ko ext2_add_link
> 98424 3.6148 vmlinux native_safe_halt
> 40461 1.4860 vmlinux wait_on_page_read
> 29084 1.0682 vmlinux find_get_page
>
> pretty slammed on those 2 ext2 functions! I think it's pretty
> overwhelmed by the linear search.
Can you test ext4 with nojournal mode, but with dir_index enabled? I suspect that testing ext2 for directory performance is pointless. My personal threshold for ext2 directories was 10k files before I considered it a lost cause, and all of your tests are with 10k+ files per directory.
Just another log on the fire beneath getting rid of ext2 (and eventually ext3) in favour of ext4, IMHO. I'd be surprised if there are many benchmarks that ext2 can beat ext4 in nojournal mode, if allowed to enable "reversible" format changes like dir_index, uninit_bg, etc.
Cheers, Andreas
On 12/08/2010 04:44 PM, Andreas Dilger wrote:
> On 2010-12-08, at 14:07, Eric Sandeen wrote:
>> On 12/08/2010 01:01 PM, Andreas Dilger wrote:
>>> I think an important factor here is that this is being tested on a
>>> ramdisk, and is likely CPU bound, so any CPU reduction will directly
>>> be measured as a performance improvement. Probably oprofile is in
>>> order to see where other major CPU users are.
>> Yep, I ran oprofile.
>>
>> samples % app name symbol name
>> 1140046 41.8702 ext2.ko ext2_find_entry
>> 1052117 38.6408 ext2.ko ext2_add_link
>> 98424 3.6148 vmlinux native_safe_halt
>> 40461 1.4860 vmlinux wait_on_page_read
>> 29084 1.0682 vmlinux find_get_page
>>
>> pretty slammed on those 2 ext2 functions! I think it's pretty
>> overwhelmed by the linear search.
> Can you test ext4 with nojournal mode, but with dir_index enabled? I suspect that testing ext2 for directory performance is pointless. My personal threshold for ext2 directories was 10k files before I considered it a lost cause, and all of your tests are with 10k+ files per directory.
>
> Just another log on the fire beneath getting rid of ext2 (and eventually ext3) in favour of ext4, IMHO. I'd be surprised if there are many benchmarks that ext2 can beat ext4 in nojournal mode, if allowed to enable "reversible" format changes like dir_index, uninit_bg, etc.
>
> Cheers, Andreas
>
If we could get rid of ext2 (and eventually ext3), it would actually help reduce
the testing matrix and possibly let us invest even more in testing ext4. Having
to maintain three very similar code bases and test them all for correctness and
performance is a real pain :)
ric
On 12/8/10 3:44 PM, Andreas Dilger wrote:
> On 2010-12-08, at 14:07, Eric Sandeen wrote:
>> On 12/08/2010 01:01 PM, Andreas Dilger wrote:
>>> I think an important factor here is that this is being tested on
>>> a ramdisk, and is likely CPU bound, so any CPU reduction will
>>> directly be measured as a performance improvement. Probably
>>> oprofile is in order to see where other major CPU users are.
>>
>> Yep, I ran oprofile.
>>
>> samples % app name symbol name 1140046
>> 41.8702 ext2.ko ext2_find_entry 1052117 38.6408
>> ext2.ko ext2_add_link 98424 3.6148 vmlinux
>> native_safe_halt 40461 1.4860 vmlinux
>> wait_on_page_read 29084 1.0682 vmlinux
>> find_get_page
>>
>> pretty slammed on those 2 ext2 functions! I think it's pretty
>> overwhelmed by the linear search.
>
> Can you test ext4 with nojournal mode, but with dir_index enabled? I
> suspect that testing ext2 for directory performance is pointless.
Oh, I agree. I just had a report of a regression in a certain distro,
which is frowned upon... :) I'm positive ext4 nojournal would beat the
pants off it due to dir_index.
> My personal threshold for ext2 directories was 10k files before I
> considered it a lost cause, and all of your tests are with 10k+ files
> per directory.
Agreed, it's not very realistic but then it was a simple fix too.
Still took time though...
> Just another log on the fire beneath getting rid of ext2 (and
> eventually ext3) in favour of ext4, IMHO. I'd be surprised if there
> are many benchmarks that ext2 can beat ext4 in nojournal mode, if
> allowed to enable "reversible" format changes like dir_index,
> uninit_bg, etc.
Hey, I wouldn't complain.
-Eric
> Cheers, Andreas
>
>
>
>
>
> -- To unsubscribe from this list: send the line "unsubscribe
> linux-ext4" in the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 08, 2010 at 08:06:25PM -0500, Ric Wheeler wrote:
> If we could get rid of ext2 (and eventually ext3), it would actually
> help reduce the testing matrix and possibly let us invest even more
> in testing ext4. Having to maintain three very similar code bases
> and test them all for correctness and performance is a real pain :)
>
A distribution can do that at any time, just by unconfiguring
CONFIG_EXT2 and/or CONFIG_EXT3 and configuring
CONFIG_EXT4_USE_FOR_EXT23. :-)
- Ted
On 12/10/2010 12:55 PM, Ted Ts'o wrote:
> On Wed, Dec 08, 2010 at 08:06:25PM -0500, Ric Wheeler wrote:
>
>> If we could get rid of ext2 (and eventually ext3), it would actually
>> help reduce the testing matrix and possibly let us invest even more
>> in testing ext4. Having to maintain three very similar code bases
>> and test them all for correctness and performance is a real pain :)
>>
> A distribution can do that at any time, just by unconfiguring
> CONFIG_EXT2 and/or CONFIG_EXT3 and configuring
> CONFIG_EXT4_USE_FOR_EXT23. :-)
>
> - Ted
Way too easy :)
What makes users stop using things is google searches that show up Ted saying
"We don't need ext2 any more, just use ext4 with no journal mode" ;)
Ric
On 2010-12-10, at 10:55, Ted Ts'o wrote:
> On Wed, Dec 08, 2010 at 08:06:25PM -0500, Ric Wheeler wrote:
>> If we could get rid of ext2 (and eventually ext3), it would actually
>> help reduce the testing matrix and possibly let us invest even more
>> in testing ext4. Having to maintain three very similar code bases
>> and test them all for correctness and performance is a real pain :)
>
> A distribution can do that at any time, just by unconfiguring
> CONFIG_EXT2 and/or CONFIG_EXT3 and configuring
> CONFIG_EXT4_USE_FOR_EXT23. :-)
That doesn't get rid of the maintenance efforts, however. There are a
constant stream of "patch for ext[234] needs to be ported to ext[342]".
Cheers, Andreas
On 12/10/10 5:40 PM, Andreas Dilger wrote:
> On 2010-12-10, at 10:55, Ted Ts'o wrote:
>> On Wed, Dec 08, 2010 at 08:06:25PM -0500, Ric Wheeler wrote:
>>> If we could get rid of ext2 (and eventually ext3), it would actually
>>> help reduce the testing matrix and possibly let us invest even more
>>> in testing ext4. Having to maintain three very similar code bases
>>> and test them all for correctness and performance is a real pain :)
>>
>> A distribution can do that at any time, just by unconfiguring
>> CONFIG_EXT2 and/or CONFIG_EXT3 and configuring
>> CONFIG_EXT4_USE_FOR_EXT23. :-)
>
> That doesn't get rid of the maintenance efforts, however. There are a
> constant stream of "patch for ext[234] needs to be ported to ext[342]".
Agreed....
our 3-way fork is getting cumbersome.
> Cheers, Andreas
On Tue 07-12-10 11:47:51, Eric Sandeen wrote:
> The addition of 64k block capability in the rec_len_from_disk
> and rec_len_to_disk functions added a bit of math overhead which
> slows down file create workloads needlessly when the architecture
> cannot even support 64k blocks, thanks to page size limits.
>
> The directory entry checking can also be optimized a bit
> by sprinkling in some unlikely() conditions to move the
> error handling out of line.
>
> These 2 patches speed up a bonnie++ file creation workload for
> me by several percent on ext2 & ext3.
Thanks. I've merged both patches into my tree.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR