Re: [f2fs-dev] [PATCH v2] f2fs: get victim segment again after new cp

2016-07-25 Thread Jaegeuk Kim 
On Sun, Jul 24, 2016 at 06:38:46AM +0800, Chao Yu wrote:
...

> >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >> index de6c41c..ec17096 100644
> >> --- a/fs/f2fs/gc.c
> >> +++ b/fs/f2fs/gc.c
> >> @@ -908,10 +908,14 @@ gc_more:
> >> * enough free sections, we should flush dent/node blocks and do
> >> * garbage collections.
> >> */
> >> -  if (__get_victim(sbi, , gc_type) || prefree_segments(sbi))
> >> +  if (__get_victim(sbi, , gc_type) || 
> >> prefree_segments(sbi)) {
> >>write_checkpoint(sbi, );
> >> -  else if (has_not_enough_free_secs(sbi, 0))
> > 
> > If segno is NULL_SEGNO, we get a panic when checking the below conditions.
> > Anyway, I don't think we need this condition at all.
> 
> I thinks the condition which trigger our f2fs_bug_on is really a corner case,
> we'd better not waste the victim we found everytime, especially for these
> victims from last background gc's victims.
> 
> What about checking segno first?
> 
> if(segno != NULL_SEGNO && (!get_valid_blocks(sbi, segno, sbi->segs_per_sec) ||
>   IS_CURSEC(sbi, GET_SECNO(sbi, segno)))
>   segno = NULL_SEGNO;

My viewpoint is write_checkpoint() flushes node and dentry pages, which provides
another better victims. Another concern is get_valid_blocks and IS_CURSEG are
not covered by any lock.

Thanks,

> 
> Thanks,
> 
> > Let me remove them and just set NULL_SEGNO only after checkpoint.
> > Please check the dev-test repo.
> > 
> > Thanks,
> > 
> >> +  if(!get_valid_blocks(sbi, segno, sbi->segs_per_sec)
> >> +  || IS_CURSEC(sbi, segno / 
> >> sbi->segs_per_sec))
> >> +  segno = NULL_SEGNO;
> >> +  } else if (has_not_enough_free_secs(sbi, 0)) {
> >>write_checkpoint(sbi, );
> >> +  }
> >>}
> >>  
> >>if (segno == NULL_SEGNO && !__get_victim(sbi, , gc_type))
> >> -- 
> >> 1.9.1
> > 
> > --
> > What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
> > patterns at an interface-level. Reveals which users, apps, and protocols 
> > are 
> > consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
> > J-Flow, sFlow and other flows. Make informed decisions using capacity 
> > planning
> > reports.http://sdm.link/zohodev2dev
> > ___
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > 

--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v2] f2fs: get victim segment again after new cp

2016-07-23 Thread Chao Yu 
Hi Jaegeuk,

On 2016/7/23 2:59, Jaegeuk Kim wrote:
> Hi Yunlei,
> 
> On Fri, Jul 22, 2016 at 07:08:31PM +0800, Yunlei He wrote:
>> Previous selected segment may become free after write_checkpoint,
>> if we do garbage collect on this segment, and then new_curseg happen
>> to reuse it, it may cause f2fs_bug_on as below.
>>
>>  panic+0x154/0x29c
>>  do_garbage_collect+0x15c/0xaf4
>>  f2fs_gc+0x2dc/0x444
>>  f2fs_balance_fs.part.22+0xcc/0x14c
>>  f2fs_balance_fs+0x28/0x34
>>  f2fs_map_blocks+0x5ec/0x790
>>  f2fs_preallocate_blocks+0xe0/0x100
>>  f2fs_file_write_iter+0x64/0x11c
>>  new_sync_write+0xac/0x11c
>>  vfs_write+0x144/0x1e4
>>  SyS_write+0x60/0xc0
>>
>> Here, maybe we check sit and ssa type during reset_curseg. So, we check
>> segment is stale or not, and select a new victim to avoid this.
>>
>> Signed-off-by: Yunlei He 
>> ---
>>  fs/f2fs/gc.c | 8 ++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index de6c41c..ec17096 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -908,10 +908,14 @@ gc_more:
>>   * enough free sections, we should flush dent/node blocks and do
>>   * garbage collections.
>>   */
>> -if (__get_victim(sbi, , gc_type) || prefree_segments(sbi))
>> +if (__get_victim(sbi, , gc_type) || 
>> prefree_segments(sbi)) {
>>  write_checkpoint(sbi, );
>> -else if (has_not_enough_free_secs(sbi, 0))
> 
> If segno is NULL_SEGNO, we get a panic when checking the below conditions.
> Anyway, I don't think we need this condition at all.

I thinks the condition which trigger our f2fs_bug_on is really a corner case,
we'd better not waste the victim we found everytime, especially for these
victims from last background gc's victims.

What about checking segno first?

if(segno != NULL_SEGNO && (!get_valid_blocks(sbi, segno, sbi->segs_per_sec) ||
IS_CURSEC(sbi, GET_SECNO(sbi, segno)))
segno = NULL_SEGNO;

Thanks,

> Let me remove them and just set NULL_SEGNO only after checkpoint.
> Please check the dev-test repo.
> 
> Thanks,
> 
>> +if(!get_valid_blocks(sbi, segno, sbi->segs_per_sec)
>> +|| IS_CURSEC(sbi, segno / 
>> sbi->segs_per_sec))
>> +segno = NULL_SEGNO;
>> +} else if (has_not_enough_free_secs(sbi, 0)) {
>>  write_checkpoint(sbi, );
>> +}
>>  }
>>  
>>  if (segno == NULL_SEGNO && !__get_victim(sbi, , gc_type))
>> -- 
>> 1.9.1
> 
> --
> What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
> patterns at an interface-level. Reveals which users, apps, and protocols are 
> consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
> J-Flow, sFlow and other flows. Make informed decisions using capacity planning
> reports.http://sdm.link/zohodev2dev
> ___
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 

