All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] f2fs: check block reserve strictly a little bit
@ 2017-11-07  9:56 Yunlei He
  2017-11-09 17:55 ` Jaegeuk Kim
  0 siblings, 1 reply; 4+ messages in thread
From: Yunlei He @ 2017-11-07  9:56 UTC (permalink / raw)
  To: jaegeuk, yuchao0, linux-f2fs-devel; +Cc: ning.jia, heyunlei

This patch check new block reserve strictly, in case of
garbage data of node page.

Signed-off-by: Yunlei He <heyunlei@huawei.com>
---
 fs/f2fs/data.c    |  9 +++++++--
 fs/f2fs/file.c    | 13 +++++--------
 fs/f2fs/segment.c |  1 +
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index b0781ed..3017a87 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -558,13 +558,18 @@ int reserve_new_blocks(struct dnode_of_data *dn, blkcnt_t count)
 
 	f2fs_wait_on_page_writeback(dn->node_page, NODE, true);
 
-	for (; count > 0; dn->ofs_in_node++) {
+	for (; count > 0; dn->ofs_in_node++, count--) {
 		block_t blkaddr = datablock_addr(dn->inode,
 					dn->node_page, dn->ofs_in_node);
 		if (blkaddr == NULL_ADDR) {
 			dn->data_blkaddr = NEW_ADDR;
 			__set_data_blkaddr(dn);
-			count--;
+		} else {
+			f2fs_msg(sbi->sb, KERN_ERR, "reserve non NULL block, "
+				"blkaddr:%u, ino:%lu, nid:%u, ofs_in_node:%u",
+				blkaddr, dn->inode->i_ino, dn->nid,
+				dn->ofs_in_node);
+			f2fs_bug_on(sbi, 1);
 		}
 	}
 
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index cee0f36..dfc23c2 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1218,19 +1218,16 @@ static int f2fs_do_zero_range(struct dnode_of_data *dn, pgoff_t start,
 	struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
 	pgoff_t index = start;
 	unsigned int ofs_in_node = dn->ofs_in_node;
-	blkcnt_t count = 0;
-	int ret;
+	int ret = 0;
 
 	for (; index < end; index++, dn->ofs_in_node++) {
 		if (datablock_addr(dn->inode, dn->node_page,
 					dn->ofs_in_node) == NULL_ADDR)
-			count++;
-	}
+			ret = reserve_new_blocks(dn, 1);
 
-	dn->ofs_in_node = ofs_in_node;
-	ret = reserve_new_blocks(dn, count);
-	if (ret)
-		return ret;
+		if (ret)
+			return ret;
+	}
 
 	dn->ofs_in_node = ofs_in_node;
 	for (index = start; index < end; index++, dn->ofs_in_node++) {
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 20722b2..e670967 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1815,6 +1815,7 @@ static void update_sit_entry(struct f2fs_sb_info *sbi, block_t blkaddr, int del)
 	bool mir_exist;
 #endif
 
+	verify_block_addr(sbi, blkaddr);
 	segno = GET_SEGNO(sbi, blkaddr);
 
 	se = get_seg_entry(sbi, segno);
-- 
1.9.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] f2fs: check block reserve strictly a little bit
  2017-11-07  9:56 [PATCH] f2fs: check block reserve strictly a little bit Yunlei He
@ 2017-11-09 17:55 ` Jaegeuk Kim
  2017-11-10  2:30   ` heyunlei
  0 siblings, 1 reply; 4+ messages in thread
From: Jaegeuk Kim @ 2017-11-09 17:55 UTC (permalink / raw)
  To: Yunlei He; +Cc: ning.jia, linux-f2fs-devel

On 11/07, Yunlei He wrote:
> This patch check new block reserve strictly, in case of
> garbage data of node page.
> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
>  fs/f2fs/data.c    |  9 +++++++--
>  fs/f2fs/file.c    | 13 +++++--------
>  fs/f2fs/segment.c |  1 +
>  3 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index b0781ed..3017a87 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -558,13 +558,18 @@ int reserve_new_blocks(struct dnode_of_data *dn, blkcnt_t count)
>  
>  	f2fs_wait_on_page_writeback(dn->node_page, NODE, true);
>  
> -	for (; count > 0; dn->ofs_in_node++) {
> +	for (; count > 0; dn->ofs_in_node++, count--) {
>  		block_t blkaddr = datablock_addr(dn->inode,
>  					dn->node_page, dn->ofs_in_node);
>  		if (blkaddr == NULL_ADDR) {
>  			dn->data_blkaddr = NEW_ADDR;
>  			__set_data_blkaddr(dn);
> -			count--;
> +		} else {
> +			f2fs_msg(sbi->sb, KERN_ERR, "reserve non NULL block, "
> +				"blkaddr:%u, ino:%lu, nid:%u, ofs_in_node:%u",
> +				blkaddr, dn->inode->i_ino, dn->nid,
> +				dn->ofs_in_node);
> +			f2fs_bug_on(sbi, 1);

What if there is a hole?

>  		}
>  	}
>  
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index cee0f36..dfc23c2 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1218,19 +1218,16 @@ static int f2fs_do_zero_range(struct dnode_of_data *dn, pgoff_t start,
>  	struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
>  	pgoff_t index = start;
>  	unsigned int ofs_in_node = dn->ofs_in_node;
> -	blkcnt_t count = 0;
> -	int ret;
> +	int ret = 0;
>  
>  	for (; index < end; index++, dn->ofs_in_node++) {
>  		if (datablock_addr(dn->inode, dn->node_page,
>  					dn->ofs_in_node) == NULL_ADDR)
> -			count++;
> -	}
> +			ret = reserve_new_blocks(dn, 1);
>  
> -	dn->ofs_in_node = ofs_in_node;
> -	ret = reserve_new_blocks(dn, count);
> -	if (ret)
> -		return ret;
> +		if (ret)
> +			return ret;
> +	}
>  
>  	dn->ofs_in_node = ofs_in_node;
>  	for (index = start; index < end; index++, dn->ofs_in_node++) {
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 20722b2..e670967 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1815,6 +1815,7 @@ static void update_sit_entry(struct f2fs_sb_info *sbi, block_t blkaddr, int del)
>  	bool mir_exist;
>  #endif
>  
> +	verify_block_addr(sbi, blkaddr);
>  	segno = GET_SEGNO(sbi, blkaddr);
>  
>  	se = get_seg_entry(sbi, segno);
> -- 
> 1.9.1

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] f2fs: check block reserve strictly a little bit
  2017-11-09 17:55 ` Jaegeuk Kim
