All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] f2fs: fix to align to section for fallocate() on pinned file
@ 2021-03-05  9:56 ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2021-03-05  9:56 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

Now, fallocate() on a pinned file only allocates blocks which aligns
to segment rather than section, so GC may try to migrate pinned file's
block, and after several times of failure, pinned file's block could
be migrated to other place, however user won't be aware of such
condition, and then old obsolete block address may be readed/written
incorrectly.

To avoid such condition, let's try to allocate pinned file's blocks
with section alignment.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/f2fs.h    |  2 +-
 fs/f2fs/file.c    |  2 +-
 fs/f2fs/segment.c | 34 ++++++++++++++++++++++++++--------
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 700ef46e4147..ff6c212bf2c5 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3399,7 +3399,7 @@ void f2fs_get_new_segment(struct f2fs_sb_info *sbi,
 			unsigned int *newseg, bool new_sec, int dir);
 void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
 					unsigned int start, unsigned int end);
-void f2fs_allocate_new_segment(struct f2fs_sb_info *sbi, int type);
+void f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type);
 void f2fs_allocate_new_segments(struct f2fs_sb_info *sbi);
 int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range);
 bool f2fs_exist_trim_candidates(struct f2fs_sb_info *sbi,
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 1863944f4073..1b70b56434e4 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1666,7 +1666,7 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 		down_write(&sbi->pin_sem);
 
 		f2fs_lock_op(sbi);
-		f2fs_allocate_new_segment(sbi, CURSEG_COLD_DATA_PINNED);
+		f2fs_allocate_new_section(sbi, CURSEG_COLD_DATA_PINNED);
 		f2fs_unlock_op(sbi);
 
 		map.m_seg_type = CURSEG_COLD_DATA_PINNED;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index b26217910b85..ff50e79d2bb7 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2918,7 +2918,8 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
 	up_read(&SM_I(sbi)->curseg_lock);
 }
 
-static void __allocate_new_segment(struct f2fs_sb_info *sbi, int type)
+static void __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
+								bool new_sec)
 {
 	struct curseg_info *curseg = CURSEG_I(sbi, type);
 	unsigned int old_segno;
@@ -2926,10 +2927,22 @@ static void __allocate_new_segment(struct f2fs_sb_info *sbi, int type)
 	if (!curseg->inited)
 		goto alloc;
 
-	if (!curseg->next_blkoff &&
-		!get_valid_blocks(sbi, curseg->segno, false) &&
-		!get_ckpt_valid_blocks(sbi, curseg->segno))
-		return;
+	if (curseg->next_blkoff ||
+		get_valid_blocks(sbi, curseg->segno, new_sec))
+		goto alloc;
+
+	if (new_sec) {
+		unsigned int segno = START_SEGNO(curseg->segno);
+		int i;
+
+		for (i = 0; i < sbi->segs_per_sec; i++, segno++) {
+			if (get_ckpt_valid_blocks(sbi, segno))
+				goto alloc;
+		}
+	} else {
+		if (!get_ckpt_valid_blocks(sbi, curseg->segno))
+			return;
+	}
 
 alloc:
 	old_segno = curseg->segno;
@@ -2937,10 +2950,15 @@ static void __allocate_new_segment(struct f2fs_sb_info *sbi, int type)
 	locate_dirty_segment(sbi, old_segno);
 }
 
-void f2fs_allocate_new_segment(struct f2fs_sb_info *sbi, int type)
+static void __allocate_new_section(struct f2fs_sb_info *sbi, int type)
+{
+	__allocate_new_segment(sbi, type, true);
+}
+
+void f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type)
 {
 	down_write(&SIT_I(sbi)->sentry_lock);
-	__allocate_new_segment(sbi, type);
+	__allocate_new_section(sbi, type);
 	up_write(&SIT_I(sbi)->sentry_lock);
 }
 
@@ -2950,7 +2968,7 @@ void f2fs_allocate_new_segments(struct f2fs_sb_info *sbi)
 
 	down_write(&SIT_I(sbi)->sentry_lock);
 	for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++)