--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v2] f2fs: get victim segment again after new cp

2016-07-22 Thread Jaegeuk Kim 
Hi Yunlei,

On Fri, Jul 22, 2016 at 07:08:31PM +0800, Yunlei He wrote:
> Previous selected segment may become free after write_checkpoint,
> if we do garbage collect on this segment, and then new_curseg happen
> to reuse it, it may cause f2fs_bug_on as below.
> 
>   panic+0x154/0x29c
>   do_garbage_collect+0x15c/0xaf4
>   f2fs_gc+0x2dc/0x444
>   f2fs_balance_fs.part.22+0xcc/0x14c
>   f2fs_balance_fs+0x28/0x34
>   f2fs_map_blocks+0x5ec/0x790
>   f2fs_preallocate_blocks+0xe0/0x100
>   f2fs_file_write_iter+0x64/0x11c
>   new_sync_write+0xac/0x11c
>   vfs_write+0x144/0x1e4
>   SyS_write+0x60/0xc0
> 
> Here, maybe we check sit and ssa type during reset_curseg. So, we check
> segment is stale or not, and select a new victim to avoid this.
> 
> Signed-off-by: Yunlei He 
> ---
>  fs/f2fs/gc.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index de6c41c..ec17096 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -908,10 +908,14 @@ gc_more:
>* enough free sections, we should flush dent/node blocks and do
>* garbage collections.
>*/
> - if (__get_victim(sbi, , gc_type) || prefree_segments(sbi))
> + if (__get_victim(sbi, , gc_type) || 
> prefree_segments(sbi)) {
>   write_checkpoint(sbi, );
> - else if (has_not_enough_free_secs(sbi, 0))

If segno is NULL_SEGNO, we get a panic when checking the below conditions.
Anyway, I don't think we need this condition at all.
Let me remove them and just set NULL_SEGNO only after checkpoint.
Please check the dev-test repo.

Thanks,

> + if(!get_valid_blocks(sbi, segno, sbi->segs_per_sec)
> + || IS_CURSEC(sbi, segno / 
> sbi->segs_per_sec))
> + segno = NULL_SEGNO;
> + } else if (has_not_enough_free_secs(sbi, 0)) {
>   write_checkpoint(sbi, );
> + }
>   }
>  
>   if (segno == NULL_SEGNO && !__get_victim(sbi, , gc_type))
> -- 
> 1.9.1

--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v2] f2fs: get victim segment again after new cp

2016-07-22 Thread Yunlei He 
Previous selected segment may become free after write_checkpoint,
if we do garbage collect on this segment, and then new_curseg happen
to reuse it, it may cause f2fs_bug_on as below.

panic+0x154/0x29c
do_garbage_collect+0x15c/0xaf4
f2fs_gc+0x2dc/0x444
f2fs_balance_fs.part.22+0xcc/0x14c
f2fs_balance_fs+0x28/0x34
f2fs_map_blocks+0x5ec/0x790
f2fs_preallocate_blocks+0xe0/0x100
f2fs_file_write_iter+0x64/0x11c
new_sync_write+0xac/0x11c
vfs_write+0x144/0x1e4
SyS_write+0x60/0xc0

Here, maybe we check sit and ssa type during reset_curseg. So, we check
segment is stale or not, and select a new victim to avoid this.

Signed-off-by: Yunlei He 
---
 fs/f2fs/gc.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index de6c41c..ec17096 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -908,10 +908,14 @@ gc_more:
 * enough free sections, we should flush dent/node blocks and do
 * garbage collections.
 */
-   if (__get_victim(sbi, , gc_type) || prefree_segments(sbi))
+   if (__get_victim(sbi, , gc_type) || 
prefree_segments(sbi)) {
write_checkpoint(sbi, );
-   else if (has_not_enough_free_secs(sbi, 0))
+   if(!get_valid_blocks(sbi, segno, sbi->segs_per_sec)
+   || IS_CURSEC(sbi, segno / 
sbi->segs_per_sec))
+   segno = NULL_SEGNO;
+   } else if (has_not_enough_free_secs(sbi, 0)) {
write_checkpoint(sbi, );
+   }
}
 
if (segno == NULL_SEGNO && !__get_victim(sbi, , gc_type))
-- 
1.9.1


--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel