Re: [PATCH] Take 2: Fix false positives in tst-pthread-getattr testcase

[Carlos O'Donell <carlos_odonell at mentor dot com>]

On 7/20/2012 7:58 AM, Siddhesh Poyarekar wrote:
> Hi,
> 
> Back in June, Carlos saw a failure in a test case I had written:
> 
> http://sourceware.org/ml/libc-alpha/2012-06/msg00599.html
> 
> While analysing this, I discovered that portions of the test case were
> being optimized out by some versions of gcc, resulting in false
> positives in some cases (and Carlos was not so (un)lucky).
> Additionally, there was a mysterious problem in the kernel (still is,
> AFAICT) where it would (occasionally) incorrectly return the
> RLIMIT_STACK as unlimited. I attempted a fix for the optimization:
> 
> http://sourceware.org/ml/libc-alpha/2012-06/msg00713.html
> 
> but there was still a mysterious problem in the kernel. I was asked not
> to commit the change since we were in 2.16 freeze. I have now redone
> the patch a bit (attached), adding a cap to the stack size we test with
> in the patch to 8M to work around the mysterious kernel behaviour. Does
> this look OK to commit?
> 
> Regards,
> Siddhesh
> 
> 
> nptl/ChangeLog:
> 
> 2012-07-20  Siddhesh Poyarekar  <siddhesh@redhat.com>
> 
> 	* tst-pthread-getattr.c (MAX_STACK_SIZE): New max cap for stack
> 	size.

I see nothing in your comments that mentions this is a workaround
to a kernel problem? We should document in the test case comments
*why* we are doing this, thus future maintainers can decide to remove
the code once the kernel is fixed.

Does that make sense?

> 	(_MIN): New macro.
> 	(allocate_and_test): Return STACKADDR.  Access STACKADDR instead
> 	of MEM to test.
> 	(check_stack_top): Read valued written into STACKADDR in
> 	allocate_and_test.  Cap stack size to MAX_STACK_SIZE.
> 

OK to checkin with additional verbose comments about kernel problem.

Cheers,
Carlos.
-- 
Carlos O'Donell
Mentor Graphics / CodeSourcery
carlos_odonell@mentor.com
carlos@codesourcery.com
+1 (613) 963 1026