Re: [f2fs-dev] [PATCH v1] f2fs: avoid victim selection from previous victim section

2022-11-22 Thread Chao Yu

Hi Yonggil,

I guess your email client forces converting tab and space characters of
patch, please check that.

On 2022/11/22 10:36, Yonggil Song wrote:

When f2fs chooses GC victim in large section & LFS mode,
next_victim_seg[gc_type] is referenced first. After segment is freed,
next_victim_seg[gc_type] has the next segment number.
However, next_victim_seg[gc_type] still has the last segment number
even after the last segment of section is freed. In this case, when f2fs
chooses a victim for the next GC round, the last segment of previous victim
section is chosen as a victim.

Initialize next_victim_seg[gc_type] to NULL_SEGNO for the last segment in
large section.

Fixes: e3080b0120a1 ("f2fs: support subsectional garbage collection")


Good catch, I'm fine with this fix.

Thanks,


Signed-off-by: Yonggil Song 
---
  fs/f2fs/gc.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 4546e01b2ee0..10677d53ef0e 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1744,8 +1744,9 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
  get_valid_blocks(sbi, segno, false) == 0)
  seg_freed++;
  
-if (__is_large_section(sbi) && segno + 1 < end_segno)

-sbi->next_victim_seg[gc_type] = segno + 1;
+if (__is_large_section(sbi))
+sbi->next_victim_seg[gc_type] =
+(segno + 1 < end_segno) ? segno + 1 : 
NULL_SEGNO;
  skip:
  f2fs_put_page(sum_page, 0);
  }
--
2.34.1



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


Re: [f2fs-dev] (2) [PATCH v1] f2fs: avoid victim selection from previous victim section

2022-11-22 Thread Yonggil Song
Hi Chao,

Thanks for your review.
I'll fix this and resend a mail.

Thanks

>Hi Yonggil,
>
>I guess your email client forces converting tab and space characters of
>patch, please check that.
>
>On 2022/11/22 10:36, Yonggil Song wrote:
>> When f2fs chooses GC victim in large section & LFS mode,
>> next_victim_seg[gc_type] is referenced first. After segment is freed,
>> next_victim_seg[gc_type] has the next segment number.
>> However, next_victim_seg[gc_type] still has the last segment number
>> even after the last segment of section is freed. In this case, when f2fs
>> chooses a victim for the next GC round, the last segment of previous victim
>> section is chosen as a victim.
>> 
>> Initialize next_victim_seg[gc_type] to NULL_SEGNO for the last segment in
>> large section.
>> 
>> Fixes: e3080b0120a1 ("f2fs: support subsectional garbage collection")
>
>Good catch, I'm fine with this fix.
>
>Thanks,
>
>> Signed-off-by: Yonggil Song 
>> ---
>>   fs/f2fs/gc.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index 4546e01b2ee0..10677d53ef0e 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -1744,8 +1744,9 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>>   get_valid_blocks(sbi, segno, false) == 0)
>>   seg_freed++;
>>   
>> -if (__is_large_section(sbi) && segno + 1 < end_segno)
>> -sbi->next_victim_seg[gc_type] = segno + 1;
>> +if (__is_large_section(sbi))
>> +sbi->next_victim_seg[gc_type] =
>> +(segno + 1 < end_segno) ? segno + 1 : 
>> NULL_SEGNO;
>>   skip:
>>   f2fs_put_page(sum_page, 0);
>>   }
>> -- 
>> 2.34.1


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


[f2fs-dev] [Bug 216050] f2fs_gc occupies 100% cpu

2022-11-22 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=216050

--- Comment #89 from bogdan.nico...@gmail.com ---
I confirm the bug persists both with background_gc=on and background_gc=sync.
It's especially prone to manifest when the machine is idle for a long time. It
almost feels like the gc hangs because it has nothing to collect and therefore
it is entering an infinite loop.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

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


Re: [f2fs-dev] [PATCH 2/5] fs: affs: initialize fsdata in affs_truncate()

2022-11-22 Thread David Sterba
On Mon, Nov 21, 2022 at 12:21:31PM +0100, Alexander Potapenko wrote:
> When aops->write_begin() does not initialize fsdata, KMSAN may report
> an error passing the latter to aops->write_end().
> 
> Fix this by unconditionally initializing fsdata.
> 
> Suggested-by: Eric Biggers 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Alexander Potapenko 

With the fixed Fixes: reference,

Acked-by: David Sterba 


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


Re: [f2fs-dev] [PATCH 2/5] fs: affs: initialize fsdata in affs_truncate()