-		__allocate_new_segment(sbi, i);
+		__allocate_new_segment(sbi, i, false);
 	up_write(&SIT_I(sbi)->sentry_lock);
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [f2fs-dev] [PATCH] f2fs: fix to align to section for fallocate() on pinned file
@ 2021-03-05  9:56 ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2021-03-05  9:56 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

Now, fallocate() on a pinned file only allocates blocks which aligns
to segment rather than section, so GC may try to migrate pinned file's
block, and after several times of failure, pinned file's block could
be migrated to other place, however user won't be aware of such
condition, and then old obsolete block address may be readed/written
incorrectly.

To avoid such condition, let's try to allocate pinned file's blocks
with section alignment.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/f2fs.h    |  2 +-
 fs/f2fs/file.c    |  2 +-
 fs/f2fs/segment.c | 34 ++++++++++++++++++++++++++--------
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 700ef46e4147..ff6c212bf2c5 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3399,7 +3399,7 @@ void f2fs_get_new_segment(struct f2fs_sb_info *sbi,
 			unsigned int *newseg, bool new_sec, int dir);
 void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
 					unsigned int start, unsigned int end);
-void f2fs_allocate_new_segment(struct f2fs_sb_info *sbi, int type);
+void f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type);
 void f2fs_allocate_new_segments(struct f2fs_sb_info *sbi);
 int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range);
 bool f2fs_exist_trim_candidates(struct f2fs_sb_info *sbi,
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 1863944f4073..1b70b56434e4 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1666,7 +1666,7 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 		down_write(&sbi->pin_sem);
 
 		f2fs_lock_op(sbi);
-		f2fs_allocate_new_segment(sbi, CURSEG_COLD_DATA_PINNED);
+		f2fs_allocate_new_section(sbi, CURSEG_COLD_DATA_PINNED);
 		f2fs_unlock_op(sbi);
 
 		map.m_seg_type = CURSEG_COLD_DATA_PINNED;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index b26217910b85..ff50e79d2bb7 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2918,7 +2918,8 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
 	up_read(&SM_I(sbi)->curseg_lock);
 }
 
-static void __allocate_new_segment(struct f2fs_sb_info *sbi, int type)
+static void __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
+								bool new_sec)
 {
 	struct curseg_info *curseg = CURSEG_I(sbi, type);
 	unsigned int old_segno;
@@ -2926,10 +2927,22 @@ static void __allocate_new_segment(struct f2fs_sb_info *sbi, int type)
 	if (!curseg->inited)
 		goto alloc;
 
-	if (!curseg->next_blkoff &&
-		!get_valid_blocks(sbi, curseg->segno, false) &&
-		!get_ckpt_valid_blocks(sbi, curseg->segno))
-		return;
+	if (curseg->next_blkoff ||
+		get_valid_blocks(sbi, curseg->segno, new_sec))
+		goto alloc;
+
+	if (new_sec) {
+		unsigned int segno = START_SEGNO(curseg->segno);
+		int i;
+
+		for (i = 0; i < sbi->segs_per_sec; i++, segno++) {
+			if (get_ckpt_valid_blocks(sbi, segno))
+				goto alloc;
+		}
+	} else {
+		if (!get_ckpt_valid_blocks(sbi, curseg->segno))
+			return;
+	}
 
 alloc:
 	old_segno = curseg->segno;
@@ -2937,10 +2950,15 @@ static void __allocate_new_segment(struct f2fs_sb_info *sbi, int type)
 	locate_dirty_segment(sbi, old_segno);
 }
 
-void f2fs_allocate_new_segment(struct f2fs_sb_info *sbi, int type)
+static void __allocate_new_section(struct f2fs_sb_info *sbi, int type)
+{
+	__allocate_new_segment(sbi, type, true);
+}
+
+void f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type)
 {
 	down_write(&SIT_I(sbi)->sentry_lock);
-	__allocate_new_segment(sbi, type);
+	__allocate_new_section(sbi, type);
 	up_write(&SIT_I(sbi)->sentry_lock);
 }
 
@@ -2950,7 +2968,7 @@ void f2fs_allocate_new_segments(struct f2fs_sb_info *sbi)
 
 	down_write(&SIT_I(sbi)->sentry_lock);
 	for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++)
-		__allocate_new_segment(sbi, i);
+		__allocate_new_segment(sbi, i, false);
 	up_write(&SIT_I(sbi)->sentry_lock);
 }
 
