2017-06-19 13:44:43

by Sumit Semwal

[permalink] [raw]
Subject: [PATCH 2/2] selftests: lib: prime_numbers: update presence check

The test for prime numbers doesn't differentiate between missing
prime_numbers.ko and failure in prime_numbers.ko.

Update it to check for presence of the file itself to skip, therefore
correctly exercising the test failure case.

Signed-off-by: Sumit Semwal <[email protected]>
---
tools/testing/selftests/lib/prime_numbers.sh | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/lib/prime_numbers.sh b/tools/testing/selftests/lib/prime_numbers.sh
index da4cbcd766f5..492f0dcdf088 100755
--- a/tools/testing/selftests/lib/prime_numbers.sh
+++ b/tools/testing/selftests/lib/prime_numbers.sh
@@ -1,8 +1,9 @@
#!/bin/sh
# Checks fast/slow prime_number generation for inconsistencies

-if ! /sbin/modprobe -q -r prime_numbers; then
- echo "prime_numbers: [SKIP]"
+if ! find /lib/modules/$(uname -r) -type f -name prime_numbers.ko | grep -q .;
+then
+ echo "prime_numbers: prime_numbers.ko not found: [SKIP]"
exit 77
fi

--
2.7.4


2017-06-19 13:52:03

by Chris Wilson

[permalink] [raw]
Subject: Re: [PATCH 2/2] selftests: lib: prime_numbers: update presence check

Quoting Sumit Semwal (2017-06-19 14:44:32)
> The test for prime numbers doesn't differentiate between missing
> prime_numbers.ko and failure in prime_numbers.ko.
>
> Update it to check for presence of the file itself to skip, therefore
> correctly exercising the test failure case.

modprobe -r shouldn't be executing the module? But you still need to
unload the module before you can load it with the selftest module
parameters. If you can't unload the module due to an earlier failure,
you cannot discern whether or not the module itself is at fault, so
still want to SKIP.
-Chris

2017-06-19 14:46:43

by Sumit Semwal

[permalink] [raw]
Subject: Re: [PATCH 2/2] selftests: lib: prime_numbers: update presence check

Hi Chris,

On 19 June 2017 at 19:21, Chris Wilson <[email protected]> wrote:
> Quoting Sumit Semwal (2017-06-19 14:44:32)
>> The test for prime numbers doesn't differentiate between missing
>> prime_numbers.ko and failure in prime_numbers.ko.
>>
>> Update it to check for presence of the file itself to skip, therefore
>> correctly exercising the test failure case.
>
> modprobe -r shouldn't be executing the module? But you still need to
> unload the module before you can load it with the selftest module
> parameters. If you can't unload the module due to an earlier failure,
> you cannot discern whether or not the module itself is at fault, so
> still want to SKIP.

My bad here: I missed the '-r' in the first modprobe.

I am wondering if 'modprobe -q -n' won't suffice, as it is a dry-run
only, but will still search for the module? Unless of course, there's
something specific about '-q -r' that seems better still?

Shuah,
Could you please disregard this patch, and the other patch I sent:
will send out a new version soon.

> -Chris

Best,
Sumit.

2017-06-19 15:11:57

by Sumit Semwal

[permalink] [raw]
Subject: Re: [PATCH 2/2] selftests: lib: prime_numbers: update presence check

On 19 June 2017 at 20:27, Chris Wilson <[email protected]> wrote:
> Quoting Sumit Semwal (2017-06-19 15:46:20)
>> Hi Chris,
>>
>> On 19 June 2017 at 19:21, Chris Wilson <[email protected]> wrote:
>> > Quoting Sumit Semwal (2017-06-19 14:44:32)
>> >> The test for prime numbers doesn't differentiate between missing
>> >> prime_numbers.ko and failure in prime_numbers.ko.
>> >>
>> >> Update it to check for presence of the file itself to skip, therefore
>> >> correctly exercising the test failure case.
>> >
>> > modprobe -r shouldn't be executing the module? But you still need to
>> > unload the module before you can load it with the selftest module
>> > parameters. If you can't unload the module due to an earlier failure,
>> > you cannot discern whether or not the module itself is at fault, so
>> > still want to SKIP.
>>
>> My bad here: I missed the '-r' in the first modprobe.
>>
>> I am wondering if 'modprobe -q -n' won't suffice, as it is a dry-run
>> only, but will still search for the module? Unless of course, there's
>> something specific about '-q -r' that seems better still?
>
> I think there are two things to be tested here, both causing a SKIP.
>
> - If the module doesn't exist at all; modprobe -q -n seems sensible for
> querying its existence.
>
> - If the module cannot be [un]loaded; for which I was using the
> modprobe -q -r. If we can't unload the module, then we can't test :)

