Btrfs: improve the performance fluctuating of the fsync

Commit Message

Miao Xie Dec. 18, 2013, 10:52 a.m. UTC

In order to improve the performance of fsync, we use the outstanding
ordered extents to avoid looking up the checksum from the csum tree.
But we didn't filter out the ordered extents whose csum is still being
calculated, when we got those ordered extents, we had to wait for the
csum calculation. It made the performance dropped down suddenly. (On
my box, it drop down from 56MB/s to 4-10MB/s)

But actually, the csum calculation of the ordered extents which were
introduced by the current fsync had already completed. Those ordered
extents whose csum was being calculated didn't belong to the current
fsync, we can ignore them.

By this patch, the performance fluctuating doesn't happen, and the average
performance grows up by ~2%.

Test Environment:
CPU:		2CPU * 2Cores
Memory:		4GB
Partition:	20GB(HDD)

Test Command:
 # sysbench --num-threads=8 --test=fileio --file-num=1 \
 > --file-total-size=8G --file-block-size=32768 \
 > --file-io-mode=sync --file-fsync-freq=100 \
 > --file-fsync-end=no --max-requests=10000 \
 > --file-test-mode=rndwr run

Signed-off-by: Miao Xie <[e-mail]>
---
 fs/btrfs/ordered-data.c | 3 +++
 fs/btrfs/tree-log.c     | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

Comments

Leonidas Spyropoulos Dec. 18, 2013, 11:09 a.m. UTC | #1

On Wed, Dec 18, 2013 at 06:52:44PM +0800, Miao Xie wrote:
> In order to improve the performance of fsync, we use the outstanding
> ordered extents to avoid looking up the checksum from the csum tree.
> But we didn't filter out the ordered extents whose csum is still being
> calculated, when we got those ordered extents, we had to wait for the
> csum calculation. It made the performance dropped down suddenly. (On
> my box, it drop down from 56MB/s to 4-10MB/s)
> 
> But actually, the csum calculation of the ordered extents which were
> introduced by the current fsync had already completed. Those ordered
> extents whose csum was being calculated didn't belong to the current
> fsync, we can ignore them.
> 
> By this patch, the performance fluctuating doesn't happen, and the average
> performance grows up by ~2%.
> [..] 

Will this help with apt-get performance over btrfs file system? As far as I understand it it's happening because of multiple fsync calls.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Josef Bacik Dec. 18, 2013, 9:38 p.m. UTC | #2

On 12/18/2013 05:52 AM, Miao Xie wrote:
> In order to improve the performance of fsync, we use the outstanding
> ordered extents to avoid looking up the checksum from the csum tree.
> But we didn't filter out the ordered extents whose csum is still being
> calculated, when we got those ordered extents, we had to wait for the
> csum calculation. It made the performance dropped down suddenly. (On
> my box, it drop down from 56MB/s to 4-10MB/s)
>
> But actually, the csum calculation of the ordered extents which were
> introduced by the current fsync had already completed. Those ordered
> extents whose csum was being calculated didn't belong to the current
> fsync, we can ignore them.

This isn't true because we will just start IO and carry on and wait 
later on, so we could very well have ordered extents that we started for 
this fsync without their csums ready which is why this code exists.  Thanks,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sander Jan. 2, 2014, 10:10 a.m. UTC | #3

Leonidas Spyropoulos wrote (ao):
> Will this help with apt-get performance over btrfs file system? As far
> as I understand it it's happening because of multiple fsync calls.

apt-get install eatmydata

"This package contains a small LD_PRELOAD library (libeatmydata) and a
couple of helper utilities designed to transparently disable fsync and
friends (like open(O_SYNC))."

Then use it like:
eatmydata apt-get install <package>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index b8c2ded..df87ed5 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -433,6 +433,9 @@  void btrfs_get_logged_extents(struct btrfs_root *log, struct inode *inode)
 	spin_lock_irq(&tree->lock);
 	for (n = rb_first(&tree->tree); n; n = rb_next(n)) {
 		ordered = rb_entry(n, struct btrfs_ordered_extent, rb_node);
+		if (ordered->csum_bytes_left)
+			continue;
+
 		spin_lock(&log->log_extents_lock[index]);
 		if (list_empty(&ordered->log_list)) {
 			list_add_tail(&ordered->log_list, &log->logged_list[index]);
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index ba2f151..3eae2eb 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -3631,8 +3631,6 @@  again:
 		 * start over after this.
 		 */
 
-		wait_event(ordered->wait, ordered->csum_bytes_left == 0);
-
 		list_for_each_entry(sum, &ordered->list, list) {
 			ret = btrfs_csum_file_blocks(trans, log, sum);
 			if (ret) {