@ 2017-11-10  2:30   ` heyunlei
  2017-11-10 19:55     ` Jaegeuk Kim
  0 siblings, 1 reply; 4+ messages in thread
From: heyunlei @ 2017-11-10  2:30 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: Jianing (Euler), linux-f2fs-devel



>-----Original Message-----
>From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
>Sent: Friday, November 10, 2017 1:55 AM
>To: heyunlei
>Cc: Yuchao (T); linux-f2fs-devel@lists.sourceforge.net; Wangbintian; Jianing (Euler)
>Subject: Re: [f2fs-dev][PATCH] f2fs: check block reserve strictly a little bit
>
>On 11/07, Yunlei He wrote:
>> This patch check new block reserve strictly, in case of
>> garbage data of node page.
>>
>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>> ---
>>  fs/f2fs/data.c    |  9 +++++++--
>>  fs/f2fs/file.c    | 13 +++++--------
>>  fs/f2fs/segment.c |  1 +
>>  3 files changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index b0781ed..3017a87 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -558,13 +558,18 @@ int reserve_new_blocks(struct dnode_of_data *dn, blkcnt_t count)
>>
>>  	f2fs_wait_on_page_writeback(dn->node_page, NODE, true);
>>
>> -	for (; count > 0; dn->ofs_in_node++) {
>> +	for (; count > 0; dn->ofs_in_node++, count--) {
>>  		block_t blkaddr = datablock_addr(dn->inode,
>>  					dn->node_page, dn->ofs_in_node);
>>  		if (blkaddr == NULL_ADDR) {
>>  			dn->data_blkaddr = NEW_ADDR;
>>  			__set_data_blkaddr(dn);
>> -			count--;
>> +		} else {
>> +			f2fs_msg(sbi->sb, KERN_ERR, "reserve non NULL block, "
>> +				"blkaddr:%u, ino:%lu, nid:%u, ofs_in_node:%u",
>> +				blkaddr, dn->inode->i_ino, dn->nid,
>> +				dn->ofs_in_node);
>> +			f2fs_bug_on(sbi, 1);
>
>What if there is a hole?

Hole can hold a non NULL block index? 
NEW_ADDR may be possible, for the case date block is reserved but without wb
after new cp.
How about this?
	} else if (blkaddr != NEW_ADDR ){
		 	f2fs_msg();
	}
Thanks. 
>
>>  		}
>>  	}
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index cee0f36..dfc23c2 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -1218,19 +1218,16 @@ static int f2fs_do_zero_range(struct dnode_of_data *dn, pgoff_t start,
>>  	struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
>>  	pgoff_t index = start;
>>  	unsigned int ofs_in_node = dn->ofs_in_node;
>> -	blkcnt_t count = 0;
>> -	int ret;
>> +	int ret = 0;
>>
>>  	for (; index < end; index++, dn->ofs_in_node++) {
>>  		if (datablock_addr(dn->inode, dn->node_page,
>>  					dn->ofs_in_node) == NULL_ADDR)
>> -			count++;
>> -	}
>> +			ret = reserve_new_blocks(dn, 1);
>>
>> -	dn->ofs_in_node = ofs_in_node;
>> -	ret = reserve_new_blocks(dn, count);
>> -	if (ret)
>> -		return ret;
>> +		if (ret)
>> +			return ret;
>> +	}
>>
>>  	dn->ofs_in_node = ofs_in_node;
>>  	for (index = start; index < end; index++, dn->ofs_in_node++) {
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 20722b2..e670967 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -1815,6 +1815,7 @@ static void update_sit_entry(struct f2fs_sb_info *sbi, block_t blkaddr, int del)
>>  	bool mir_exist;
>>  #endif
>>
>> +	verify_block_addr(sbi, blkaddr);
>>  	segno = GET_SEGNO(sbi, blkaddr);
>>
>>  	se = get_seg_entry(sbi, segno);
>> --
>> 1.9.1

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] f2fs: check block reserve strictly a little bit
  2017-11-10  2:30   ` heyunlei
@ 2017-11-10 19:55     ` Jaegeuk Kim
  0 siblings, 0 replies; 4+ messages in thread
