diff mbox

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

Message ID 1438158621-16944-1-git-send-email-derek.j.morton@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Derek Morton July 29, 2015, 8:30 a.m. UTC
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@intel.com>
---
 tests/gem_render_linear_blits.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Chris Wilson July 29, 2015, 8:53 a.m. UTC | #1
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@intel.com>
> ---
>  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?
-Chris
Derek Morton July 29, 2015, 9:52 a.m. UTC | #2
>
>
>-----Original Message-----
>From: Chris Wilson [mailto:chris@chris-wilson.co.uk] 
>Sent: Wednesday, July 29, 2015 9:54 AM
>To: Morton, Derek J
>Cc: intel-gfx@lists.freedesktop.org; Wood, Thomas
>Subject: Re: [Intel-gfx] [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@intel.com>
>> ---
>>  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.

//Derek

>-Chris
>
>--
>Chris Wilson, Intel Open Source Technology Centre
>
Chris Wilson July 29, 2015, 9:56 a.m. UTC | #3
On Wed, Jul 29, 2015 at 09:52:31AM +0000, Morton, Derek J wrote:
> 
> >
> >
> >-----Original Message-----
> >From: Chris Wilson [mailto:chris@chris-wilson.co.uk] 
> >Sent: Wednesday, July 29, 2015 9:54 AM
> >To: Morton, Derek J
> >Cc: intel-gfx@lists.freedesktop.org; Wood, Thomas
> >Subject: Re: [Intel-gfx] [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@intel.com>
> >> ---
> >>  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
tim.gore@intel.com July 29, 2015, 1:10 p.m. UTC | #4
Tim Gore 
Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ

> -----Original Message-----

> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf

> Of Chris Wilson

> Sent: Wednesday, July 29, 2015 10:56 AM

> To: Morton, Derek J

> Cc: intel-gfx@lists.freedesktop.org; Wood, Thomas

> Subject: Re: [Intel-gfx] [PATCH i-g-t] tests/gem_render_linear_blits: Increase

> min swap required

> 

> On Wed, Jul 29, 2015 at 09:52:31AM +0000, Morton, Derek J wrote:

> >

> > >

> > >

> > >-----Original Message-----

> > >From: Chris Wilson [mailto:chris@chris-wilson.co.uk]

> > >Sent: Wednesday, July 29, 2015 9:54 AM

> > >To: Morton, Derek J

> > >Cc: intel-gfx@lists.freedesktop.org; Wood, Thomas

> > >Subject: Re: [Intel-gfx] [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@intel.com>

> > >> ---

> > >>  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


I don’t see how this implies a kernel bug. It seems like a test problem (my
subtest as it happens). I was unaware of Android systems with small swap
partitions (or indeed any swap at all). Not sure I can understand the logic of
such a tiny swap partition but given the situation, unless we can accurately
characterise the memory usage of the test in advance then we have to
either skip the test for small swap, or try to monitor memory usage in an
ongoing way during the test.

 Tim Gore
> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org

> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Chris Wilson July 29, 2015, 1:15 p.m. UTC | #5
On Wed, Jul 29, 2015 at 01:10:23PM +0000, Gore, Tim wrote:
> I don’t see how this implies a kernel bug. It seems like a test problem (my
> subtest as it happens). I was unaware of Android systems with small swap
> partitions (or indeed any swap at all). Not sure I can understand the logic of
> such a tiny swap partition but given the situation, unless we can accurately
> characterise the memory usage of the test in advance then we have to
> either skip the test for small swap, or try to monitor memory usage in an
> ongoing way during the test.

If the system has enough resources to run the test (that is enough
physical to run an individual batch plus enough swap to hold the rest),
then the test must not oom.
-Chris
tim.gore@intel.com July 29, 2015, 2:38 p.m. UTC | #6
> -----Original Message-----

> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]

> Sent: Wednesday, July 29, 2015 2:16 PM

> To: Gore, Tim

> Cc: Morton, Derek J; intel-gfx@lists.freedesktop.org; Wood, Thomas

> Subject: Re: [Intel-gfx] [PATCH i-g-t] tests/gem_render_linear_blits: Increase

> min swap required

> 

> On Wed, Jul 29, 2015 at 01:10:23PM +0000, Gore, Tim wrote:

> > I don’t see how this implies a kernel bug. It seems like a test

> > problem (my subtest as it happens). I was unaware of Android systems

> > with small swap partitions (or indeed any swap at all). Not sure I can

> > understand the logic of such a tiny swap partition but given the

> > situation, unless we can accurately characterise the memory usage of

> > the test in advance then we have to either skip the test for small

> > swap, or try to monitor memory usage in an ongoing way during the test.

> 

> If the system has enough resources to run the test (that is enough physical to

> run an individual batch plus enough swap to hold the rest), then the test

> must not oom.

> -Chris

> 

> --

> Chris Wilson, Intel Open Source Technology Centre


Maybe my test is flawed. To force the use of swap memory I calculate count
In order to fill (available-ram + swap/2) with buffer objects. So the physical ram
gets completely filled and then further buffer objects are created to force
the use of swap. Because the swap partition is so much smaller than the ram
in this system, a relatively small error in calculating count (due to overheads)
leads to the swap filling up and oom. 

    Tim 

Tim Gore 
Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ
Dave Gordon July 29, 2015, 3:14 p.m. UTC | #7
On 29/07/15 14:15, Chris Wilson wrote:
> On Wed, Jul 29, 2015 at 01:10:23PM +0000, Gore, Tim wrote:
>> I don’t see how this implies a kernel bug. It seems like a test problem (my
>> subtest as it happens). I was unaware of Android systems with small swap
>> partitions (or indeed any swap at all). Not sure I can understand the logic of
>> such a tiny swap partition but given the situation, unless we can accurately
>> characterise the memory usage of the test in advance then we have to
>> either skip the test for small swap, or try to monitor memory usage in an
>> ongoing way during the test.
>
> If the system has enough resources to run the test (that is enough
> physical to run an individual batch plus enough swap to hold the rest),
> then the test must not oom.
> -Chris

The test is deliberately attempting to use enough memory to force some 
stuff out to swap, while not hitting a total OOM. That can be a very 
narrow window when the swapspace is small; and the test just guesses in 
advance how much will do the trick rather than gradually increasing its 
demands until it detects that stuff is being swapped.

So not a kernel bug, but something of a failure in the implementation of 
the test. Is there an interface it could use to /detect and measure/ 
when stuff is swapped out?

.Dave.
Chris Wilson July 29, 2015, 3:20 p.m. UTC | #8
On Wed, Jul 29, 2015 at 04:14:55PM +0100, Dave Gordon wrote:
> On 29/07/15 14:15, Chris Wilson wrote:
> >On Wed, Jul 29, 2015 at 01:10:23PM +0000, Gore, Tim wrote:
> >>I don’t see how this implies a kernel bug. It seems like a test problem (my
> >>subtest as it happens). I was unaware of Android systems with small swap
> >>partitions (or indeed any swap at all). Not sure I can understand the logic of
> >>such a tiny swap partition but given the situation, unless we can accurately
> >>characterise the memory usage of the test in advance then we have to
> >>either skip the test for small swap, or try to monitor memory usage in an
> >>ongoing way during the test.
> >
> >If the system has enough resources to run the test (that is enough
> >physical to run an individual batch plus enough swap to hold the rest),
> >then the test must not oom.
> >-Chris
> 
> The test is deliberately attempting to use enough memory to force
> some stuff out to swap, while not hitting a total OOM. That can be a
> very narrow window when the swapspace is small; and the test just
> guesses in advance how much will do the trick rather than gradually
> increasing its demands until it detects that stuff is being swapped.
> 
> So not a kernel bug, but something of a failure in the

Pardon? Which part of we have enough physical and virtual to complete
the test, but an oom is triggered instead is an incorrect assumption?
-Chris
tim.gore@intel.com July 29, 2015, 3:34 p.m. UTC | #9
> -----Original Message-----

> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]

> Sent: Wednesday, July 29, 2015 4:20 PM

> To: Gordon, David S

> Cc: Gore, Tim; Morton, Derek J; intel-gfx@lists.freedesktop.org; Wood,

> Thomas

> Subject: Re: [Intel-gfx] [PATCH i-g-t] tests/gem_render_linear_blits: Increase

> min swap required

> 

> On Wed, Jul 29, 2015 at 04:14:55PM +0100, Dave Gordon wrote:

> > On 29/07/15 14:15, Chris Wilson wrote:

> > >On Wed, Jul 29, 2015 at 01:10:23PM +0000, Gore, Tim wrote:

> > >>I don’t see how this implies a kernel bug. It seems like a test

> > >>problem (my subtest as it happens). I was unaware of Android systems

> > >>with small swap partitions (or indeed any swap at all). Not sure I

> > >>can understand the logic of such a tiny swap partition but given the

> > >>situation, unless we can accurately characterise the memory usage of

> > >>the test in advance then we have to either skip the test for small

> > >>swap, or try to monitor memory usage in an ongoing way during the test.

> > >

> > >If the system has enough resources to run the test (that is enough

> > >physical to run an individual batch plus enough swap to hold the

> > >rest), then the test must not oom.

> > >-Chris

> >

> > The test is deliberately attempting to use enough memory to force some

> > stuff out to swap, while not hitting a total OOM. That can be a very

> > narrow window when the swapspace is small; and the test just guesses

> > in advance how much will do the trick rather than gradually increasing

> > its demands until it detects that stuff is being swapped.

> >

> > So not a kernel bug, but something of a failure in the

> 

> Pardon? Which part of we have enough physical and virtual to complete the

> test, but an oom is triggered instead is an incorrect assumption?

> -Chris

> 

> --

> Chris Wilson, Intel Open Source Technology Centre


"we have enough physical and virtual to complete the test" - is the
incorrect assumption. We would have enough space if the test were
able to calculate memory usage accurately enough, but the way the
test calculates memory usage is too imprecise, we don’t allow for any
overhead at all. If overhead > (swap/2 - oom threshold) then we're dead.

 Tim

Tim Gore
Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ
Derek Morton July 29, 2015, 3:47 p.m. UTC | #10
>

>

>-----Original Message-----

>From: Gordon, David S 

>Sent: Wednesday, July 29, 2015 4:15 PM

>To: Chris Wilson; Gore, Tim; Morton, Derek J; intel-gfx@lists.freedesktop.org; Wood, Thomas

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

>

>On 29/07/15 14:15, Chris Wilson wrote:

>> On Wed, Jul 29, 2015 at 01:10:23PM +0000, Gore, Tim wrote:

>>> I don’t see how this implies a kernel bug. It seems like a test 

>>> problem (my subtest as it happens). I was unaware of Android systems 

>>> with small swap partitions (or indeed any swap at all). Not sure I 

>>> can understand the logic of such a tiny swap partition but given the 

>>> situation, unless we can accurately characterise the memory usage of 

>>> the test in advance then we have to either skip the test for small 

>>> swap, or try to monitor memory usage in an ongoing way during the test.

>>

>> If the system has enough resources to run the test (that is enough 

>> physical to run an individual batch plus enough swap to hold the 

>> rest), then the test must not oom.

>> -Chris

>

>The test is deliberately attempting to use enough memory to force some stuff out to swap, while not hitting a total OOM. That can be a very narrow window when the swapspace is small; and the test just guesses in advance how much will do the trick rather than gradually increasing its demands until it detects that stuff is being swapped.

>

>So not a kernel bug, but something of a failure in the implementation of the test. Is there an interface it could use to /detect and measure/ when stuff is swapped out?


And there is also the problem that when stuff is swapped in and out that we have no control over what goes into the swap memory. It may well be for some other process that is asleep and be nothing to do with the running test.

I could rewrite the subtest to allocate a big lump of memory to fill physical memory, then allocate the swap/2 worth of buffer objects but there is no guarantee the buffer objects will go into swap rather than something else.

Also the actual workings of the OOM killer are a black art. When exactly does it start killing processes. There does not appear to be a fixed line that could be monitored with intel_get_avail_ram_mb(). And anyway how does intel_get_avail_ram_mb() interact with swap memory? Does it include swap memory? In which case the subtest would always fail. If not how can we tell when swap memory is filling up? Can intel_get_avail_ram_mb() go up due to the system swapping memory out while available swap memory goes down? The subtest is using intel_get_total_swap_mb(). What if there is already stuff in swap? The sysinfo structure appears to have a freeswap field.

How about I try the following:
1. Add an intel_get_avail_swap_mb() function
2. Update the test to allocate buffers until half of swap is used then do the blits. (no idea how slow calling intel_get_avail_swap_mb repeatedly will be)

//Derek

>

>.Dave.

>
diff mbox

Patch

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);
 		run_test(fd, count);