linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] loop: change fsync to fdatasync when update dio
       [not found] <CGME20230126051713epcas1p10a9005ad21887893a486100cbbd376e5@epcas1p1.samsung.com>
@ 2023-01-26  5:16 ` Huijin Park
  2023-01-26  5:31   ` Christoph Hellwig
  0 siblings, 1 reply; 3+ messages in thread
From: Huijin Park @ 2023-01-26  5:16 UTC (permalink / raw)
  To: Jens Axboe, Christoph Hellwig, Chaitanya Kulkarni
  Cc: Ming Lei, linux-block, linux-kernel, huijin.park, bbanghj.park

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>

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
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] loop: change fsync to fdatasync when update dio
  2023-01-26  5:16 ` [PATCH] loop: change fsync to fdatasync when update dio Huijin Park
@ 2023-01-26  5:31   ` Christoph Hellwig
  2023-02-02 18:12     ` Huijin Park
  0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2023-01-26  5:31 UTC (permalink / raw)
  To: Huijin Park
  Cc: Jens Axboe, Christoph Hellwig, Chaitanya Kulkarni, linux-block,
	linux-kernel, bbanghj.park

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] loop: change fsync to fdatasync when update dio
  2023-01-26  5:31   ` Christoph Hellwig
@ 2023-02-02 18:12     ` Huijin Park
  0 siblings, 0 replies; 3+ messages in thread
From: Huijin Park @ 2023-02-02 18:12 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Huijin Park, Jens Axboe, Chaitanya Kulkarni, linux-block, linux-kernel

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-02-02 18:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20230126051713epcas1p10a9005ad21887893a486100cbbd376e5@epcas1p1.samsung.com>
2023-01-26  5:16 ` [PATCH] loop: change fsync to fdatasync when update dio Huijin Park
2023-01-26  5:31   ` Christoph Hellwig
2023-02-02 18:12     ` Huijin Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).