2022-11-22 Thread Alexander Potapenko via Linux-f2fs-devel
On Mon, Nov 21, 2022 at 8:46 PM Eric Biggers  wrote:
>
> On Mon, Nov 21, 2022 at 12:21:31PM +0100, Alexander Potapenko wrote:
> > When aops->write_begin() does not initialize fsdata, KMSAN may report
> > an error passing the latter to aops->write_end().
> >
> > Fix this by unconditionally initializing fsdata.
> >
> > Suggested-by: Eric Biggers 
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>
> Are you sure that is the correct Fixes commit?  What about commit f2b6a16eb8f5
> ("fs: affs convert to new aops")?

Hmm, indeed, you are right.

> - Eric



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg


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


[f2fs-dev] [RESEND][PATCH] f2fs: avoid victim selection from previous victim section

2022-11-22 Thread Yonggil Song
When f2fs chooses GC victim in large section & LFS mode,
next_victim_seg[gc_type] is referenced first. After segment is freed,
next_victim_seg[gc_type] has the next segment number.
However, next_victim_seg[gc_type] still has the last segment number
even after the last segment of section is freed. In this case, when f2fs
chooses a victim for the next GC round, the last segment of previous victim
section is chosen as a victim.

Initialize next_victim_seg[gc_type] to NULL_SEGNO for the last segment in
large section.

Fixes: e3080b0120a1 ("f2fs: support subsectional garbage collection")
Signed-off-by: Yonggil Song 
---
 fs/f2fs/gc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 0f967b1e98f2..f1b68eda2235 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1749,8 +1749,9 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
get_valid_blocks(sbi, segno, false) == 0)
seg_freed++;
 
-   if (__is_large_section(sbi) && segno + 1 < end_segno)
-   sbi->next_victim_seg[gc_type] = segno + 1;
+   if (__is_large_section(sbi))
+   sbi->next_victim_seg[gc_type] =
+   (segno + 1 < end_segno) ? segno + 1 : 
NULL_SEGNO;
 skip:
f2fs_put_page(sum_page, 0);
}
-- 
2.34.1


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


[f2fs-dev] [Bug 216050] f2fs_gc occupies 100% cpu

2022-11-22 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=216050

