Re: [PATCH v2 3/4] selftests/mm: Report unique test names for each cow test

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]

On Tue, Jun 03, 2025 at 02:51:45PM +0200, David Hildenbrand wrote:
> On 27.05.25 18:04, Mark Brown wrote:

> >   		ret = mprotect(mem, size, PROT_READ);
> > -		ret |= mprotect(mem, size, PROT_READ|PROT_WRITE);
> >   		if (ret) {

> Not sure if that change is really required: if the second mprotect succeeds,
> errno should not be updated. At least if my memory is correct :)

> Same applies to similar cases below.

I thought about checking to see if that was guaranteed to be the case,
then I thought that if that wasn't clear to me right now without
checking it probably also wasn't going to be obvious to future readers
so it was better to just write something clear.  Previously we didn't
report errno so it didn't matter.

> >   	} else {
> > -		ksft_test_result_fail("Leak from parent into child\n");

> Same here and in other cases below (I probably didn't catch all).

> We should log that somehow to indicate what exactly is going wrong, likely
> using ksft_print_msg().

Can you send a patch with the logging that you think would be clear
please?  I dropped these because they just seemed to be reporting the
overall point of the test, unlike the cases where we ran into some error
during the setup and didn't actually manage to perform the test we were
trying to do.  Perhaps the tests should be renamed.

> >   	tmp = malloc(size);
> >   	if (!tmp) {
> > -		ksft_test_result_fail("malloc() failed\n");
> > +		ksft_print_msg("malloc() failed\n");

> perror?

malloc() can only set one errno.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]