Skip to contentSkip to conversation

Btrfs: set plug for fsync

PATCH Superseded Liu Bo 2 participants 4 messages Nov 10, 2017 – Nov 20, 2017 Btrfs file system

lore
Share & tools
lore

Liu Bo

author
#0 lore raw

PATCH 1/1 Btrfs: set plug for fsync

Setting plug can merge adjacent IOs before dispatching IOs to the disk driver.

Without plug, it'd not be a problem for single disk usecases, but for multiple disks using raid profile, a large IO can be split to several IOs of stripe length, and plug can be helpful to bring them together for each disk so that we can save several disk access.

Moreover, fsync issues synchronous writes, so plug can really take effect.

---
 fs/btrfs/file.c | 3 +++
 1 file changed, 3 insertions(+)
fs/btrfs/file.c +3 -0
index e43da6c..063180b 100644
@@ -2018,10 +2018,13 @@ int btrfs_release_file(struct inode *inode, struct file *filp)
 static int start_ordered_ops(struct inode *inode, loff_t start, loff_t end)
 {
 	int ret;
+	struct blk_plug plug;
 
+	blk_start_plug(&plug);
 	atomic_inc(&BTRFS_I(inode)->sync_writers);
 	ret = btrfs_fdatawrite_range(inode, start, end);
 	atomic_dec(&BTRFS_I(inode)->sync_writers);
+	blk_finish_plug(&plug);
 
 	return ret;
 }
Signature · 1 line
2.9.4
#1 lore raw
Quoted 21 lines · Liu Bo

Setting plug can merge adjacent IOs before dispatching IOs to the disk driver.

Without plug, it'd not be a problem for single disk usecases, but for multiple disks using raid profile, a large IO can be split to several IOs of stripe length, and plug can be helpful to bring them together for each disk so that we can save several disk access.

Moreover, fsync issues synchronous writes, so plug can really take effect.

---
 fs/btrfs/file.c | 3 +++
 1 file changed, 3 insertions(+)
fs/btrfs/file.c +0 -0
index e43da6c..063180b 100644

fs/btrfs/file.c int btrfs_release_file(struct inode *inode, struct file *filp) · line 2023

Quoted 6 lines
fs/btrfs/file.c +2 -0
@@ -2018,10 +2018,13 @@ int btrfs_release_file(struct inode *inode, struct file *filp)
 static int start_ordered_ops(struct inode *inode, loff_t start, loff_t end)
 {
 	int ret;
+	struct blk_plug plug;
 
+	blk_start_plug(&plug);

Can you please add a comment here? Essentially repeating the commit message. The plug/unplug calls are never obvious so the expectations could be documented.

Quoted 14 lines
fs/btrfs/file.c +1 -0
 	atomic_inc(&BTRFS_I(inode)->sync_writers);
 	ret = btrfs_fdatawrite_range(inode, start, end);
 	atomic_dec(&BTRFS_I(inode)->sync_writers);
+	blk_finish_plug(&plug);
 
 	return ret;
 }
Signature · 6 lines
2.9.4

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#2 lore raw

PATCH v2 1/1 Btrfs: set plug for fsync

In reply to Liu Bo

Setting plug can merge adjacent IOs before dispatching IOs to the disk driver.

Without plug, it'd not be a problem for single disk usecases, but for multiple disks using raid profile, a large IO can be split to several IOs of stripe length, and plug can be helpful to bring them together for each disk so that we can save several disk access.

Moreover, fsync issues synchronous writes, so plug can really take effect.

---
v2: Explain why setting plug makes sense.

 fs/btrfs/file.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
fs/btrfs/file.c +10 -0
index e43da6c..504e96d 100644
@@ -2018,10 +2018,20 @@ int btrfs_release_file(struct inode *inode, struct file *filp)
 static int start_ordered_ops(struct inode *inode, loff_t start, loff_t end)
 {
 	int ret;
+	struct blk_plug plug;
 
+	/*
+	 * This is only called in fsync, which would do synchronous
+	 * writes, so a plug can merge adjacent IOs as much as
+	 * possible.  Esp. in case of multiple disks using raid
+	 * profile, a large IO can be split to several segments of
+	 * stripe length(64K).
+	 */
+	blk_start_plug(&plug);
 	atomic_inc(&BTRFS_I(inode)->sync_writers);
 	ret = btrfs_fdatawrite_range(inode, start, end);
 	atomic_dec(&BTRFS_I(inode)->sync_writers);
+	blk_finish_plug(&plug);
 
 	return ret;
 }
Signature · 1 line
2.9.4
Reviewed-by on v2 1/1
#3 lore raw
Quoted 8 lines · Liu Bo

Setting plug can merge adjacent IOs before dispatching IOs to the disk driver.

Without plug, it'd not be a problem for single disk usecases, but for multiple disks using raid profile, a large IO can be split to several IOs of stripe length, and plug can be helpful to bring them together for each disk so that we can save several disk access.

Quoted 4 lines

Moreover, fsync issues synchronous writes, so plug can really take effect.