--- Comment #90 from Guido (guido.iod...@gmail.com) ---
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index a71e818cd67b..c351c3269874 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1325,6 +1325,7 @@ struct page *f2fs_get_lock_data_page(struct inode
> *inode, pgoff_t index,
> lock_page(page);
> if (unlikely(page->mapping != mapping)) {
> f2fs_put_page(page, 1);
> +   f2fs_io_schedule_timeout(DEFAULT_IO_TIMEOUT);
> goto repeat;
> }
> if (unlikely(!PageUptodate(page))) {


this patch seems to avoid the 100% cpu occupation but still doesn't solve the
bug. I was wrong in the last comment, it's an improvement!

As a workaround I tried to build the f2fs module from 5.17 but I failed. I'm
not an expert, so I don't know how to forward-port the module.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

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


Re: [f2fs-dev] [PATCH v3 14/23] f2fs: Convert f2fs_write_cache_pages() to use filemap_get_folios_tag()

2022-11-22 Thread Vishal Moola
On Mon, Nov 14, 2022 at 1:38 PM Vishal Moola  wrote:
>
> On Sun, Nov 13, 2022 at 11:02 PM Chao Yu  wrote:
> >
> > On 2022/10/18 4:24, Vishal Moola (Oracle) wrote:
> > > Converted the function to use a folio_batch instead of pagevec. This is in
> > > preparation for the removal of find_get_pages_range_tag().
> > >
> > > Also modified f2fs_all_cluster_page_ready to take in a folio_batch instead
> > > of pagevec. This does NOT support large folios. The function currently
> >
> > Vishal,
> >
> > It looks this patch tries to revert Fengnan's change:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=01fc4b9a6ed8eacb64e5609bab7ac963e1c7e486
> >
> > How about doing some tests to evaluate its performance effect?
>
> Yeah I'll play around with it to see how much of a difference it makes.

I did some testing. Looks like reverting Fengnan's change allows for
occasional, but significant, spikes in write latency. I'll work on a variation
of the patch that maintains the use of F2FS_ONSTACK_PAGES and send
that in the next version of the patch series. Thanks for pointing that out!

How do the remaining f2fs patches in the series look to you?
Patch 16/23 f2fs_sync_meta_pages() in particular seems like it may
be prone to problems. If there are any changes that need to be made to
it I can include those in the next version as well.


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


Re: [f2fs-dev] [PATCH v3 14/23] f2fs: Convert f2fs_write_cache_pages() to use filemap_get_folios_tag()

2022-11-22 Thread Vishal Moola
On Tue, Nov 22, 2022 at 6:26 PM Vishal Moola  wrote:
>
> On Mon, Nov 14, 2022 at 1:38 PM Vishal Moola  wrote:
> >
> > On Sun, Nov 13, 2022 at 11:02 PM Chao Yu  wrote:
> > >
> > > On 2022/10/18 4:24, Vishal Moola (Oracle) wrote:
> > > > Converted the function to use a folio_batch instead of pagevec. This is 
> > > > in
> > > > preparation for the removal of find_get_pages_range_tag().
> > > >
> > > > Also modified f2fs_all_cluster_page_ready to take in a folio_batch 
> > > > instead
> > > > of pagevec. This does NOT support large folios. The function currently
> > >
> > > Vishal,
> > >
> > > It looks this patch tries to revert Fengnan's change:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=01fc4b9a6ed8eacb64e5609bab7ac963e1c7e486
> > >
> > > How about doing some tests to evaluate its performance effect?
> >
> > Yeah I'll play around with it to see how much of a difference it makes.
>
> I did some testing. Looks like reverting Fengnan's change allows for
> occasional, but significant, spikes in write latency. I'll work on a variation
> of the patch that maintains the use of F2FS_ONSTACK_PAGES and send
> that in the next version of the patch series. Thanks for pointing that out!

Here are some numbers for reference to performance. I'm thinking we may
want to go with the new version, but I'll let you be the judge of that.
I ran some fio random write tests with block size 64k on a system with 8 cpus.

1 job with 1 io-depth:
Baseline:
  slat (usec): min=8, max=849, avg=16.47, stdev=12.33
  clat (nsec): min=253, max=751838, avg=346.51, stdev=2452.10
  lat (usec): min=9, max=854, avg=17.00, stdev=12.74
  lat (nsec)   : 500=97.09%, 750=1.73%, 1000=0.57%
  lat (usec)   : 2=0.41%, 4=0.09%, 10=0.06%, 20=0.04%, 50=0.01%
  lat (usec)   : 100=0.01%, 1000=0.01%

This patch:
  slat (usec): min=9, max=3690, avg=16.61, stdev=17.36
  clat (nsec): min=28, max=380434, avg=336.59, stdev=1571.23
  lat (usec): min=10, max=3699, avg=17.13, stdev=17.51
  lat (nsec)   : 50=0.01%, 500=97.95%, 750=1.42%, 1000=0.33%
  lat (usec)   : 2=0.19%, 4=0.05%, 10=0.03%, 20=0.03%, 50=0.01%
  lat (usec)   : 100=0.01%, 250=0.01%, 500=0.01%

Folios w/ F2FS_ONSTACK_PAGES (next version):
  slat (usec): min=12, max=13623, avg=19.48, stdev=48.94
  clat (nsec): min=265, max=386917, avg=380.97, stdev=1679.85
  lat (usec): min=12, max=13635, avg=20.06, stdev=49.27
  lat (nsec)   : 500=93.55%, 750=4.62%, 1000=0.92%
  lat (usec)   : 2=0.65%, 4=0.09%, 10=0.10%, 20=0.06%, 50=0.01%
  lat (usec)   : 100=0.01%, 250=0.01%, 500=0.01%

1 job with 16 io-depth:
Baseline:
  slat (usec): min=8, max=3907, avg=16.89, stdev=23.39
  clat (usec): min=12, max=15160k, avg=5.61, stdev=265051.86
  lat (usec): min=137, max=15160k, avg=11132.68, stdev=265051.75
  lat (usec)   : 20=0.01%, 250=57.66%, 500=39.56%, 750=1.96%, 1000=0.22%
  lat (msec)   : 2=0.16%, 4=0.06%, 10=0.01%, 2000=0.29%, >=2000=0.08%

This patch:
  slat (usec): min=9, max=1230, avg=17.15, stdev=12.95
  clat (usec): min=4, max=39471k, avg=14825.22, stdev=588237.30
  lat (usec): min=80, max=39471k, avg=14842.55, stdev=588237.27
  lat (usec)   : 10=0.01%, 250=38.78%, 500=59.53%, 750=1.12%, 1000=0.16%
  lat (msec)   : 2=0.04%, 2000=0.34%, >=2000=0.02%

Folios w/ F2FS_ONSTACK_PAGES (next version):
  slat (usec): min=9, max=1188, avg=18.74, stdev=14.12
  clat (usec): min=5, max=15278k, avg=8936.75, stdev=214230.09
  lat (usec): min=90, max=15278k, avg=8955.67, stdev=214230.10
  lat (usec)   : 10=0.01%, 250=9.68%, 500=86.49%, 750=2.74%, 1000=0.54%
  lat (msec)   : 2=0.18%, 2000=0.32%, >=2000=0.04%


> How do the remaining f2fs patches in the series look to you?
> Patch 16/23 f2fs_sync_meta_pages() in particular seems like it may
> be prone to problems. If there are any changes that need to be made to
> it I can include those in the next version as well.


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