Linux-f2fs-devel Archive on lore.kernel.org
 help / color / Atom feed
* [f2fs-dev] [PATCH] f2fs: fix to force keeping write barrier for strict fsync mode
@ 2020-01-20 10:00 Chao Yu
  2020-01-23 22:18 ` Jaegeuk Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Chao Yu @ 2020-01-20 10:00 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

If barrier is enabled, for strict fsync mode, we should force to
use atomic write semantics to avoid data corruption due to no
barrier support in lower device.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/file.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 86ddbb55d2b1..c9dd45f82fbd 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -241,6 +241,13 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end,
 	};
 	unsigned int seq_id = 0;
 
+	/*
+	 * for strict fsync mode, force to keep atomic write sematics to avoid
+	 * data corruption if lower device doesn't support write barrier.
+	 */
+	if (!atomic && F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT)
+		atomic = true;
+
 	if (unlikely(f2fs_readonly(inode->i_sb) ||
 				is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
 		return 0;
-- 
2.18.0.rc1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH] f2fs: fix to force keeping write barrier for strict fsync mode
  2020-01-20 10:00 [f2fs-dev] [PATCH] f2fs: fix to force keeping write barrier for strict fsync mode Chao Yu
@ 2020-01-23 22:18 ` Jaegeuk Kim
  2020-02-05  1:39   ` Chao Yu
  0 siblings, 1 reply; 3+ messages in thread
From: Jaegeuk Kim @ 2020-01-23 22:18 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel

On 01/20, Chao Yu wrote:
> If barrier is enabled, for strict fsync mode, we should force to
> use atomic write semantics to avoid data corruption due to no
> barrier support in lower device.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/file.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 86ddbb55d2b1..c9dd45f82fbd 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -241,6 +241,13 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end,
>  	};
>  	unsigned int seq_id = 0;
>  
> +	/*
> +	 * for strict fsync mode, force to keep atomic write sematics to avoid
> +	 * data corruption if lower device doesn't support write barrier.
> +	 */
> +	if (!atomic && F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT)
> +		atomic = true;

This allows to relax IO ordering and cache flush. I'm not sure that's what you
want to do here for strict mode.

> +
>  	if (unlikely(f2fs_readonly(inode->i_sb) ||
>  				is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
>  		return 0;
> -- 
> 2.18.0.rc1


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH] f2fs: fix to force keeping write barrier for strict fsync mode
  2020-01-23 22:18 ` Jaegeuk Kim
@ 2020-02-05  1:39   ` Chao Yu
  0 siblings, 0 replies; 3+ messages in thread
From: Chao Yu @ 2020-02-05  1:39 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

On 2020/1/24 6:18, Jaegeuk Kim wrote:
> On 01/20, Chao Yu wrote:
>> If barrier is enabled, for strict fsync mode, we should force to
>> use atomic write semantics to avoid data corruption due to no
>> barrier support in lower device.
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  fs/f2fs/file.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 86ddbb55d2b1..c9dd45f82fbd 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -241,6 +241,13 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end,
>>  	};
>>  	unsigned int seq_id = 0;
>>  
>> +	/*
>> +	 * for strict fsync mode, force to keep atomic write sematics to avoid
>> +	 * data corruption if lower device doesn't support write barrier.
>> +	 */
>> +	if (!atomic && F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT)
>> +		atomic = true;
> 
> This allows to relax IO ordering and cache flush. I'm not sure that's what you
> want to do here for strict mode.

I intend to solve potential data corruption mentioned in below report:

https://www.mail-archive.com/linux-f2fs-devel@lists.sourceforge.net/msg15126.html

It occurs in this scenario:

- write page #0; persist
- overwrite page #0
- fsync
 - write data page #0 OPU into device's cache
 - write inode page into device's cache
 - issue flush

If SPO is triggered during flush command, inode page can be persisted before data
page #0, so that after recovery, inode page can be recovered with new physical block
address of data page #0, however there may contains dummy data in new physical block
address.

So what user see is after overwrite & fsync + SPO, old data in file was corrupted, if
any user do care about such case, we can enhance to avoid the corruption in strict mode
and suggest user to use fsync's strict mode.

Thoughts?

Thanks,

> 
>> +
>>  	if (unlikely(f2fs_readonly(inode->i_sb) ||
>>  				is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
>>  		return 0;
>> -- 
>> 2.18.0.rc1
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-20 10:00 [f2fs-dev] [PATCH] f2fs: fix to force keeping write barrier for strict fsync mode Chao Yu
2020-01-23 22:18 ` Jaegeuk Kim
2020-02-05  1:39   ` Chao Yu

Linux-f2fs-devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-f2fs-devel/0 linux-f2fs-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-f2fs-devel linux-f2fs-devel/ https://lore.kernel.org/linux-f2fs-devel \
		linux-f2fs-devel@lists.sourceforge.net
	public-inbox-index linux-f2fs-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/net.sourceforge.lists.linux-f2fs-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git