-- 
2.29.2



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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] f2fs: fix to align to section for fallocate() on pinned file
  2021-03-05  9:56 ` [f2fs-dev] " Chao Yu
@ 2021-03-23 13:56   ` Chao Yu
  -1 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2021-03-23 13:56 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

On 2021/3/5 17:56, Chao Yu wrote:
> Now, fallocate() on a pinned file only allocates blocks which aligns
> to segment rather than section, so GC may try to migrate pinned file's
> block, and after several times of failure, pinned file's block could
> be migrated to other place, however user won't be aware of such
> condition, and then old obsolete block address may be readed/written
> incorrectly.
> 
> To avoid such condition, let's try to allocate pinned file's blocks
> with section alignment.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>

Jaegeuk,

Could you please check and apply below diff into original patch?

---
  fs/f2fs/file.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 236f3f69681a..24fa68fdcaa0 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1648,13 +1648,13 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
  		return 0;

  	if (f2fs_is_pinned_file(inode)) {
-		block_t len = (map.m_len >> sbi->log_blocks_per_seg) <<
-					sbi->log_blocks_per_seg;
+		block_t sec_blks = BLKS_PER_SEC(sbi);
+		block_t len = rounddown(map.m_len, sec_blks);

-		if (map.m_len % sbi->blocks_per_seg)
-			len += sbi->blocks_per_seg;
+		if (map.m_len % sec_blks)
+			len += sec_blks;

-		map.m_len = sbi->blocks_per_seg;
+		map.m_len = sec_blks;
  next_alloc:
  		if (has_not_enough_free_secs(sbi, 0,
  			GET_SEC_FROM_SEG(sbi, overprovision_segments(sbi)))) {
-- 
2.22.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: fix to align to section for fallocate() on pinned file
@ 2021-03-23 13:56   ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2021-03-23 13:56 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

On 2021/3/5 17:56, Chao Yu wrote:
> Now, fallocate() on a pinned file only allocates blocks which aligns
> to segment rather than section, so GC may try to migrate pinned file's
> block, and after several times of failure, pinned file's block could
> be migrated to other place, however user won't be aware of such
> condition, and then old obsolete block address may be readed/written
> incorrectly.
> 
> To avoid such condition, let's try to allocate pinned file's blocks
> with section alignment.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>

Jaegeuk,

Could you please check and apply below diff into original patch?

---
  fs/f2fs/file.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 236f3f69681a..24fa68fdcaa0 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1648,13 +1648,13 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
  		return 0;

  	if (f2fs_is_pinned_file(inode)) {
-		block_t len = (map.m_len >> sbi->log_blocks_per_seg) <<
-					sbi->log_blocks_per_seg;
+		block_t sec_blks = BLKS_PER_SEC(sbi);
+		block_t len = rounddown(map.m_len, sec_blks);

-		if (map.m_len % sbi->blocks_per_seg)
-			len += sbi->blocks_per_seg;
+		if (map.m_len % sec_blks)
+			len += sec_blks;

-		map.m_len = sbi->blocks_per_seg;
+		map.m_len = sec_blks;
  next_alloc:
  		if (has_not_enough_free_secs(sbi, 0,
  			GET_SEC_FROM_SEG(sbi, overprovision_segments(sbi)))) {
-- 
2.22.1




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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] f2fs: fix to align to section for fallocate() on pinned file
  2021-03-23 13:56   ` [f2fs-dev] " Chao Yu
@ 2021-03-23 18:32     ` Jaegeuk Kim
  -1 siblings, 0 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2021-03-23 18:32 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

On 03/23, Chao Yu wrote:
> On 2021/3/5 17:56, Chao Yu wrote:
> > Now, fallocate() on a pinned file only allocates blocks which aligns
> > to segment rather than section, so GC may try to migrate pinned file's
> > block, and after several times of failure, pinned file's block could
> > be migrated to other place, however user won't be aware of such
> > condition, and then old obsolete block address may be readed/written
> > incorrectly.
> > 
> > To avoid such condition, let's try to allocate pinned file's blocks
> > with section alignment.
> > 
> > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> 
> Jaegeuk,
> 
> Could you please check and apply below diff into original patch?
> 
> ---
>  fs/f2fs/file.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 236f3f69681a..24fa68fdcaa0 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1648,13 +1648,13 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
>  		return 0;
> 
>  	if (f2fs_is_pinned_file(inode)) {
> -		block_t len = (map.m_len >> sbi->log_blocks_per_seg) <<
> -					sbi->log_blocks_per_seg;
> +		block_t sec_blks = BLKS_PER_SEC(sbi);
> +		block_t len = rounddown(map.m_len, sec_blks);

len is declared above, so let me rephrase this as well.

> 
> -		if (map.m_len % sbi->blocks_per_seg)
> -			len += sbi->blocks_per_seg;
> +		if (map.m_len % sec_blks)
> +			len += sec_blks;

is this roundup()?

Could you check this?
https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=e1175f02291141bbd924fc578299305fcde35855

> 
> -		map.m_len = sbi->blocks_per_seg;
> +		map.m_len = sec_blks;
>  next_alloc:
>  		if (has_not_enough_free_secs(sbi, 0,
>  			GET_SEC_FROM_SEG(sbi, overprovision_segments(sbi)))) {
> -- 
> 2.22.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: fix to align to section for fallocate() on pinned file
@ 2021-03-23 18:32     ` Jaegeuk Kim
  0 siblings, 0 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2021-03-23 18:32 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel

On 03/23, Chao Yu wrote:
> On 2021/3/5 17:56, Chao Yu wrote:
> > Now, fallocate() on a pinned file only allocates blocks which aligns
> > to segment rather than section, so GC may try to migrate pinned file's
> > block, and after several times of failure, pinned file's block could
> > be migrated to other place, however user won't be aware of such
> > condition, and then old obsolete block address may be readed/written
> > incorrectly.
> > 
> > To avoid such condition, let's try to allocate pinned file's blocks
> > with section alignment.
> > 
> > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> 
> Jaegeuk,
> 
> Could you please check and apply below diff into original patch?
> 
> ---
>  fs/f2fs/file.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 236f3f69681a..24fa68fdcaa0 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1648,13 +1648,13 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
>  		return 0;
> 
>  	if (f2fs_is_pinned_file(inode)) {
> -		block_t len = (map.m_len >> sbi->log_blocks_per_seg) <<
> -					sbi->log_blocks_per_seg;
> +		block_t sec_blks = BLKS_PER_SEC(sbi);
> +		block_t len = rounddown(map.m_len, sec_blks);

len is declared above, so let me rephrase this as well.

> 
> -		if (map.m_len % sbi->blocks_per_seg)
> -			len += sbi->blocks_per_seg;
> +		if (map.m_len % sec_blks)
> +			len += sec_blks;

is this roundup()?

Could you check this?
https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=e1175f02291141bbd924fc578299305fcde35855

> 
> -		map.m_len = sbi->blocks_per_seg;
> +		map.m_len = sec_blks;
>  next_alloc:
>  		if (has_not_enough_free_secs(sbi, 0,
>  			GET_SEC_FROM_SEG(sbi, overprovision_segments(sbi)))) {
> -- 
> 2.22.1
> 


_______________________________________________
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] 8+ messages in thread
* Re: [PATCH] f2fs: fix to align to section for fallocate() on pinned file
  2021-03-23 18:32     ` [f2fs-dev] " Jaegeuk Kim
@ 2021-03-24  1:23       ` Chao Yu
  -1 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2021-03-24  1:23 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel, linux-kernel

On 2021/3/24 2:32, Jaegeuk Kim wrote:
> On 03/23, Chao Yu wrote:
>> On 2021/3/5 17:56, Chao Yu wrote:
>>> Now, fallocate() on a pinned file only allocates blocks which aligns
>>> to segment rather than section, so GC may try to migrate pinned file's
>>> block, and after several times of failure, pinned file's block could
>>> be migrated to other place, however user won't be aware of such
>>> condition, and then old obsolete block address may be readed/written
>>> incorrectly.
>>>
>>> To avoid such condition, let's try to allocate pinned file's blocks
>>> with section alignment.
>>>
>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>
>> Jaegeuk,
>>
>> Could you please check and apply below diff into original patch?
>>
>> ---
>>   fs/f2fs/file.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 236f3f69681a..24fa68fdcaa0 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -1648,13 +1648,13 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
>>   		return 0;
>>
>>   	if (f2fs_is_pinned_file(inode)) {
>> -		block_t len = (map.m_len >> sbi->log_blocks_per_seg) <<
>> -					sbi->log_blocks_per_seg;
>> +		block_t sec_blks = BLKS_PER_SEC(sbi);
>> +		block_t len = rounddown(map.m_len, sec_blks);
> 
> len is declared above, so let me rephrase this as well.

Oh, right.

> 
>>
>> -		if (map.m_len % sbi->blocks_per_seg)
>> -			len += sbi->blocks_per_seg;
>> +		if (map.m_len % sec_blks)
>> +			len += sec_blks;
> 
> is this roundup()?

More clean.

> 
> Could you check this?
> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=e1175f02291141bbd924fc578299305fcde35855

Looks good to me. :)

Thanks,

> 
>>
>> -		map.m_len = sbi->blocks_per_seg;
>> +		map.m_len = sec_blks;
>>   next_alloc:
>>   		if (has_not_enough_free_secs(sbi, 0,
>>   			GET_SEC_FROM_SEG(sbi, overprovision_segments(sbi)))) {
>> -- 
>> 2.22.1
>>
> .
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: fix to align to section for fallocate() on pinned file
@ 2021-03-24  1:23       ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2021-03-24  1:23 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel

On 2021/3/24 2:32, Jaegeuk Kim wrote:
> On 03/23, Chao Yu wrote:
>> On 2021/3/5 17:56, Chao Yu wrote:
>>> Now, fallocate() on a pinned file only allocates blocks which aligns
>>> to segment rather than section, so GC may try to migrate pinned file's
>>> block, and after several times of failure, pinned file's block could
>>> be migrated to other place, however user won't be aware of such
>>> condition, and then old obsolete block address may be readed/written
>>> incorrectly.
>>>
>>> To avoid such condition, let's try to allocate pinned file's blocks
>>> with section alignment.
>>>
>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>
>> Jaegeuk,
>>
>> Could you please check and apply below diff into original patch?
>>
>> ---
>>   fs/f2fs/file.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 236f3f69681a..24fa68fdcaa0 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -1648,13 +1648,13 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
>>   		return 0;
>>
>>   	if (f2fs_is_pinned_file(inode)) {
>> -		block_t len = (map.m_len >> sbi->log_blocks_per_seg) <<
>> -					sbi->log_blocks_per_seg;
>> +		block_t sec_blks = BLKS_PER_SEC(sbi);
>> +		block_t len = rounddown(map.m_len, sec_blks);
> 
> len is declared above, so let me rephrase this as well.

Oh, right.

> 
>>
>> -		if (map.m_len % sbi->blocks_per_seg)
>> -			len += sbi->blocks_per_seg;
>> +		if (map.m_len % sec_blks)
>> +			len += sec_blks;
> 
> is this roundup()?

More clean.

> 
> Could you check this?
> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=e1175f02291141bbd924fc578299305fcde35855

Looks good to me. :)

Thanks,

> 
>>
>> -		map.m_len = sbi->blocks_per_seg;
>> +		map.m_len = sec_blks;
>>   next_alloc:
>>   		if (has_not_enough_free_secs(sbi, 0,
>>   			GET_SEC_FROM_SEG(sbi, overprovision_segments(sbi)))) {
>> -- 
>> 2.22.1
>>
> .
> 


_______________________________________________
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] 8+ messages in thread
end of thread, other threads:[~2021-03-24  1:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-05  9:56 [PATCH] f2fs: fix to align to section for fallocate() on pinned file Chao Yu
2021-03-05  9:56 ` [f2fs-dev] " Chao Yu
2021-03-23 13:56 ` Chao Yu
2021-03-23 13:56   ` [f2fs-dev] " Chao Yu
2021-03-23 18:32   ` Jaegeuk Kim
2021-03-23 18:32     ` [f2fs-dev] " Jaegeuk Kim
2021-03-24  1:23     ` Chao Yu
2021-03-24  1:23       ` [f2fs-dev] " Chao Yu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.