Hi Rishi and Garrett,

On Wed, Mar 3, 2010 at 7:15 PM, naresh kamboju <naresh.kernel@gmail.com> wrote:
> On Wed, Mar 3, 2010 at 5:25 PM, Garrett Cooper <yanegomi@gmail.com> wrote:
>> On Wed, Mar 3, 2010 at 3:49 AM, naresh kamboju <naresh.kernel@gmail.com> wrote:
>>> On Wed, Mar 3, 2010 at 5:12 PM, Garrett Cooper <yanegomi@gmail.com> wrote:
>>>> On Wed, Mar 3, 2010 at 3:33 AM, naresh kamboju <naresh.kernel@gmail.com> wrote:
>>>>> On Wed, Mar 3, 2010 at 3:15 PM, Rishikesh K Rajak
>>>>> <risrajak@linux.vnet.ibm.com> wrote:
>>>>>> On Wed, Mar 03, 2010 at 01:18:50AM -0800, Garrett Cooper wrote:
>>>>>>> Uh, hold on a sec before we call it good...
>>>>>>
>>>>>> oh ok, holiding on for bit.
>>>>>>
>>>>>> Naresh, can you please send a patch with incorporating garret's comment
>>>>>> ?
>>>>>>>>> +        /* Step 3 Implementation */
>>>>>>>>>       /* Make sure the two children has been waiting */
>>>>>>>>> -     /*do {
>>>>>>>>> -             sleep(1);
>>>>>>>>> +     do {
>>>>>>>>>               sem_getvalue(sem_1, &val);
>>>>>>>>>               //printf("val = %d\n", val);
>>>>>>>>>       } while (val != 1);
>>>>>>>>> -     */
>>>>>>>
>>>>>>> Please provide another patch with a limit to this --
>>>>>
>>>>> Garrett,
>>>>>
>>>>> When sem_wait is called 'val' value will be decremented by one.
>>>>> To ensure that sem_wait is called, we are checking 'val' value by
>>>>> calling sem_getvalue(). in this case we don’t need to decrement the
>>>>> values by --. IIUC.
>>>>> (snip)
>>>>> OTOH,
>>>>>>> I get annoyed
>>>>>>> with tests that have infinite loops in them because the underlying
>>>>>>> functionality is broken.
>>>>> I agree with you, having infinite loops in test case is not a good.
>>>>> However, in this patch while loop is not infinite loop. It is a
>>>>> conditional loop with finite value.
>>>>>
>>>>> Please let me know if you have any issues.
>>>>
>>>>    The problem was that it wasn't failing properly as stated in the
>>>> manpage on mips* (was decrementing past 0) and it was blocking
>>>> indefinitely. Hence I had to yank those tests from the default run.
>>> do you mean, after applying above patch you have noticed these kind of behavior?
>>
>> Not with this patch; I've seen this kind of behavior in general under
>> odd conditions with my former team's embedded setup running tests with
>> POSIX semaphores, so I don't doubt that others could run into the
>> similar functional issues given the right conditions.
>
> Hey Garrett,
>
> Thanks for you information :-)
>
> I have tested these on MIPS architecture and  reproduced infinite
> waiting situation after applying this patch. (with strace no issue
> found ex: #strace ./8-1.test)
> As you said there may be issues in MIPS-POSIX library. it may take
> some time to fix these issues or may not be fixed.
> However, I’ll discuss this issue with MIPS folks in different thread.
>
> ATM, my patch is not a good idea for MIPS architectures.
> So, after your comments I have modified my patch and tested.
> Here in this latest patch i did not change any thing regarding while loop.
> I replaced sleep() in an appropriate place.
> It is working fine on X86, ARM and MIPS.

please review the patch.
I have tested this patch on X86, ARM and MIPS and results are good.

If you feel ok. please commit. otherwise give me your comments.

Best regards
Naresh Kamboju
>
> Signed-off-by: Naresh Kamboju < naresh.kernel@gmail.com >
> ---
>  testcases/open_posix_testsuite/conformance/interfaces/sem_post/8-1.c
> |    2  1 +     1 -     0 !
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: b/testcases/open_posix_testsuite/conformance/interfaces/sem_post/8-1.c
> ===================================================================
> --- a/testcases/open_posix_testsuite/conformance/interfaces/sem_post/8-1.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/sem_post/8-1.c
> @@ -161,7 +161,6 @@ int main()
>        }
>        fprintf(stderr, "P: child_1:%d forked\n", c_1);
>
> -       sleep(1);
>        c_2 = fork();
>        if (c_2 == 0)
>        {
> @@ -198,6 +197,7 @@ int main()
>                //printf("val = %d\n", val);
>        } while (val != 0);
>        */
> +       sleep(1);
>        /* Ok, let's release the lock */
>        fprintf(stderr, "P: release lock\n");
>        sem_post(sem);
>
>
> please let me know if you have any issues.
>
> Best regards,
> Naresh Kamboju
>> Thanks,
>> -Garrett
>>
>