| Message ID | 20231030094024.263707-1-bo.wu@vivo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [f2fs-dev,1/1] f2fs: fix fallocate failed under pinned block situation | expand |
On 2023/10/30 17:40, Wu Bo wrote: > If GC victim has pinned block, it can't be recycled. > And if GC is foreground running, after many failure try, the pinned file > is expected to be clear pin flag. To enable the section be recycled. > > But when fallocate trigger FG_GC, GC can never recycle the pinned > section. Because GC will go to stop before the failure try meet the threshold: > if (has_enough_free_secs(sbi, sec_freed, 0)) { > if (!gc_control->no_bg_gc && > total_sec_freed < gc_control->nr_free_secs) > goto go_gc_more; > goto stop; > } > > So when fallocate trigger FG_GC, at least recycle one. Hmm... it may break pinfile's semantics at least on one pinned file? In this case, I prefer to fail fallocate() rather than unpinning file, in order to avoid leaving invalid LBA references of unpinned file held by userspace. Thoughts? Thanks, > > This issue can be reproduced by filling f2fs space as following layout. > Every segment has one block is pinned: > +-+-+-+-+-+-+-----+-+ > | | |p| | | | ... | | seg_n > +-+-+-+-+-+-+-----+-+ > +-+-+-+-+-+-+-----+-+ > | | |p| | | | ... | | seg_n+1 > +-+-+-+-+-+-+-----+-+ > ... > +-+-+-+-+-+-+-----+-+ > | | |p| | | | ... | | seg_n+k > +-+-+-+-+-+-+-----+-+ > > And following are steps to reproduce this issue: > dd if=/dev/zero of=./f2fs_pin.img bs=2M count=1024 > mkfs.f2fs f2fs_pin.img > mkdir f2fs > mount f2fs_pin.img ./f2fs > cd f2fs > dd if=/dev/zero of=./large_padding bs=1M count=1760 > ./pin_filling.sh > rm padding* > sync > touch fallocate_40m > f2fs_io pinfile set fallocate_40m > fallocate -l 41943040 fallocate_40m > > fallocate always fail with EAGAIN even there has enough free space. > > 'pin_filling.sh' is: > count=1 > while : > do > # filling the seg space > for i in {1..511}: > do > name=padding_$count-$i > echo write $name > dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 > if [ $? -ne 0 ]; then > exit 0 > fi > done > sync > > # pin one block in a segment > name=pin_file$count > dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 > sync > f2fs_io pinfile set $name > count=$(($count + 1)) > done > > Signed-off-by: Wu Bo <bo.wu@vivo.com> > --- > fs/f2fs/file.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index ca5904129b16..e8a13616543f 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -1690,7 +1690,7 @@ static int f2fs_expand_inode_data(struct inode *inode, loff_t offset, > .init_gc_type = FG_GC, > .should_migrate_blocks = false, > .err_gc_skipped = true, > - .nr_free_secs = 0 }; > + .nr_free_secs = 1 }; > pgoff_t pg_start, pg_end; > loff_t new_size; > loff_t off_end;
On 2023/11/7 22:39, Chao Yu wrote: > On 2023/10/30 17:40, Wu Bo wrote: >> If GC victim has pinned block, it can't be recycled. >> And if GC is foreground running, after many failure try, the pinned file >> is expected to be clear pin flag. To enable the section be recycled. >> >> But when fallocate trigger FG_GC, GC can never recycle the pinned >> section. Because GC will go to stop before the failure try meet the >> threshold: >> if (has_enough_free_secs(sbi, sec_freed, 0)) { >> if (!gc_control->no_bg_gc && >> total_sec_freed < gc_control->nr_free_secs) >> goto go_gc_more; >> goto stop; >> } >> >> So when fallocate trigger FG_GC, at least recycle one. > > Hmm... it may break pinfile's semantics at least on one pinned file? > In this case, I prefer to fail fallocate() rather than unpinning file, > in order to avoid leaving invalid LBA references of unpinned file held > by userspace. As f2fs designed now, FG_GC is able to unpin the pinned file. fallocate() triggered FG_GC, but can't recycle space. It breaks the design logic of FG_GC. This issue is happened in Android OTA scenario. fallocate() always return failure cause OTA fail. And this commit changed previous behavior of fallocate(): Commit 2e42b7f817ac ("f2fs: stop allocating pinned sections if EAGAIN happens") Before this commit, if fallocate() meet this situation, it will trigger FG_GC to recycle pinned space finally. FG_GC is expected to recycle pinned space when there is no more free space. And this is the right time to do it when fallocate() need free space. It is weird when f2fs shows enough spare space but can't fallocate(). So I think it should be fixed. > > Thoughts? > > Thanks, > >> >> This issue can be reproduced by filling f2fs space as following layout. >> Every segment has one block is pinned: >> +-+-+-+-+-+-+-----+-+ >> | | |p| | | | ... | | seg_n >> +-+-+-+-+-+-+-----+-+ >> +-+-+-+-+-+-+-----+-+ >> | | |p| | | | ... | | seg_n+1 >> +-+-+-+-+-+-+-----+-+ >> ... >> +-+-+-+-+-+-+-----+-+ >> | | |p| | | | ... | | seg_n+k >> +-+-+-+-+-+-+-----+-+ >> >> And following are steps to reproduce this issue: >> dd if=/dev/zero of=./f2fs_pin.img bs=2M count=1024 >> mkfs.f2fs f2fs_pin.img >> mkdir f2fs >> mount f2fs_pin.img ./f2fs >> cd f2fs >> dd if=/dev/zero of=./large_padding bs=1M count=1760 >> ./pin_filling.sh >> rm padding* >> sync >> touch fallocate_40m >> f2fs_io pinfile set fallocate_40m >> fallocate -l 41943040 fallocate_40m >> >> fallocate always fail with EAGAIN even there has enough free space. >> >> 'pin_filling.sh' is: >> count=1 >> while : >> do >> # filling the seg space >> for i in {1..511}: >> do >> name=padding_$count-$i >> echo write $name >> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >> if [ $? -ne 0 ]; then >> exit 0 >> fi >> done >> sync >> >> # pin one block in a segment >> name=pin_file$count >> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >> sync >> f2fs_io pinfile set $name >> count=$(($count + 1)) >> done >> >> Signed-off-by: Wu Bo <bo.wu@vivo.com> >> --- >> fs/f2fs/file.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >> index ca5904129b16..e8a13616543f 100644 >> --- a/fs/f2fs/file.c >> +++ b/fs/f2fs/file.c >> @@ -1690,7 +1690,7 @@ static int f2fs_expand_inode_data(struct inode >> *inode, loff_t offset, >> .init_gc_type = FG_GC, >> .should_migrate_blocks = false, >> .err_gc_skipped = true, >> - .nr_free_secs = 0 }; >> + .nr_free_secs = 1 }; >> pgoff_t pg_start, pg_end; >> loff_t new_size; >> loff_t off_end;
On 2023/11/8 21:48, Wu Bo wrote: > On 2023/11/7 22:39, Chao Yu wrote: >> On 2023/10/30 17:40, Wu Bo wrote: >>> If GC victim has pinned block, it can't be recycled. >>> And if GC is foreground running, after many failure try, the pinned file >>> is expected to be clear pin flag. To enable the section be recycled. >>> >>> But when fallocate trigger FG_GC, GC can never recycle the pinned >>> section. Because GC will go to stop before the failure try meet the >>> threshold: >>> if (has_enough_free_secs(sbi, sec_freed, 0)) { >>> if (!gc_control->no_bg_gc && >>> total_sec_freed < gc_control->nr_free_secs) >>> goto go_gc_more; >>> goto stop; >>> } >>> >>> So when fallocate trigger FG_GC, at least recycle one. >> >> Hmm... it may break pinfile's semantics at least on one pinned file? >> In this case, I prefer to fail fallocate() rather than unpinning file, >> in order to avoid leaving invalid LBA references of unpinned file held >> by userspace. > > As f2fs designed now, FG_GC is able to unpin the pinned file. > > fallocate() triggered FG_GC, but can't recycle space. It breaks the > design logic of FG_GC. Yes, contradictoriness exists. IMO, unpin file by GC looks more dangerous, it may cause potential data corruption w/ below case: 1. app pins file & holds LBAs of data blocks. 2. GC unpins file and migrates its data to new LBAs. 3. other file reuses previous LBAs. 4. app read/write data via previous LBAs. So I suggest to normalize use of pinfile and do not add more unpin cases in filesystem inner processes. > > This issue is happened in Android OTA scenario. fallocate() always > return failure cause OTA fail. Can you please check why other pinned files were so fragmented that f2fs_gc() can not recycle one free section? Thanks, > > And this commit changed previous behavior of fallocate(): > > Commit 2e42b7f817ac ("f2fs: stop allocating pinned sections if EAGAIN > happens") > > Before this commit, if fallocate() meet this situation, it will trigger > FG_GC to recycle pinned space finally. > > FG_GC is expected to recycle pinned space when there is no more free > space. And this is the right time to do it when fallocate() need free > space. > > It is weird when f2fs shows enough spare space but can't fallocate(). So > I think it should be fixed. > >> >> Thoughts? >> >> Thanks, >> >>> >>> This issue can be reproduced by filling f2fs space as following layout. >>> Every segment has one block is pinned: >>> +-+-+-+-+-+-+-----+-+ >>> | | |p| | | | ... | | seg_n >>> +-+-+-+-+-+-+-----+-+ >>> +-+-+-+-+-+-+-----+-+ >>> | | |p| | | | ... | | seg_n+1 >>> +-+-+-+-+-+-+-----+-+ >>> ... >>> +-+-+-+-+-+-+-----+-+ >>> | | |p| | | | ... | | seg_n+k >>> +-+-+-+-+-+-+-----+-+ >>> >>> And following are steps to reproduce this issue: >>> dd if=/dev/zero of=./f2fs_pin.img bs=2M count=1024 >>> mkfs.f2fs f2fs_pin.img >>> mkdir f2fs >>> mount f2fs_pin.img ./f2fs >>> cd f2fs >>> dd if=/dev/zero of=./large_padding bs=1M count=1760 >>> ./pin_filling.sh >>> rm padding* >>> sync >>> touch fallocate_40m >>> f2fs_io pinfile set fallocate_40m >>> fallocate -l 41943040 fallocate_40m >>> >>> fallocate always fail with EAGAIN even there has enough free space. >>> >>> 'pin_filling.sh' is: >>> count=1 >>> while : >>> do >>> # filling the seg space >>> for i in {1..511}: >>> do >>> name=padding_$count-$i >>> echo write $name >>> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >>> if [ $? -ne 0 ]; then >>> exit 0 >>> fi >>> done >>> sync >>> >>> # pin one block in a segment >>> name=pin_file$count >>> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >>> sync >>> f2fs_io pinfile set $name >>> count=$(($count + 1)) >>> done >>> >>> Signed-off-by: Wu Bo <bo.wu@vivo.com> >>> --- >>> fs/f2fs/file.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>> index ca5904129b16..e8a13616543f 100644 >>> --- a/fs/f2fs/file.c >>> +++ b/fs/f2fs/file.c >>> @@ -1690,7 +1690,7 @@ static int f2fs_expand_inode_data(struct inode >>> *inode, loff_t offset, >>> .init_gc_type = FG_GC, >>> .should_migrate_blocks = false, >>> .err_gc_skipped = true, >>> - .nr_free_secs = 0 }; >>> + .nr_free_secs = 1 }; >>> pgoff_t pg_start, pg_end; >>> loff_t new_size; >>> loff_t off_end;
On 2023/11/11 12:49, Chao Yu wrote: > On 2023/11/8 21:48, Wu Bo wrote: >> On 2023/11/7 22:39, Chao Yu wrote: >>> On 2023/10/30 17:40, Wu Bo wrote: >>>> If GC victim has pinned block, it can't be recycled. >>>> And if GC is foreground running, after many failure try, the pinned >>>> file >>>> is expected to be clear pin flag. To enable the section be recycled. >>>> >>>> But when fallocate trigger FG_GC, GC can never recycle the pinned >>>> section. Because GC will go to stop before the failure try meet the >>>> threshold: >>>> if (has_enough_free_secs(sbi, sec_freed, 0)) { >>>> if (!gc_control->no_bg_gc && >>>> total_sec_freed < gc_control->nr_free_secs) >>>> goto go_gc_more; >>>> goto stop; >>>> } >>>> >>>> So when fallocate trigger FG_GC, at least recycle one. >>> >>> Hmm... it may break pinfile's semantics at least on one pinned file? >>> In this case, I prefer to fail fallocate() rather than unpinning file, >>> in order to avoid leaving invalid LBA references of unpinned file held >>> by userspace. >> >> As f2fs designed now, FG_GC is able to unpin the pinned file. >> >> fallocate() triggered FG_GC, but can't recycle space. It breaks the >> design logic of FG_GC. > > Yes, contradictoriness exists. > > IMO, unpin file by GC looks more dangerous, it may cause potential data > corruption w/ below case: > 1. app pins file & holds LBAs of data blocks. > 2. GC unpins file and migrates its data to new LBAs. > 3. other file reuses previous LBAs. > 4. app read/write data via previous LBAs. > > So I suggest to normalize use of pinfile and do not add more unpin cases > in filesystem inner processes. > >> >> This issue is happened in Android OTA scenario. fallocate() always >> return failure cause OTA fail. > > Can you please check why other pinned files were so fragmented that > f2fs_gc() > can not recycle one free section? > Not because pinned files were fragmented, but if the GC victim section has one block is pinned will cause this issue. If the section don't unpin the block, it can't be recycled. But there is high chance that the pinned section will be chosen next time under f2fs current victim selection strategy. So if we want to avoid unpin files, I think change victim selection to considering pinned blocks can fix this issue. > Thanks, > >> >> And this commit changed previous behavior of fallocate(): >> >> Commit 2e42b7f817ac ("f2fs: stop allocating pinned sections if EAGAIN >> happens") >> >> Before this commit, if fallocate() meet this situation, it will trigger >> FG_GC to recycle pinned space finally. >> >> FG_GC is expected to recycle pinned space when there is no more free >> space. And this is the right time to do it when fallocate() need free >> space. >> >> It is weird when f2fs shows enough spare space but can't fallocate(). So >> I think it should be fixed. >> >>> >>> Thoughts? >>> >>> Thanks, >>> >>>> >>>> This issue can be reproduced by filling f2fs space as following >>>> layout. >>>> Every segment has one block is pinned: >>>> +-+-+-+-+-+-+-----+-+ >>>> | | |p| | | | ... | | seg_n >>>> +-+-+-+-+-+-+-----+-+ >>>> +-+-+-+-+-+-+-----+-+ >>>> | | |p| | | | ... | | seg_n+1 >>>> +-+-+-+-+-+-+-----+-+ >>>> ... >>>> +-+-+-+-+-+-+-----+-+ >>>> | | |p| | | | ... | | seg_n+k >>>> +-+-+-+-+-+-+-----+-+ >>>> >>>> And following are steps to reproduce this issue: >>>> dd if=/dev/zero of=./f2fs_pin.img bs=2M count=1024 >>>> mkfs.f2fs f2fs_pin.img >>>> mkdir f2fs >>>> mount f2fs_pin.img ./f2fs >>>> cd f2fs >>>> dd if=/dev/zero of=./large_padding bs=1M count=1760 >>>> ./pin_filling.sh >>>> rm padding* >>>> sync >>>> touch fallocate_40m >>>> f2fs_io pinfile set fallocate_40m >>>> fallocate -l 41943040 fallocate_40m >>>> >>>> fallocate always fail with EAGAIN even there has enough free space. >>>> >>>> 'pin_filling.sh' is: >>>> count=1 >>>> while : >>>> do >>>> # filling the seg space >>>> for i in {1..511}: >>>> do >>>> name=padding_$count-$i >>>> echo write $name >>>> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >>>> if [ $? -ne 0 ]; then >>>> exit 0 >>>> fi >>>> done >>>> sync >>>> >>>> # pin one block in a segment >>>> name=pin_file$count >>>> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >>>> sync >>>> f2fs_io pinfile set $name >>>> count=$(($count + 1)) >>>> done >>>> >>>> Signed-off-by: Wu Bo <bo.wu@vivo.com> >>>> --- >>>> fs/f2fs/file.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>>> index ca5904129b16..e8a13616543f 100644 >>>> --- a/fs/f2fs/file.c >>>> +++ b/fs/f2fs/file.c >>>> @@ -1690,7 +1690,7 @@ static int f2fs_expand_inode_data(struct inode >>>> *inode, loff_t offset, >>>> .init_gc_type = FG_GC, >>>> .should_migrate_blocks = false, >>>> .err_gc_skipped = true, >>>> - .nr_free_secs = 0 }; >>>> + .nr_free_secs = 1 }; >>>> pgoff_t pg_start, pg_end; >>>> loff_t new_size; >>>> loff_t off_end;
On 2023/11/17 7:34, Wu Bo wrote: > On 2023/11/11 12:49, Chao Yu wrote: >> On 2023/11/8 21:48, Wu Bo wrote: >>> On 2023/11/7 22:39, Chao Yu wrote: >>>> On 2023/10/30 17:40, Wu Bo wrote: >>>>> If GC victim has pinned block, it can't be recycled. >>>>> And if GC is foreground running, after many failure try, the pinned file >>>>> is expected to be clear pin flag. To enable the section be recycled. >>>>> >>>>> But when fallocate trigger FG_GC, GC can never recycle the pinned >>>>> section. Because GC will go to stop before the failure try meet the >>>>> threshold: >>>>> if (has_enough_free_secs(sbi, sec_freed, 0)) { >>>>> if (!gc_control->no_bg_gc && >>>>> total_sec_freed < gc_control->nr_free_secs) >>>>> goto go_gc_more; >>>>> goto stop; >>>>> } >>>>> >>>>> So when fallocate trigger FG_GC, at least recycle one. >>>> >>>> Hmm... it may break pinfile's semantics at least on one pinned file? >>>> In this case, I prefer to fail fallocate() rather than unpinning file, >>>> in order to avoid leaving invalid LBA references of unpinned file held >>>> by userspace. >>> >>> As f2fs designed now, FG_GC is able to unpin the pinned file. >>> >>> fallocate() triggered FG_GC, but can't recycle space. It breaks the >>> design logic of FG_GC. >> >> Yes, contradictoriness exists. >> >> IMO, unpin file by GC looks more dangerous, it may cause potential data >> corruption w/ below case: >> 1. app pins file & holds LBAs of data blocks. >> 2. GC unpins file and migrates its data to new LBAs. >> 3. other file reuses previous LBAs. >> 4. app read/write data via previous LBAs. >> >> So I suggest to normalize use of pinfile and do not add more unpin cases >> in filesystem inner processes. >> >>> >>> This issue is happened in Android OTA scenario. fallocate() always >>> return failure cause OTA fail. >> >> Can you please check why other pinned files were so fragmented that f2fs_gc() >> can not recycle one free section? >> > Not because pinned files were fragmented, but if the GC victim section has one block is pinned will cause this issue. > > If the section don't unpin the block, it can't be recycled. But there is high chance that the pinned section will be chosen next time under f2fs current victim selection strategy. > > So if we want to avoid unpin files, I think change victim selection to considering pinned blocks can fix this issue. Oh, I get it. How about this? diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 325dab01a29d..3fb52dec5df8 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -1730,7 +1730,10 @@ next_alloc: f2fs_down_write(&sbi->gc_lock); stat_inc_gc_call_count(sbi, FOREGROUND); err = f2fs_gc(sbi, &gc_control); - if (err && err != -ENODATA) + + if (err == -EAGAIN) + f2fs_balance_fs(sbi, true); + else if (err && err != -ENODATA) goto out_err; } However, the code won't fix contradictoriness issue, because the root cause is we left fragmented pinned data in filesystem, which should be avoided in GC-reliance LFS filesyetem as much as possible. Thanks, > >> Thanks, >> >>> >>> And this commit changed previous behavior of fallocate(): >>> >>> Commit 2e42b7f817ac ("f2fs: stop allocating pinned sections if EAGAIN >>> happens") >>> >>> Before this commit, if fallocate() meet this situation, it will trigger >>> FG_GC to recycle pinned space finally. >>> >>> FG_GC is expected to recycle pinned space when there is no more free >>> space. And this is the right time to do it when fallocate() need free >>> space. >>> >>> It is weird when f2fs shows enough spare space but can't fallocate(). So >>> I think it should be fixed. >>> >>>> >>>> Thoughts? >>>> >>>> Thanks, >>>> >>>>> >>>>> This issue can be reproduced by filling f2fs space as following layout. >>>>> Every segment has one block is pinned: >>>>> +-+-+-+-+-+-+-----+-+ >>>>> | | |p| | | | ... | | seg_n >>>>> +-+-+-+-+-+-+-----+-+ >>>>> +-+-+-+-+-+-+-----+-+ >>>>> | | |p| | | | ... | | seg_n+1 >>>>> +-+-+-+-+-+-+-----+-+ >>>>> ... >>>>> +-+-+-+-+-+-+-----+-+ >>>>> | | |p| | | | ... | | seg_n+k >>>>> +-+-+-+-+-+-+-----+-+ >>>>> >>>>> And following are steps to reproduce this issue: >>>>> dd if=/dev/zero of=./f2fs_pin.img bs=2M count=1024 >>>>> mkfs.f2fs f2fs_pin.img >>>>> mkdir f2fs >>>>> mount f2fs_pin.img ./f2fs >>>>> cd f2fs >>>>> dd if=/dev/zero of=./large_padding bs=1M count=1760 >>>>> ./pin_filling.sh >>>>> rm padding* >>>>> sync >>>>> touch fallocate_40m >>>>> f2fs_io pinfile set fallocate_40m >>>>> fallocate -l 41943040 fallocate_40m >>>>> >>>>> fallocate always fail with EAGAIN even there has enough free space. >>>>> >>>>> 'pin_filling.sh' is: >>>>> count=1 >>>>> while : >>>>> do >>>>> # filling the seg space >>>>> for i in {1..511}: >>>>> do >>>>> name=padding_$count-$i >>>>> echo write $name >>>>> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >>>>> if [ $? -ne 0 ]; then >>>>> exit 0 >>>>> fi >>>>> done >>>>> sync >>>>> >>>>> # pin one block in a segment >>>>> name=pin_file$count >>>>> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >>>>> sync >>>>> f2fs_io pinfile set $name >>>>> count=$(($count + 1)) >>>>> done >>>>> >>>>> Signed-off-by: Wu Bo <bo.wu@vivo.com> >>>>> --- >>>>> fs/f2fs/file.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>>>> index ca5904129b16..e8a13616543f 100644 >>>>> --- a/fs/f2fs/file.c >>>>> +++ b/fs/f2fs/file.c >>>>> @@ -1690,7 +1690,7 @@ static int f2fs_expand_inode_data(struct inode >>>>> *inode, loff_t offset, >>>>> .init_gc_type = FG_GC, >>>>> .should_migrate_blocks = false, >>>>> .err_gc_skipped = true, >>>>> - .nr_free_secs = 0 }; >>>>> + .nr_free_secs = 1 }; >>>>> pgoff_t pg_start, pg_end; >>>>> loff_t new_size; >>>>> loff_t off_end;
On 2023/11/28 14:22, Chao Yu wrote: > On 2023/11/17 7:34, Wu Bo wrote: >> On 2023/11/11 12:49, Chao Yu wrote: >>> On 2023/11/8 21:48, Wu Bo wrote: >>>> On 2023/11/7 22:39, Chao Yu wrote: >>>>> On 2023/10/30 17:40, Wu Bo wrote: >>>>>> If GC victim has pinned block, it can't be recycled. >>>>>> And if GC is foreground running, after many failure try, the >>>>>> pinned file >>>>>> is expected to be clear pin flag. To enable the section be recycled. >>>>>> >>>>>> But when fallocate trigger FG_GC, GC can never recycle the pinned >>>>>> section. Because GC will go to stop before the failure try meet the >>>>>> threshold: >>>>>> if (has_enough_free_secs(sbi, sec_freed, 0)) { >>>>>> if (!gc_control->no_bg_gc && >>>>>> total_sec_freed < gc_control->nr_free_secs) >>>>>> goto go_gc_more; >>>>>> goto stop; >>>>>> } >>>>>> >>>>>> So when fallocate trigger FG_GC, at least recycle one. >>>>> >>>>> Hmm... it may break pinfile's semantics at least on one pinned file? >>>>> In this case, I prefer to fail fallocate() rather than unpinning >>>>> file, >>>>> in order to avoid leaving invalid LBA references of unpinned file >>>>> held >>>>> by userspace. >>>> >>>> As f2fs designed now, FG_GC is able to unpin the pinned file. >>>> >>>> fallocate() triggered FG_GC, but can't recycle space. It breaks the >>>> design logic of FG_GC. >>> >>> Yes, contradictoriness exists. >>> >>> IMO, unpin file by GC looks more dangerous, it may cause potential data >>> corruption w/ below case: >>> 1. app pins file & holds LBAs of data blocks. >>> 2. GC unpins file and migrates its data to new LBAs. >>> 3. other file reuses previous LBAs. >>> 4. app read/write data via previous LBAs. >>> >>> So I suggest to normalize use of pinfile and do not add more unpin >>> cases >>> in filesystem inner processes. >>> >>>> >>>> This issue is happened in Android OTA scenario. fallocate() always >>>> return failure cause OTA fail. >>> >>> Can you please check why other pinned files were so fragmented that >>> f2fs_gc() >>> can not recycle one free section? >>> >> Not because pinned files were fragmented, but if the GC victim >> section has one block is pinned will cause this issue. >> >> If the section don't unpin the block, it can't be recycled. But there >> is high chance that the pinned section will be chosen next time under >> f2fs current victim selection strategy. >> >> So if we want to avoid unpin files, I think change victim selection >> to considering pinned blocks can fix this issue. > > Oh, I get it. > > How about this? > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index 325dab01a29d..3fb52dec5df8 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -1730,7 +1730,10 @@ next_alloc: > f2fs_down_write(&sbi->gc_lock); > stat_inc_gc_call_count(sbi, FOREGROUND); > err = f2fs_gc(sbi, &gc_control); > - if (err && err != -ENODATA) > + > + if (err == -EAGAIN) > + f2fs_balance_fs(sbi, true); > + else if (err && err != -ENODATA) > goto out_err; > } Do you mean to call f2fs_balance_fs() to recycle one section? But in this situation, f2fs_balance_fs() will return at enough-free-section check: if (has_enough_free_secs(sbi, 0, 0)) return; > > However, the code won't fix contradictoriness issue, because the root > cause > is we left fragmented pinned data in filesystem, which should be > avoided in > GC-reliance LFS filesyetem as much as possible. > > Thanks, > >> >>> Thanks, >>> >>>> >>>> And this commit changed previous behavior of fallocate(): >>>> >>>> Commit 2e42b7f817ac ("f2fs: stop allocating pinned sections if EAGAIN >>>> happens") >>>> >>>> Before this commit, if fallocate() meet this situation, it will >>>> trigger >>>> FG_GC to recycle pinned space finally. >>>> >>>> FG_GC is expected to recycle pinned space when there is no more free >>>> space. And this is the right time to do it when fallocate() need free >>>> space. >>>> >>>> It is weird when f2fs shows enough spare space but can't >>>> fallocate(). So >>>> I think it should be fixed. >>>> >>>>> >>>>> Thoughts? >>>>> >>>>> Thanks, >>>>> >>>>>> >>>>>> This issue can be reproduced by filling f2fs space as following >>>>>> layout. >>>>>> Every segment has one block is pinned: >>>>>> +-+-+-+-+-+-+-----+-+ >>>>>> | | |p| | | | ... | | seg_n >>>>>> +-+-+-+-+-+-+-----+-+ >>>>>> +-+-+-+-+-+-+-----+-+ >>>>>> | | |p| | | | ... | | seg_n+1 >>>>>> +-+-+-+-+-+-+-----+-+ >>>>>> ... >>>>>> +-+-+-+-+-+-+-----+-+ >>>>>> | | |p| | | | ... | | seg_n+k >>>>>> +-+-+-+-+-+-+-----+-+ >>>>>> >>>>>> And following are steps to reproduce this issue: >>>>>> dd if=/dev/zero of=./f2fs_pin.img bs=2M count=1024 >>>>>> mkfs.f2fs f2fs_pin.img >>>>>> mkdir f2fs >>>>>> mount f2fs_pin.img ./f2fs >>>>>> cd f2fs >>>>>> dd if=/dev/zero of=./large_padding bs=1M count=1760 >>>>>> ./pin_filling.sh >>>>>> rm padding* >>>>>> sync >>>>>> touch fallocate_40m >>>>>> f2fs_io pinfile set fallocate_40m >>>>>> fallocate -l 41943040 fallocate_40m >>>>>> >>>>>> fallocate always fail with EAGAIN even there has enough free space. >>>>>> >>>>>> 'pin_filling.sh' is: >>>>>> count=1 >>>>>> while : >>>>>> do >>>>>> # filling the seg space >>>>>> for i in {1..511}: >>>>>> do >>>>>> name=padding_$count-$i >>>>>> echo write $name >>>>>> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >>>>>> if [ $? -ne 0 ]; then >>>>>> exit 0 >>>>>> fi >>>>>> done >>>>>> sync >>>>>> >>>>>> # pin one block in a segment >>>>>> name=pin_file$count >>>>>> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >>>>>> sync >>>>>> f2fs_io pinfile set $name >>>>>> count=$(($count + 1)) >>>>>> done >>>>>> >>>>>> Signed-off-by: Wu Bo <bo.wu@vivo.com> >>>>>> --- >>>>>> fs/f2fs/file.c | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>>>>> index ca5904129b16..e8a13616543f 100644 >>>>>> --- a/fs/f2fs/file.c >>>>>> +++ b/fs/f2fs/file.c >>>>>> @@ -1690,7 +1690,7 @@ static int f2fs_expand_inode_data(struct inode >>>>>> *inode, loff_t offset, >>>>>> .init_gc_type = FG_GC, >>>>>> .should_migrate_blocks = false, >>>>>> .err_gc_skipped = true, >>>>>> - .nr_free_secs = 0 }; >>>>>> + .nr_free_secs = 1 }; >>>>>> pgoff_t pg_start, pg_end; >>>>>> loff_t new_size; >>>>>> loff_t off_end;
On 2023/11/28 20:51, Wu Bo wrote: > > On 2023/11/28 14:22, Chao Yu wrote: >> On 2023/11/17 7:34, Wu Bo wrote: >>> On 2023/11/11 12:49, Chao Yu wrote: >>>> On 2023/11/8 21:48, Wu Bo wrote: >>>>> On 2023/11/7 22:39, Chao Yu wrote: >>>>>> On 2023/10/30 17:40, Wu Bo wrote: >>>>>>> If GC victim has pinned block, it can't be recycled. >>>>>>> And if GC is foreground running, after many failure try, the pinned file >>>>>>> is expected to be clear pin flag. To enable the section be recycled. >>>>>>> >>>>>>> But when fallocate trigger FG_GC, GC can never recycle the pinned >>>>>>> section. Because GC will go to stop before the failure try meet the >>>>>>> threshold: >>>>>>> if (has_enough_free_secs(sbi, sec_freed, 0)) { >>>>>>> if (!gc_control->no_bg_gc && >>>>>>> total_sec_freed < gc_control->nr_free_secs) >>>>>>> goto go_gc_more; >>>>>>> goto stop; >>>>>>> } >>>>>>> >>>>>>> So when fallocate trigger FG_GC, at least recycle one. >>>>>> >>>>>> Hmm... it may break pinfile's semantics at least on one pinned file? >>>>>> In this case, I prefer to fail fallocate() rather than unpinning file, >>>>>> in order to avoid leaving invalid LBA references of unpinned file held >>>>>> by userspace. >>>>> >>>>> As f2fs designed now, FG_GC is able to unpin the pinned file. >>>>> >>>>> fallocate() triggered FG_GC, but can't recycle space. It breaks the >>>>> design logic of FG_GC. >>>> >>>> Yes, contradictoriness exists. >>>> >>>> IMO, unpin file by GC looks more dangerous, it may cause potential data >>>> corruption w/ below case: >>>> 1. app pins file & holds LBAs of data blocks. >>>> 2. GC unpins file and migrates its data to new LBAs. >>>> 3. other file reuses previous LBAs. >>>> 4. app read/write data via previous LBAs. >>>> >>>> So I suggest to normalize use of pinfile and do not add more unpin cases >>>> in filesystem inner processes. >>>> >>>>> >>>>> This issue is happened in Android OTA scenario. fallocate() always >>>>> return failure cause OTA fail. >>>> >>>> Can you please check why other pinned files were so fragmented that f2fs_gc() >>>> can not recycle one free section? >>>> >>> Not because pinned files were fragmented, but if the GC victim section has one block is pinned will cause this issue. >>> >>> If the section don't unpin the block, it can't be recycled. But there is high chance that the pinned section will be chosen next time under f2fs current victim selection strategy. >>> >>> So if we want to avoid unpin files, I think change victim selection to considering pinned blocks can fix this issue. >> >> Oh, I get it. >> >> How about this? >> >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >> index 325dab01a29d..3fb52dec5df8 100644 >> --- a/fs/f2fs/file.c >> +++ b/fs/f2fs/file.c >> @@ -1730,7 +1730,10 @@ next_alloc: >> f2fs_down_write(&sbi->gc_lock); >> stat_inc_gc_call_count(sbi, FOREGROUND); >> err = f2fs_gc(sbi, &gc_control); >> - if (err && err != -ENODATA) >> + >> + if (err == -EAGAIN) >> + f2fs_balance_fs(sbi, true); >> + else if (err && err != -ENODATA) >> goto out_err; >> } > Do you mean to call f2fs_balance_fs() to recycle one section? > But in this situation, f2fs_balance_fs() will return at enough-free-section check: > if (has_enough_free_secs(sbi, 0, 0)) > return; As you said, there are lots of free segments, so I guess it's fine for latter 2m-aligned allocation, and for the case number of free section is lower than fggc threshold, we can call f2fs_balance_fs() to reclaim enough free sections. Thanks, >> >> However, the code won't fix contradictoriness issue, because the root cause >> is we left fragmented pinned data in filesystem, which should be avoided in >> GC-reliance LFS filesyetem as much as possible. >> >> Thanks, >> >>> >>>> Thanks, >>>> >>>>> >>>>> And this commit changed previous behavior of fallocate(): >>>>> >>>>> Commit 2e42b7f817ac ("f2fs: stop allocating pinned sections if EAGAIN >>>>> happens") >>>>> >>>>> Before this commit, if fallocate() meet this situation, it will trigger >>>>> FG_GC to recycle pinned space finally. >>>>> >>>>> FG_GC is expected to recycle pinned space when there is no more free >>>>> space. And this is the right time to do it when fallocate() need free >>>>> space. >>>>> >>>>> It is weird when f2fs shows enough spare space but can't fallocate(). So >>>>> I think it should be fixed. >>>>> >>>>>> >>>>>> Thoughts? >>>>>> >>>>>> Thanks, >>>>>> >>>>>>> >>>>>>> This issue can be reproduced by filling f2fs space as following layout. >>>>>>> Every segment has one block is pinned: >>>>>>> +-+-+-+-+-+-+-----+-+ >>>>>>> | | |p| | | | ... | | seg_n >>>>>>> +-+-+-+-+-+-+-----+-+ >>>>>>> +-+-+-+-+-+-+-----+-+ >>>>>>> | | |p| | | | ... | | seg_n+1 >>>>>>> +-+-+-+-+-+-+-----+-+ >>>>>>> ... >>>>>>> +-+-+-+-+-+-+-----+-+ >>>>>>> | | |p| | | | ... | | seg_n+k >>>>>>> +-+-+-+-+-+-+-----+-+ >>>>>>> >>>>>>> And following are steps to reproduce this issue: >>>>>>> dd if=/dev/zero of=./f2fs_pin.img bs=2M count=1024 >>>>>>> mkfs.f2fs f2fs_pin.img >>>>>>> mkdir f2fs >>>>>>> mount f2fs_pin.img ./f2fs >>>>>>> cd f2fs >>>>>>> dd if=/dev/zero of=./large_padding bs=1M count=1760 >>>>>>> ./pin_filling.sh >>>>>>> rm padding* >>>>>>> sync >>>>>>> touch fallocate_40m >>>>>>> f2fs_io pinfile set fallocate_40m >>>>>>> fallocate -l 41943040 fallocate_40m >>>>>>> >>>>>>> fallocate always fail with EAGAIN even there has enough free space. >>>>>>> >>>>>>> 'pin_filling.sh' is: >>>>>>> count=1 >>>>>>> while : >>>>>>> do >>>>>>> # filling the seg space >>>>>>> for i in {1..511}: >>>>>>> do >>>>>>> name=padding_$count-$i >>>>>>> echo write $name >>>>>>> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >>>>>>> if [ $? -ne 0 ]; then >>>>>>> exit 0 >>>>>>> fi >>>>>>> done >>>>>>> sync >>>>>>> >>>>>>> # pin one block in a segment >>>>>>> name=pin_file$count >>>>>>> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >>>>>>> sync >>>>>>> f2fs_io pinfile set $name >>>>>>> count=$(($count + 1)) >>>>>>> done >>>>>>> >>>>>>> Signed-off-by: Wu Bo <bo.wu@vivo.com> >>>>>>> --- >>>>>>> fs/f2fs/file.c | 2 +- >>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>>>>>> index ca5904129b16..e8a13616543f 100644 >>>>>>> --- a/fs/f2fs/file.c >>>>>>> +++ b/fs/f2fs/file.c >>>>>>> @@ -1690,7 +1690,7 @@ static int f2fs_expand_inode_data(struct inode >>>>>>> *inode, loff_t offset, >>>>>>> .init_gc_type = FG_GC, >>>>>>> .should_migrate_blocks = false, >>>>>>> .err_gc_skipped = true, >>>>>>> - .nr_free_secs = 0 }; >>>>>>> + .nr_free_secs = 1 }; >>>>>>> pgoff_t pg_start, pg_end; >>>>>>> loff_t new_size; >>>>>>> loff_t off_end;
On 2023/12/9 17:46, Chao Yu wrote: > On 2023/11/28 20:51, Wu Bo wrote: >> >> On 2023/11/28 14:22, Chao Yu wrote: >>> On 2023/11/17 7:34, Wu Bo wrote: >>>> On 2023/11/11 12:49, Chao Yu wrote: >>>>> On 2023/11/8 21:48, Wu Bo wrote: >>>>>> On 2023/11/7 22:39, Chao Yu wrote: >>>>>>> On 2023/10/30 17:40, Wu Bo wrote: >>>>>>>> If GC victim has pinned block, it can't be recycled. >>>>>>>> And if GC is foreground running, after many failure try, the >>>>>>>> pinned file >>>>>>>> is expected to be clear pin flag. To enable the section be >>>>>>>> recycled. >>>>>>>> >>>>>>>> But when fallocate trigger FG_GC, GC can never recycle the pinned >>>>>>>> section. Because GC will go to stop before the failure try meet >>>>>>>> the >>>>>>>> threshold: >>>>>>>> if (has_enough_free_secs(sbi, sec_freed, 0)) { >>>>>>>> if (!gc_control->no_bg_gc && >>>>>>>> total_sec_freed < gc_control->nr_free_secs) >>>>>>>> goto go_gc_more; >>>>>>>> goto stop; >>>>>>>> } >>>>>>>> >>>>>>>> So when fallocate trigger FG_GC, at least recycle one. >>>>>>> >>>>>>> Hmm... it may break pinfile's semantics at least on one pinned >>>>>>> file? >>>>>>> In this case, I prefer to fail fallocate() rather than unpinning >>>>>>> file, >>>>>>> in order to avoid leaving invalid LBA references of unpinned >>>>>>> file held >>>>>>> by userspace. >>>>>> >>>>>> As f2fs designed now, FG_GC is able to unpin the pinned file. >>>>>> >>>>>> fallocate() triggered FG_GC, but can't recycle space. It breaks the >>>>>> design logic of FG_GC. >>>>> >>>>> Yes, contradictoriness exists. >>>>> >>>>> IMO, unpin file by GC looks more dangerous, it may cause potential >>>>> data >>>>> corruption w/ below case: >>>>> 1. app pins file & holds LBAs of data blocks. >>>>> 2. GC unpins file and migrates its data to new LBAs. >>>>> 3. other file reuses previous LBAs. >>>>> 4. app read/write data via previous LBAs. >>>>> >>>>> So I suggest to normalize use of pinfile and do not add more unpin >>>>> cases >>>>> in filesystem inner processes. >>>>> >>>>>> >>>>>> This issue is happened in Android OTA scenario. fallocate() always >>>>>> return failure cause OTA fail. >>>>> >>>>> Can you please check why other pinned files were so fragmented >>>>> that f2fs_gc() >>>>> can not recycle one free section? >>>>> >>>> Not because pinned files were fragmented, but if the GC victim >>>> section has one block is pinned will cause this issue. >>>> >>>> If the section don't unpin the block, it can't be recycled. But >>>> there is high chance that the pinned section will be chosen next >>>> time under f2fs current victim selection strategy. >>>> >>>> So if we want to avoid unpin files, I think change victim selection >>>> to considering pinned blocks can fix this issue. >>> >>> Oh, I get it. >>> >>> How about this? >>> >>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>> index 325dab01a29d..3fb52dec5df8 100644 >>> --- a/fs/f2fs/file.c >>> +++ b/fs/f2fs/file.c >>> @@ -1730,7 +1730,10 @@ next_alloc: >>> f2fs_down_write(&sbi->gc_lock); >>> stat_inc_gc_call_count(sbi, FOREGROUND); >>> err = f2fs_gc(sbi, &gc_control); >>> - if (err && err != -ENODATA) >>> + >>> + if (err == -EAGAIN) >>> + f2fs_balance_fs(sbi, true); >>> + else if (err && err != -ENODATA) >>> goto out_err; >>> } >> Do you mean to call f2fs_balance_fs() to recycle one section? >> But in this situation, f2fs_balance_fs() will return at >> enough-free-section check: >> if (has_enough_free_secs(sbi, 0, 0)) >> return; > > As you said, there are lots of free segments, so I guess it's fine for > latter 2m-aligned allocation, and for the case number of free section is > lower than fggc threshold, we can call f2fs_balance_fs() to reclaim > enough > free sections. > > Thanks, Yes, this make sense. I didn't see allocation will continue after f2fs_balance_fs() return. > >>> >>> However, the code won't fix contradictoriness issue, because the >>> root cause >>> is we left fragmented pinned data in filesystem, which should be >>> avoided in >>> GC-reliance LFS filesyetem as much as possible. >>> >>> Thanks, >>> >>>> >>>>> Thanks, >>>>> >>>>>> >>>>>> And this commit changed previous behavior of fallocate(): >>>>>> >>>>>> Commit 2e42b7f817ac ("f2fs: stop allocating pinned sections if >>>>>> EAGAIN >>>>>> happens") >>>>>> >>>>>> Before this commit, if fallocate() meet this situation, it will >>>>>> trigger >>>>>> FG_GC to recycle pinned space finally. >>>>>> >>>>>> FG_GC is expected to recycle pinned space when there is no more free >>>>>> space. And this is the right time to do it when fallocate() need >>>>>> free >>>>>> space. >>>>>> >>>>>> It is weird when f2fs shows enough spare space but can't >>>>>> fallocate(). So >>>>>> I think it should be fixed. >>>>>> >>>>>>> >>>>>>> Thoughts? >>>>>>> >>>>>>> Thanks, >>>>>>> >>>>>>>> >>>>>>>> This issue can be reproduced by filling f2fs space as following >>>>>>>> layout. >>>>>>>> Every segment has one block is pinned: >>>>>>>> +-+-+-+-+-+-+-----+-+ >>>>>>>> | | |p| | | | ... | | seg_n >>>>>>>> +-+-+-+-+-+-+-----+-+ >>>>>>>> +-+-+-+-+-+-+-----+-+ >>>>>>>> | | |p| | | | ... | | seg_n+1 >>>>>>>> +-+-+-+-+-+-+-----+-+ >>>>>>>> ... >>>>>>>> +-+-+-+-+-+-+-----+-+ >>>>>>>> | | |p| | | | ... | | seg_n+k >>>>>>>> +-+-+-+-+-+-+-----+-+ >>>>>>>> >>>>>>>> And following are steps to reproduce this issue: >>>>>>>> dd if=/dev/zero of=./f2fs_pin.img bs=2M count=1024 >>>>>>>> mkfs.f2fs f2fs_pin.img >>>>>>>> mkdir f2fs >>>>>>>> mount f2fs_pin.img ./f2fs >>>>>>>> cd f2fs >>>>>>>> dd if=/dev/zero of=./large_padding bs=1M count=1760 >>>>>>>> ./pin_filling.sh >>>>>>>> rm padding* >>>>>>>> sync >>>>>>>> touch fallocate_40m >>>>>>>> f2fs_io pinfile set fallocate_40m >>>>>>>> fallocate -l 41943040 fallocate_40m >>>>>>>> >>>>>>>> fallocate always fail with EAGAIN even there has enough free >>>>>>>> space. >>>>>>>> >>>>>>>> 'pin_filling.sh' is: >>>>>>>> count=1 >>>>>>>> while : >>>>>>>> do >>>>>>>> # filling the seg space >>>>>>>> for i in {1..511}: >>>>>>>> do >>>>>>>> name=padding_$count-$i >>>>>>>> echo write $name >>>>>>>> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null >>>>>>>> 2>&1 >>>>>>>> if [ $? -ne 0 ]; then >>>>>>>> exit 0 >>>>>>>> fi >>>>>>>> done >>>>>>>> sync >>>>>>>> >>>>>>>> # pin one block in a segment >>>>>>>> name=pin_file$count >>>>>>>> dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 >>>>>>>> sync >>>>>>>> f2fs_io pinfile set $name >>>>>>>> count=$(($count + 1)) >>>>>>>> done >>>>>>>> >>>>>>>> Signed-off-by: Wu Bo <bo.wu@vivo.com> >>>>>>>> --- >>>>>>>> fs/f2fs/file.c | 2 +- >>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>>>>>>> index ca5904129b16..e8a13616543f 100644 >>>>>>>> --- a/fs/f2fs/file.c >>>>>>>> +++ b/fs/f2fs/file.c >>>>>>>> @@ -1690,7 +1690,7 @@ static int f2fs_expand_inode_data(struct >>>>>>>> inode >>>>>>>> *inode, loff_t offset, >>>>>>>> .init_gc_type = FG_GC, >>>>>>>> .should_migrate_blocks = false, >>>>>>>> .err_gc_skipped = true, >>>>>>>> - .nr_free_secs = 0 }; >>>>>>>> + .nr_free_secs = 1 }; >>>>>>>> pgoff_t pg_start, pg_end; >>>>>>>> loff_t new_size; >>>>>>>> loff_t off_end;
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index ca5904129b16..e8a13616543f 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -1690,7 +1690,7 @@ static int f2fs_expand_inode_data(struct inode *inode, loff_t offset, .init_gc_type = FG_GC, .should_migrate_blocks = false, .err_gc_skipped = true, - .nr_free_secs = 0 }; + .nr_free_secs = 1 }; pgoff_t pg_start, pg_end; loff_t new_size; loff_t off_end;
If GC victim has pinned block, it can't be recycled. And if GC is foreground running, after many failure try, the pinned file is expected to be clear pin flag. To enable the section be recycled. But when fallocate trigger FG_GC, GC can never recycle the pinned section. Because GC will go to stop before the failure try meet the threshold: if (has_enough_free_secs(sbi, sec_freed, 0)) { if (!gc_control->no_bg_gc && total_sec_freed < gc_control->nr_free_secs) goto go_gc_more; goto stop; } So when fallocate trigger FG_GC, at least recycle one. This issue can be reproduced by filling f2fs space as following layout. Every segment has one block is pinned: +-+-+-+-+-+-+-----+-+ | | |p| | | | ... | | seg_n +-+-+-+-+-+-+-----+-+ +-+-+-+-+-+-+-----+-+ | | |p| | | | ... | | seg_n+1 +-+-+-+-+-+-+-----+-+ ... +-+-+-+-+-+-+-----+-+ | | |p| | | | ... | | seg_n+k +-+-+-+-+-+-+-----+-+ And following are steps to reproduce this issue: dd if=/dev/zero of=./f2fs_pin.img bs=2M count=1024 mkfs.f2fs f2fs_pin.img mkdir f2fs mount f2fs_pin.img ./f2fs cd f2fs dd if=/dev/zero of=./large_padding bs=1M count=1760 ./pin_filling.sh rm padding* sync touch fallocate_40m f2fs_io pinfile set fallocate_40m fallocate -l 41943040 fallocate_40m fallocate always fail with EAGAIN even there has enough free space. 'pin_filling.sh' is: count=1 while : do # filling the seg space for i in {1..511}: do name=padding_$count-$i echo write $name dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 if [ $? -ne 0 ]; then exit 0 fi done sync # pin one block in a segment name=pin_file$count dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1 sync f2fs_io pinfile set $name count=$(($count + 1)) done Signed-off-by: Wu Bo <bo.wu@vivo.com> --- fs/f2fs/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)