| Message ID | 20230126051657.163497-1-huijin.park@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | loop: change fsync to fdatasync when update dio | expand |
On Thu, Jan 26, 2023 at 02:16:57PM +0900, Huijin Park wrote: > In general, fsync has a larger overhead than fdatasync. And since the > dio option is for data, it seems like fdatasync is enough. > So this patch changes it to fdatasync which has little load relatively. The only difference is that fsync also syncs the timestamps. So this change looks correct, but also a bit useless given that buffered to direct I/O or back changes aren't exactly a fast path.
On Thu, Jan 26, 2023 at 2:31 PM Christoph Hellwig <hch@lst.de> wrote: > > On Thu, Jan 26, 2023 at 02:16:57PM +0900, Huijin Park wrote: > > In general, fsync has a larger overhead than fdatasync. And since the > > dio option is for data, it seems like fdatasync is enough. > > So this patch changes it to fdatasync which has little load relatively. > > The only difference is that fsync also syncs the timestamps. So this > change looks correct, but also a bit useless given that buffered to > direct I/O or back changes aren't exactly a fast path. Although the difference will be minimal, why I suggested it is because it can reduce unnecessary metadata i/o (helpful on slow i/o devices), and using fdatasync looked correct like your opinion. In some environment cases, loop setup for mount is required when application is initialized and this change will help.
diff --git a/drivers/block/loop.c b/drivers/block/loop.c index 1518a6423279..6c7ce8be8c0c 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -203,7 +203,7 @@ static void __loop_update_dio(struct loop_device *lo, bool dio) return; /* flush dirty pages before changing direct IO */ - vfs_fsync(file, 0); + vfs_fsync(file, 1); /* * The flag of LO_FLAGS_DIRECT_IO is handled similarly with
In general, fsync has a larger overhead than fdatasync. And since the dio option is for data, it seems like fdatasync is enough. So this patch changes it to fdatasync which has little load relatively. Signed-off-by: Huijin Park <huijin.park@samsung.com>