Re: [LTP] [PATCH] perf_event_open: improve the memory leak detection

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> >> +		/*
> >> +		 * Every 1200000 iterations, calculate the difference in memory
> >> +		 * availability. If the difference is greater than 10 * 1024 (10MB),
> >> +		 * increment the sample counter and log the event.
> >> +		 */
> >> +		if ((i % 1200000) == 0) {
> >> +			mem_avail = SAFE_READ_MEMINFO("MemAvailable:");
> >> +			diff = mem_avail_prev - mem_avail;
> >> +			diff_total += diff;
> >> +
> >> +			if (diff > 20 * 1024) {
> > 
> > Shouldn't this be 10 * 1024 or possibly slightly less than 10 * 1024?
> 
> This should be fine. The test leaks around 38MB between sample checks 
> and over 350MB in a whole test run.

Then we should fix the comment.

> > 
> >> +				sample++;
> >> +				tst_res(TINFO, "MemAvailable decreased by %ld kB at iteration %d", diff, i);
> >> +			}
> >> +
> >> +			mem_avail_prev = mem_avail;
> >> +		}
> >> +	}
> >>   
> >> -	if (diff > 50 * 1024)
> >> -		tst_res(TFAIL, "Likely kernel memory leak detected");
> >> +	if ((sample > 5) || (diff_total > 100 * 1024))
> > 
> > Maybe this can rather be if ((sample > 5) && (diff_total > 100 * 1024))
> > 
> > That means that the available memory has been eaten by something and
> > that it happened more or less in a linear fashion when the program was
> > running.
> 
> Imagine that some other process releases 300MB of memory while the test 
> is running. If you change the || to &&, you'll get a false negative in 
> that case. The sampling approach will protect against such interference.
> 
> That being said, if the available memory on your test system fluctuates 
> by 100+MB during a test run that takes <10 seconds, I'd recommend 
> investigating what's causing such fluctuation. On the test machine I 
> used to verify this patch, I can see <10MB of difference before and 
> after running the test on a fixed kernel.

So shall we remove the diff_total completely?

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp