Re: [PATCH 1/1] f2fs: fix fallocate failed under pinned block situation

[Chao Yu]

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@xxxxxxxx>
> ---
>   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;