From: Jaegeuk Kim @ 2017-11-10 19:55 UTC (permalink / raw)
  To: heyunlei; +Cc: Jianing (Euler), linux-f2fs-devel

On 11/10, heyunlei wrote:
> 
> 
> >-----Original Message-----
> >From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> >Sent: Friday, November 10, 2017 1:55 AM
> >To: heyunlei
> >Cc: Yuchao (T); linux-f2fs-devel@lists.sourceforge.net; Wangbintian; Jianing (Euler)
> >Subject: Re: [f2fs-dev][PATCH] f2fs: check block reserve strictly a little bit
> >
> >On 11/07, Yunlei He wrote:
> >> This patch check new block reserve strictly, in case of
> >> garbage data of node page.
> >>
> >> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> >> ---
> >>  fs/f2fs/data.c    |  9 +++++++--
> >>  fs/f2fs/file.c    | 13 +++++--------
> >>  fs/f2fs/segment.c |  1 +
> >>  3 files changed, 13 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> >> index b0781ed..3017a87 100644
> >> --- a/fs/f2fs/data.c
> >> +++ b/fs/f2fs/data.c
> >> @@ -558,13 +558,18 @@ int reserve_new_blocks(struct dnode_of_data *dn, blkcnt_t count)
> >>
> >>  	f2fs_wait_on_page_writeback(dn->node_page, NODE, true);
> >>
> >> -	for (; count > 0; dn->ofs_in_node++) {
> >> +	for (; count > 0; dn->ofs_in_node++, count--) {
> >>  		block_t blkaddr = datablock_addr(dn->inode,
> >>  					dn->node_page, dn->ofs_in_node);
> >>  		if (blkaddr == NULL_ADDR) {
> >>  			dn->data_blkaddr = NEW_ADDR;
> >>  			__set_data_blkaddr(dn);
> >> -			count--;
> >> +		} else {
> >> +			f2fs_msg(sbi->sb, KERN_ERR, "reserve non NULL block, "
> >> +				"blkaddr:%u, ino:%lu, nid:%u, ofs_in_node:%u",
> >> +				blkaddr, dn->inode->i_ino, dn->nid,
> >> +				dn->ofs_in_node);
> >> +			f2fs_bug_on(sbi, 1);
> >
> >What if there is a hole?
> 
> Hole can hold a non NULL block index? 
> NEW_ADDR may be possible, for the case date block is reserved but without wb
> after new cp.
> How about this?
> 	} else if (blkaddr != NEW_ADDR ){
> 		 	f2fs_msg();
> 	}

No, it is allowed to allocate new blocks in holes. We can give the # of
allocated blocks to caller which can check its intention.

> Thanks. 
> >
> >>  		}
> >>  	}
> >>
> >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> >> index cee0f36..dfc23c2 100644
> >> --- a/fs/f2fs/file.c
> >> +++ b/fs/f2fs/file.c
> >> @@ -1218,19 +1218,16 @@ static int f2fs_do_zero_range(struct dnode_of_data *dn, pgoff_t start,
> >>  	struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
> >>  	pgoff_t index = start;
> >>  	unsigned int ofs_in_node = dn->ofs_in_node;
> >> -	blkcnt_t count = 0;
> >> -	int ret;
> >> +	int ret = 0;
> >>
> >>  	for (; index < end; index++, dn->ofs_in_node++) {
> >>  		if (datablock_addr(dn->inode, dn->node_page,
> >>  					dn->ofs_in_node) == NULL_ADDR)
> >> -			count++;
> >> -	}
> >> +			ret = reserve_new_blocks(dn, 1);
> >>
> >> -	dn->ofs_in_node = ofs_in_node;
> >> -	ret = reserve_new_blocks(dn, count);
> >> -	if (ret)
> >> -		return ret;
> >> +		if (ret)
> >> +			return ret;
> >> +	}
> >>
> >>  	dn->ofs_in_node = ofs_in_node;
> >>  	for (index = start; index < end; index++, dn->ofs_in_node++) {
> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >> index 20722b2..e670967 100644
> >> --- a/fs/f2fs/segment.c
> >> +++ b/fs/f2fs/segment.c
> >> @@ -1815,6 +1815,7 @@ static void update_sit_entry(struct f2fs_sb_info *sbi, block_t blkaddr, int del)
> >>  	bool mir_exist;
> >>  #endif
> >>
> >> +	verify_block_addr(sbi, blkaddr);
> >>  	segno = GET_SEGNO(sbi, blkaddr);
> >>
> >>  	se = get_seg_entry(sbi, segno);
> >> --
> >> 1.9.1

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-10 19:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-07  9:56 [PATCH] f2fs: check block reserve strictly a little bit Yunlei He
2017-11-09 17:55 ` Jaegeuk Kim
2017-11-10  2:30   ` heyunlei
2017-11-10 19:55     ` Jaegeuk Kim

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.