Re: [PATCH v2 03/38] drm/i915: Add some selftests for sg_table manipulation

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 01/02/2017 11:34, Chris Wilson wrote:
> On Wed, Feb 01, 2017 at 11:17:39AM +0000, Tvrtko Ursulin wrote:

>>> +
>>> +			for (npages = npages_funcs; *npages; npages++) {
>>> +				prandom_seed_state(&prng,
>>> +						   i915_selftest.random_seed);
>>> +				if (!alloc_table(&pt, sz, sz, *npages, &prng))
>>> +					return 0; /* out of memory, give up */
>>
>> You don't have skip status? Sounds not ideal to silently abort.
>
> It runs until we use all physical memory, if left to its own devices. It's
> not a skip if we have already completed some tests. ENOMEM of the test
> setup itself is not what I'm testing for here, the test is for the
> iterators.

But suppose you mess up the test so the starting condition asks for 
impossible amount of memory but the test claims it passed. I don't think 
that is a good behaviour.

>>> +static int igt_sg_trim(void *ignored)
>>> +{
>>> +	IGT_TIMEOUT(end_time);
>>> +	const unsigned long max = PAGE_SIZE; /* not prime! */
>>> +	struct pfn_table pt;
>>> +	unsigned long prime;
>>> +
>>> +	for_each_prime_number(prime, max) {
>>> +		const npages_fn_t *npages;
>>> +		int err;
>>> +
>>> +		for (npages = npages_funcs; *npages; npages++) {
>>> +			struct rnd_state prng;
>>> +
>>> +			prandom_seed_state(&prng, i915_selftest.random_seed);
>>> +			if (!alloc_table(&pt, prime, max, *npages, &prng))
>>> +				return 0; /* out of memory, give up */
>>> +
>>> +			err = 0;
>>> +			if (i915_sg_trim(&pt.st)) {
>>> +				if (pt.st.orig_nents != prime ||
>>> +				    pt.st.nents != prime) {
>>> +					pr_err("i915_sg_trim failed (nents %u, orig_nents %u), expected %lu\n",
>>> +					       pt.st.nents, pt.st.orig_nents, prime);
>>> +					err = -EINVAL;
>>> +				} else {
>>> +					prandom_seed_state(&prng,
>>> +							   i915_selftest.random_seed);
>>> +					err = expect_pfn_sgtable(&pt,
>>> +								 *npages, &prng,
>>> +								 "i915_sg_trim",
>>> +								 end_time);
>>> +				}
>>> +			}
>>
>> Similar to alloc_table failures above - no log or action when
>> i915_sg_trim fails due out of memory?
>
> No, simply because that's an expected and acceptable result. The
> question should be whether we always want to check after sg_trim.

Same as above really, I think that creates a big doubt in the test output.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx