* [PATCH] selftests/mm/cow: Fix the incorrect error handling
@ 2025-03-11 2:37 Cyan Yang
2025-03-11 4:53 ` Dev Jain
2025-03-11 9:19 ` David Hildenbrand
0 siblings, 2 replies; 5+ messages in thread
From: Cyan Yang @ 2025-03-11 2:37 UTC (permalink / raw)
To: akpm, shuah, david; +Cc: linux-mm, linux-kselftest, linux-kernel, Cyan Yang
There are two error handlings did not check the correct return value.
This patch will fix them.
Fixes: f4b5fd6946e244cdedc3bbb9a1f24c8133b2077a ("selftests/vm: anon_cow: THP tests")
Signed-off-by: Cyan Yang <cyan.yang@sifive.com>
---
tools/testing/selftests/mm/cow.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
index 9446673645eb..16fcadc090a4 100644
--- a/tools/testing/selftests/mm/cow.c
+++ b/tools/testing/selftests/mm/cow.c
@@ -876,13 +876,13 @@ static void do_run_with_thp(test_fn fn, enum thp_run thp_run, size_t thpsize)
mremap_size = thpsize / 2;
mremap_mem = mmap(NULL, mremap_size, PROT_NONE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
- if (mem == MAP_FAILED) {
+ if (mremap_mem == MAP_FAILED) {
ksft_test_result_fail("mmap() failed\n");
goto munmap;
}
tmp = mremap(mem + mremap_size, mremap_size, mremap_size,
MREMAP_MAYMOVE | MREMAP_FIXED, mremap_mem);
- if (tmp != mremap_mem) {
+ if (tmp == MAP_FAILED) {
ksft_test_result_fail("mremap() failed\n");
goto munmap;
}
--
2.39.5 (Apple Git-154)
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] selftests/mm/cow: Fix the incorrect error handling
2025-03-11 2:37 [PATCH] selftests/mm/cow: Fix the incorrect error handling Cyan Yang
@ 2025-03-11 4:53 ` Dev Jain
2025-03-11 9:19 ` David Hildenbrand
1 sibling, 0 replies; 5+ messages in thread
From: Dev Jain @ 2025-03-11 4:53 UTC (permalink / raw)
To: Cyan Yang, akpm, shuah, david; +Cc: linux-mm, linux-kselftest, linux-kernel
On 11/03/25 8:07 am, Cyan Yang wrote:
> There are two error handlings did not check the correct return value.
> This patch will fix them.
>
> Fixes: f4b5fd6946e244cdedc3bbb9a1f24c8133b2077a ("selftests/vm: anon_cow: THP tests")
> Signed-off-by: Cyan Yang <cyan.yang@sifive.com>
> ---
> tools/testing/selftests/mm/cow.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
> index 9446673645eb..16fcadc090a4 100644
> --- a/tools/testing/selftests/mm/cow.c
> +++ b/tools/testing/selftests/mm/cow.c
> @@ -876,13 +876,13 @@ static void do_run_with_thp(test_fn fn, enum thp_run thp_run, size_t thpsize)
> mremap_size = thpsize / 2;
> mremap_mem = mmap(NULL, mremap_size, PROT_NONE,
> MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> - if (mem == MAP_FAILED) {
> + if (mremap_mem == MAP_FAILED) {
> ksft_test_result_fail("mmap() failed\n");
> goto munmap;
> }
> tmp = mremap(mem + mremap_size, mremap_size, mremap_size,
> MREMAP_MAYMOVE | MREMAP_FIXED, mremap_mem);
> - if (tmp != mremap_mem) {
This is fine. We are checking whether we were able to mremap tmp to
mremap_mem.
> + if (tmp == MAP_FAILED) {
> ksft_test_result_fail("mremap() failed\n");
> goto munmap;
> }
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] selftests/mm/cow: Fix the incorrect error handling
2025-03-11 2:37 [PATCH] selftests/mm/cow: Fix the incorrect error handling Cyan Yang
2025-03-11 4:53 ` Dev Jain
@ 2025-03-11 9:19 ` David Hildenbrand
2025-03-11 10:38 ` Cyan Yang
1 sibling, 1 reply; 5+ messages in thread
From: David Hildenbrand @ 2025-03-11 9:19 UTC (permalink / raw)
To: Cyan Yang, akpm, shuah; +Cc: linux-mm, linux-kselftest, linux-kernel
On 11.03.25 03:37, Cyan Yang wrote:
> There are two error handlings did not check the correct return value.
> This patch will fix them.
>
> Fixes: f4b5fd6946e244cdedc3bbb9a1f24c8133b2077a ("selftests/vm: anon_cow: THP tests")
> Signed-off-by: Cyan Yang <cyan.yang@sifive.com>
> ---
> tools/testing/selftests/mm/cow.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
> index 9446673645eb..16fcadc090a4 100644
> --- a/tools/testing/selftests/mm/cow.c
> +++ b/tools/testing/selftests/mm/cow.c
> @@ -876,13 +876,13 @@ static void do_run_with_thp(test_fn fn, enum thp_run thp_run, size_t thpsize)
> mremap_size = thpsize / 2;
> mremap_mem = mmap(NULL, mremap_size, PROT_NONE,
> MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> - if (mem == MAP_FAILED) {
> + if (mremap_mem == MAP_FAILED) {
> ksft_test_result_fail("mmap() failed\n");
> goto munmap;
> }
Yes, that check is wrong.
> tmp = mremap(mem + mremap_size, mremap_size, mremap_size,
> MREMAP_MAYMOVE | MREMAP_FIXED, mremap_mem);
> - if (tmp != mremap_mem) {
> + if (tmp == MAP_FAILED) {
> ksft_test_result_fail("mremap() failed\n");
> goto munmap;
> }
As Dev says, this one is just fine. Leave it as it is.
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] selftests/mm/cow: Fix the incorrect error handling
2025-03-11 9:19 ` David Hildenbrand
@ 2025-03-11 10:38 ` Cyan Yang
2025-03-11 19:48 ` David Hildenbrand
0 siblings, 1 reply; 5+ messages in thread
From: Cyan Yang @ 2025-03-11 10:38 UTC (permalink / raw)
To: David Hildenbrand; +Cc: akpm, shuah, linux-mm, linux-kselftest, linux-kernel
On Tue, Mar 11, 2025 at 10:19:32AM +0100, David Hildenbrand wrote:
> On 11.03.25 03:37, Cyan Yang wrote:
> > There are two error handlings did not check the correct return value.
> > This patch will fix them.
> >
> > Fixes: f4b5fd6946e244cdedc3bbb9a1f24c8133b2077a ("selftests/vm: anon_cow: THP tests")
> > Signed-off-by: Cyan Yang <cyan.yang@sifive.com>
> > ---
> > tools/testing/selftests/mm/cow.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
> > index 9446673645eb..16fcadc090a4 100644
> > --- a/tools/testing/selftests/mm/cow.c
> > +++ b/tools/testing/selftests/mm/cow.c
> > @@ -876,13 +876,13 @@ static void do_run_with_thp(test_fn fn, enum thp_run thp_run, size_t thpsize)
> > mremap_size = thpsize / 2;
> > mremap_mem = mmap(NULL, mremap_size, PROT_NONE,
> > MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > - if (mem == MAP_FAILED) {
> > + if (mremap_mem == MAP_FAILED) {
> > ksft_test_result_fail("mmap() failed\n");
> > goto munmap;
> > }
>
> Yes, that check is wrong.
>
> > tmp = mremap(mem + mremap_size, mremap_size, mremap_size,
> > MREMAP_MAYMOVE | MREMAP_FIXED, mremap_mem);
> > - if (tmp != mremap_mem) {
> > + if (tmp == MAP_FAILED) {
> > ksft_test_result_fail("mremap() failed\n");
> > goto munmap;
> > }
>
> As Dev says, this one is just fine. Leave it as it is.
>
Thank you for the review.
I agree with you and Dev this is just fine. The reason I prefer to modify it is
- usually caller checks the return value directly and "tmp == mremap_mem"
should be determined by "mremap".
If you still prefer to leave it as it is, I will send out the v2 to remove it.
> --
> Cheers,
>
> David / dhildenb
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] selftests/mm/cow: Fix the incorrect error handling
2025-03-11 10:38 ` Cyan Yang
@ 2025-03-11 19:48 ` David Hildenbrand
0 siblings, 0 replies; 5+ messages in thread
From: David Hildenbrand @ 2025-03-11 19:48 UTC (permalink / raw)
To: Cyan Yang; +Cc: akpm, shuah, linux-mm, linux-kselftest, linux-kernel
On 11.03.25 11:38, Cyan Yang wrote:
> On Tue, Mar 11, 2025 at 10:19:32AM +0100, David Hildenbrand wrote:
>> On 11.03.25 03:37, Cyan Yang wrote:
>>> There are two error handlings did not check the correct return value.
>>> This patch will fix them.
>>>
>>> Fixes: f4b5fd6946e244cdedc3bbb9a1f24c8133b2077a ("selftests/vm: anon_cow: THP tests")
>>> Signed-off-by: Cyan Yang <cyan.yang@sifive.com>
>>> ---
>>> tools/testing/selftests/mm/cow.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
>>> index 9446673645eb..16fcadc090a4 100644
>>> --- a/tools/testing/selftests/mm/cow.c
>>> +++ b/tools/testing/selftests/mm/cow.c
>>> @@ -876,13 +876,13 @@ static void do_run_with_thp(test_fn fn, enum thp_run thp_run, size_t thpsize)
>>> mremap_size = thpsize / 2;
>>> mremap_mem = mmap(NULL, mremap_size, PROT_NONE,
>>> MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>>> - if (mem == MAP_FAILED) {
>>> + if (mremap_mem == MAP_FAILED) {
>>> ksft_test_result_fail("mmap() failed\n");
>>> goto munmap;
>>> }
>>
>> Yes, that check is wrong.
>>
>>> tmp = mremap(mem + mremap_size, mremap_size, mremap_size,
>>> MREMAP_MAYMOVE | MREMAP_FIXED, mremap_mem);
>>> - if (tmp != mremap_mem) {
>>> + if (tmp == MAP_FAILED) {
>>> ksft_test_result_fail("mremap() failed\n");
>>> goto munmap;
>>> }
>>
>> As Dev says, this one is just fine. Leave it as it is.
>>
>
> Thank you for the review.
>
> I agree with you and Dev this is just fine. The reason I prefer to modify it is
> - usually caller checks the return value directly and "tmp == mremap_mem"
> should be determined by "mremap".
>
> If you still prefer to leave it as it is, I will send out the v2 to remove it.
Yes, leave it as is. This does not belong in this fix for a real error
checking problem. Thanks!
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-03-11 19:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-11 2:37 [PATCH] selftests/mm/cow: Fix the incorrect error handling Cyan Yang
2025-03-11 4:53 ` Dev Jain
2025-03-11 9:19 ` David Hildenbrand
2025-03-11 10:38 ` Cyan Yang
2025-03-11 19:48 ` David Hildenbrand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox