Re: [PATCH i-g-t] tests/gem_render_linear_blits: Increase min swap required

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 29, 2015 at 09:52:31AM +0000, Morton, Derek J wrote:
> 
> >
> >
> >-----Original Message-----
> >From: Chris Wilson [mailto:chris@xxxxxxxxxxxxxxxxxx] 
> >Sent: Wednesday, July 29, 2015 9:54 AM
> >To: Morton, Derek J
> >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Wood, Thomas
> >Subject: Re:  [PATCH i-g-t] tests/gem_render_linear_blits: Increase min swap required
> >
> >On Wed, Jul 29, 2015 at 09:30:21AM +0100, Derek Morton wrote:
> >> The swap-thrash subtest had a requirement that swap memory be present 
> >> but no minimum amount was specified. The subtest allowed for half the 
> >> total swap memory for overhead. Some android systems have a only a 
> >> small amount of swap space and half this was not enough resulting in 
> >> OOM errors. It was not possible to determine the exact amount of 
> >> memory the test would require in all configurations to guarentee swap 
> >> memory would be used but not trigger an OOM error.
> >> As a minimum reccomended swap partition on Linux is 256Mb the subtest 
> >> was updated to require this.
> >> 
> >> Also fixed a couple of small memory leaks.
> >> 
> >> Signed-off-by: Derek Morton <derek.j.morton@xxxxxxxxx>
> >> ---
> >>  tests/gem_render_linear_blits.c | 10 +++++++++-
> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/tests/gem_render_linear_blits.c 
> >> b/tests/gem_render_linear_blits.c index f83c6d4..5dd210d 100644
> >> --- a/tests/gem_render_linear_blits.c
> >> +++ b/tests/gem_render_linear_blits.c
> >> @@ -184,6 +184,9 @@ static void run_test (int fd, int count)
> >>  	}
> >>  	intel_batchbuffer_free(batch);
> >>  	drm_intel_bufmgr_destroy(bufmgr);
> >> +
> >> +	free(bo);
> >> +	free(start_val);
> >>  }
> >>  
> >>  igt_main
> >> @@ -210,7 +213,12 @@ igt_main
> >>  
> >>  	igt_subtest("swap-thrash") {
> >>  		uint64_t swap_mb = intel_get_total_swap_mb();
> >> -		igt_require(swap_mb > 0);
> >> +		/* The calculation of count allows 1/2 the swap memory as
> >> +		   overhead. However on Android systems with a very small swap
> >> +		   partition this is not enough resulting in OOM errors.
> >> +		   As 256Mb is a minimum recomended size for a swap partition
> >> +		   on Linux, skip the subtest if less than this. */
> >> +		igt_require(swap_mb > 255);
> >>  		count = ((intel_get_avail_ram_mb() + (swap_mb / 2)) * 1024*1024) / SIZE;
> >>  		intel_require_memory(count, SIZE, CHECK_RAM | CHECK_SWAP);
> >
> >Surely fixing intel_require_memory(CHECK_SWAP) (adding the slop of 256MiB swap or somesuch) would be better?
> 
> I don't think so. igt_require(swap_mb > 255) is simple and easy to understand. intel_require_memory() is doing a crude count+SIZE < ram+swap. It does not take into account any other memory the test might be using or allow any overhead for additional memory used elsewhere in the system. I tried several scenarios with the HW I have. 2Gb RAM and no swap (removed the +swap code) required count = ... / SIZE + 50Kb to run without any OOM errors (40Kb was intermittent). On a 1Gb system with 99Mb of Swap it would quickly OOM even though it should have an extra 50Mb (swap / 2) of free memory on top of what the +50Kb was giving. I do not see any way of accurately calculating count to be a value that guarantees swap memory will be used without going OOM when the amount of swap is small and the amount of overhead RAM used is not known accurately. Hence just skipping the subtest in that situation.
> In fact the intel_require_memory() as it stands is pointless and could even be removed as count is calculated to be a value where intel_require_memory() will never trigger an assert.

So basically, you have a kernel bug you wish to ignore?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux