On 2024/10/18 15:32, Kefeng Wang wrote:
On 2024/10/18 13:23, Barry Song wrote:
On Fri, Oct 18, 2024 at 6:20 PM Kefeng Wang
<wangkefeng.wang@xxxxxxxxxx> wrote:
On 2024/10/17 23:09, Matthew Wilcox wrote:
On Thu, Oct 17, 2024 at 10:25:04PM +0800, Kefeng Wang wrote:
Directly use folio_zero_range() to cleanup code.
Are you sure there's no performance regression introduced by this?
clear_highpage() is often optimised in ways that we can't optimise for
a plain memset(). On the other hand, if the folio is large, maybe a
modern CPU will be able to do better than clear-one-page-at-a-time.
Right, I missing this, clear_page might be better than memset, I change
this one when look at the shmem_writepage(), which already convert to
use folio_zero_range() from clear_highpage(), also I grep
folio_zero_range(), there are some other to use folio_zero_range().
fs/bcachefs/fs-io-buffered.c: folio_zero_range(folio, 0,
folio_size(folio));
fs/bcachefs/fs-io-buffered.c: folio_zero_range(f,
0, folio_size(f));
fs/bcachefs/fs-io-buffered.c: folio_zero_range(f,
0, folio_size(f));
fs/libfs.c: folio_zero_range(folio, 0, folio_size(folio));
fs/ntfs3/frecord.c: folio_zero_range(folio, 0,
folio_size(folio));
mm/page_io.c: folio_zero_range(folio, 0, folio_size(folio));
mm/shmem.c: folio_zero_range(folio, 0, folio_size(folio));
IOW, what performance testing have you done with this patch?
No performance test before, but I write a testcase,
1) allocate N large folios (folio_alloc(PMD_ORDER))
2) then calculate the diff(us) when clear all N folios
clear_highpage/folio_zero_range/folio_zero_user
3) release N folios
the result(run 5 times) shown below on my machine,
N=1,
clear_highpage folio_zero_range folio_zero_user
1 69 74 177
2 57 62 168
3 54 58 234
4 54 58 157
5 56 62 148
avg 58 62.8 176.8
N=100
clear_highpage folio_zero_range folio_zero_user
1 11015 11309 32833
2 10385 11110 49751
3 10369 11056 33095
4 10332 11017 33106
5 10483 11000 49032
avg 10516.8 11098.4 39563.4
N=512
clear_highpage folio_zero_range folio_zero_user
1 55560 60055 156876
2 55485 60024 157132
3 55474 60129 156658
4 55555 59867 157259
5 55528 59932 157108
avg 55520.4 60001.4 157006.6
folio_zero_user with many cond_resched(), so time fluctuates a lot,
clear_highpage is better folio_zero_range as you said.
Maybe add a new helper to convert all folio_zero_range(folio, 0,
folio_size(folio))
to use clear_highpage + flush_dcache_folio?
If this also improves performance for other existing callers of
folio_zero_range(), then that's a positive outcome.
rm -f /tmp/test && fallocate -l 20G /tmp/test && fallocate -d -l 20G /
tmp/test && time fallocate -l 20G /tmp/test
1）mount always(2M folio)
with patch without patch
real 0m1.214s 0m1.111s
user 0m0.000s 0m0.000s
sys 0m1.210s 0m1.109s
With this patch, the performance does have regression,
folio_zero_range() is bad than clear_highpage + flush_dcache_folio
with patch
Oh, this should without patch since it uses clear_highpage,
99.95% 0.00% fallocate [kernel.vmlinux] [k] vfs_fallocate
vfs_fallocate
- shmem_fallocate
98.54% __pi_clear_page
- 1.38% shmem_get_folio_gfp
filemap_get_entry
and this one is with patch
without patch
99.89% 0.00% fallocate [kernel.vmlinux] [k] shmem_fallocate
shmem_fallocate
- shmem_get_folio_gfp
90.12% __memset
- 9.42% zero_user_segments.constprop.0
8.16% flush_dcache_page
1.03% flush_dcache_folio
2）mount never (4K folio)
real 0m3.159s 0m3.176s
user 0m0.000s 0m0.000s
sys 0m3.150s 0m3.169s
But with this patch, the performance is improved a little,
folio_zero_range() is better than clear_highpage + flush_dcache_folio
For 4K, the result is fluctuating, so maybe no different.
with patch
97.77% 3.37% fallocate [kernel.vmlinux] [k] shmem_fallocate
- 94.40% shmem_fallocate
- 93.70% shmem_get_folio_gfp
66.60% __memset
- 7.43% filemap_get_entry
3.49% xas_load
1.32% zero_user_segments.constprop.0
without patch
97.82% 3.22% fallocate [kernel.vmlinux] [k] shmem_fallocate
- 94.61% shmem_fallocate
68.18% __pi_clear_page
- 25.60% shmem_get_folio_gfp
- 7.64% filemap_get_entry
3.51% xas_load
if (sgp != SGP_WRITE && !folio_test_uptodate(folio)) {
- long i, n = folio_nr_pages(folio);
-
- for (i = 0; i < n; i++)
- clear_highpage(folio_page(folio, i));
- flush_dcache_folio(folio);
+ folio_zero_range(folio, 0, folio_size(folio));
folio_mark_uptodate(folio);
}
Thanks
Barry
