linux-kernel - Re: [PATCH v2 1/2] mm/damon/vaddr: Fix issue in damon_va_evenly_split

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20241022175452.42218-1-sj@kernel.org>
Date: Tue, 22 Oct 2024 10:54:52 -0700
From: SeongJae Park <sj@...nel.org>
To: Zheng Yejian <zhengyejian@...weicloud.com>
Cc: SeongJae Park <sj@...nel.org>,
	akpm@...ux-foundation.org,
	damon@...ts.linux.dev,
	foersleo@...zon.de,
	linux-kernel@...r.kernel.org,
	linux-mm@...ck.org,
	shakeel.butt@...ux.dev,
	sieberf@...zon.com,
	yeweihua4@...wei.com,
	brendanhiggins@...gle.com,
	davidgow@...gle.com,
	rmoar@...gle.com,
	linux-kselftest@...r.kernel.org,
	kunit-dev@...glegroups.com
Subject: Re: [PATCH v2 1/2] mm/damon/vaddr: Fix issue in damon_va_evenly_split_region()

Hi Zheng,


We Cc kunit folks for any DAMON kunit test changes, so I Cc-ed them.

On Tue, 22 Oct 2024 16:39:26 +0800 Zheng Yejian <zhengyejian@...weicloud.com> wrote:

> According to the logic of damon_va_evenly_split_region(), currently
> following split case would not meet the expectation:
> 
>   Suppose DAMON_MIN_REGION=0x1000,
>   Case: Split [0x0, 0x3000) into 2 pieces, then the result would be
>         acutually 3 regions:
>           [0x0, 0x1000), [0x1000, 0x2000), [0x2000, 0x3000)
>         but NOT the expected 2 regions:
>           [0x0, 0x1000), [0x1000, 0x3000) !!!
> 
> The root cause is that when calculating size of each split piece in
> damon_va_evenly_split_region():
> 
>   `sz_piece = ALIGN_DOWN(sz_orig / nr_pieces, DAMON_MIN_REGION);`
> 
> both the dividing and the ALIGN_DOWN may cause loss of precision,
> then each time split one piece of size 'sz_piece' from origin 'start' to
> 'end' would cause more pieces are split out than expected!!!
> 
> To fix it, count for each piece split and make sure no more than
> 'nr_pieces'. In addition, add above case into damon_test_split_evenly().
> 
> After this patch, damon-operations test passed:

Just for a clarification.  damon-operations test doesn't fail without this
patch.  This patch introduces two changes.  A new kunit test, and a bug fix.
Without the bug fix, the new kunit test fails.

I usually prefer separating test changes from fixes (introduc a fix first, and
then the test for it, to avoid unnecessary test failures).  But, given the
small size and the simplicity of the kunit change for this patch, I think
introducing it together with the fix is ok.

> 
>  # ./tools/testing/kunit/kunit.py run damon-operations
>  [...]
>  ============== damon-operations (6 subtests) ===============
>  [PASSED] damon_test_three_regions_in_vmas
>  [PASSED] damon_test_apply_three_regions1
>  [PASSED] damon_test_apply_three_regions2
>  [PASSED] damon_test_apply_three_regions3
>  [PASSED] damon_test_apply_three_regions4
>  [PASSED] damon_test_split_evenly
>  ================ [PASSED] damon-operations =================
> 
> Fixes: 3f49584b262c ("mm/damon: implement primitives for the virtual memory address spaces")
> Signed-off-by: Zheng Yejian <zhengyejian@...weicloud.com>

Reviewed-by: SeongJae Park <sj@...nel.org>


Thanks,
SJ

[...]