:) Right; then the question is, for prime_numbers.ko, do we need to
differentiate between these 2 SKIPs?

The unloading of the prime_numbers module before running the test is
required since it isn't a standalone test module - unlike the
test_bitmap and test_printf ones.

So then, for prime_numbers: if distinguishing between the two cases
you mentioned above isn't important, we can just keep your original
code. If it is important to distinguish, I can add the -q -n test to
query existence separately.

For test_bitmap and test_printf tests, I think I will just go ahead
with -q -n itself, since we can assume that the test modules will only
be loaded/unloaded via these tests I guess?

>
> -Chris

Best,
Sumit.

2017-06-20 13:09:18

by Sumit Semwal

[permalink] [raw]
Subject: Re: [PATCH 2/2] selftests: lib: prime_numbers: update presence check

Hi Chris,

On 20 June 2017 at 17:17, Chris Wilson <[email protected]> wrote:
> Quoting Sumit Semwal (2017-06-19 16:11:33)
>> On 19 June 2017 at 20:27, Chris Wilson <[email protected]> wrote:
>> > Quoting Sumit Semwal (2017-06-19 15:46:20)
>> >> Hi Chris,
>> >>
>> >> On 19 June 2017 at 19:21, Chris Wilson <[email protected]> wrote:
>> >> > Quoting Sumit Semwal (2017-06-19 14:44:32)
>> >> >> The test for prime numbers doesn't differentiate between missing
>> >> >> prime_numbers.ko and failure in prime_numbers.ko.
>> >> >>
>> >> >> Update it to check for presence of the file itself to skip, therefore
>> >> >> correctly exercising the test failure case.
>> >> >
>> >> > modprobe -r shouldn't be executing the module? But you still need to
>> >> > unload the module before you can load it with the selftest module
>> >> > parameters. If you can't unload the module due to an earlier failure,
>> >> > you cannot discern whether or not the module itself is at fault, so
>> >> > still want to SKIP.
>> >>
>> >> My bad here: I missed the '-r' in the first modprobe.
>> >>
>> >> I am wondering if 'modprobe -q -n' won't suffice, as it is a dry-run
>> >> only, but will still search for the module? Unless of course, there's
>> >> something specific about '-q -r' that seems better still?
>> >
>> > I think there are two things to be tested here, both causing a SKIP.
>> >
>> > - If the module doesn't exist at all; modprobe -q -n seems sensible for
>> > querying its existence.
>> >
>> > - If the module cannot be [un]loaded; for which I was using the
>> > modprobe -q -r. If we can't unload the module, then we can't test :)
>>
>> :) Right; then the question is, for prime_numbers.ko, do we need to
>> differentiate between these 2 SKIPs?
>
> I don't see a downside to being verbose here. It both helps explain the
> test and alert the user about the remedy if desired.
>
>> The unloading of the prime_numbers module before running the test is
>> required since it isn't a standalone test module - unlike the
>> test_bitmap and test_printf ones.
>>
>> So then, for prime_numbers: if distinguishing between the two cases
>> you mentioned above isn't important, we can just keep your original
>> code. If it is important to distinguish, I can add the -q -n test to
>> query existence separately.
>>
>> For test_bitmap and test_printf tests, I think I will just go ahead
>> with -q -n itself, since we can assume that the test modules will only
>> be loaded/unloaded via these tests I guess?
>
> Yes. For that style of standalone test module, if the test module is still
> around it is probably for a reason :)

:) These test cases seem to try to load the module (and hence run the
tests), and then try to unload the module right after, so I think we
should be ok there.

> -Chris