Remove plugging at buffered write time

PATCH RESEND In review Fengguang Wu 2 participants 4 messages Aug 5, 2012 – Aug 9, 2012

Subsystems (3)

Memory management VFS Page cache

lore

Share & tools

lore

Fengguang Wu

author

2012-08-05 08:26 UTC #0 lore raw

cover Remove plugging at buffered write time

Hi all,

It seems this patch was silently forgotten, but the review comments have all been addressed: the patch has been split into two pieces and tests show no performance regressions (nor noticeable gains..).

Thanks to Damien for reminding me of the patch!

Thanks,
Fengguang

Fengguang Wu

author

2012-08-05 08:26 UTC #1 lore raw

PATCH 1/2 block: remove plugging at buffered write time

Buffered write(2) is not directly tied to IO, so it's not suitable to handle plug in generic_file_aio_write().

Note that plugging for O_SYNC writes is also removed. The user may pass arbitrary @size arguments, which may be much larger than the preferable I/O size, or may cross extent/device boundaries. Let the lower layers handle the plugging. The plugging code here actually turns them into no-ops.

CC Li Shaohua <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---
 mm/filemap.c |    3 ---
 1 file changed, 3 deletions(-)

mm/filemap.c +0 -3

@@ -2527,14 +2527,12 @@ ssize_t generic_file_aio_write(struct ki
 {
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = file->f_mapping->host;
-	struct blk_plug plug;
 	ssize_t ret;
 
 	BUG_ON(iocb->ki_pos != pos);
 
 	sb_start_write(inode->i_sb);
 	mutex_lock(&inode->i_mutex);
-	blk_start_plug(&plug);
 	ret = __generic_file_aio_write(iocb, iov, nr_segs, &iocb->ki_pos);
 	mutex_unlock(&inode->i_mutex);

@@ -2545,7 +2543,6 @@ ssize_t generic_file_aio_write(struct ki
 		if (err < 0 && ret > 0)
 			ret = err;
 	}
-	blk_finish_plug(&plug);
 	sb_end_write(inode->i_sb);
 	return ret;
 }

Fengguang Wu

author

2012-08-05 08:26 UTC #2 lore raw

PATCH 2/2 block: move down direct IO plugging

In reply to Fengguang Wu

Move unplugging for direct I/O from around ->direct_IO() down to do_blockdev_direct_IO(). This implicitly adds plugging for direct writes.

CC Li Shaohua <[email protected]>

Acked-by Jeff Moyer <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---
 fs/direct-io.c |    5 +++++
 mm/filemap.c   |    4 ----
 2 files changed, 5 insertions(+), 4 deletions(-)

mm/filemap.c +0 -4

@@ -1412,12 +1412,8 @@ generic_file_aio_read(struct kiocb *iocb
 			retval = filemap_write_and_wait_range(mapping, pos,
 					pos + iov_length(iov, nr_segs) - 1);
 			if (!retval) {
-				struct blk_plug plug;
-
-				blk_start_plug(&plug);
 				retval = mapping->a_ops->direct_IO(READ, iocb,
 							iov, pos, nr_segs);
-				blk_finish_plug(&plug);
 			}
 			if (retval > 0) {
 				*ppos = pos + retval;

fs/direct-io.c +5 -0

@@ -1062,6 +1062,7 @@ do_blockdev_direct_IO(int rw, struct kio
 	unsigned long user_addr;
 	size_t bytes;
 	struct buffer_head map_bh = { 0, };
+	struct blk_plug plug;
 
 	if (rw & WRITE)
 		rw = WRITE_ODIRECT;

@@ -1177,6 +1178,8 @@ do_blockdev_direct_IO(int rw, struct kio
 				PAGE_SIZE - user_addr / PAGE_SIZE);
 	}
 
+	blk_start_plug(&plug);
+
 	for (seg = 0; seg < nr_segs; seg++) {
 		user_addr = (unsigned long)iov[seg].iov_base;
 		sdio.size += bytes = iov[seg].iov_len;

@@ -1235,6 +1238,8 @@ do_blockdev_direct_IO(int rw, struct kio
 	if (sdio.bio)
 		dio_bio_submit(dio, &sdio);
 
+	blk_finish_plug(&plug);
+
 	/*
 	 * It is possible that, we return short IO due to end of file.
 	 * In that case, we need to release all the pages we got hold on.

Jens Axboe

2012-08-09 13:23 UTC #3 lore raw

In reply to Fengguang Wu

Quoted 7 lines · Fengguang Wu

Hi all,
It seems this patch was silently forgotten, but the review comments have all been addressed: the patch has been split into two pieces and tests show no performance regressions (nor noticeable gains..).
Thanks to Damien for reminding me of the patch!

Thanks Wu, applied.

Signature · 1 line

Jens Axboe