Re: [LTP] [PATCH] Synchronization required before release the lock: sem_post/8-1.c

[naresh kamboju <naresh.kernel@gmail.com>]

On Tue, Mar 2, 2010 at 2:20 PM, Rishikesh K Rajak
<risrajak@linux.vnet.ibm.com> wrote:
> On Thu, Feb 25, 2010 at 07:15:42PM +0530, naresh kamboju wrote:
>> Hi,
>>
>> I have found abnormal behavior of sem_post/8-1.c test case under posix.
>> This test case passes in some times and failed in many times :-(
>> Patch includes
>> 1.    Reverting back changes made by mreed on Sep 25 2006. Making sure
>> child has been waiting for the lock (below Refs).
>> 2.    using sleep in while loop is not a good idea, so sleep is removed
>> from while loop
>> 3.    For the synchronization I have added sleep before releasing the lock.
>>
>> Signed-off-by: Naresh Kamboju < naresh.kernel@gmail.com >
>
> Looks good to me though i needed few clarification below.
>
> Acked-By: Rishikesh K Rajak <risrajak@linux.vnet.ibm.com>
>
>> ---
>>  testcases/open_posix_testsuite/conformance/interfaces/sem_post/8-1.c
>> |   15    8 +     7 -     0 !
>>  1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> 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)
>>         {
>> @@ -176,13 +175,13 @@ int main()
>>         }
>>         fprintf(stderr, "P: child_2: %d forked\n", c_2);
>>
>> +        /* Step 3 Implementation */
>>         /* Make sure the two children has been waiting */
>> -       /*do {
>> -               sleep(1);
> I feel before getting semaphore value, we need to sync first so here
> sleep is require,though your point is valid that there is no use of
> using sleep inside while loop.
I agree with you.
AFAIU, we should call sleep() before calling getting semaphore
value.when we don't have while loop here. Because while loop condition
is depends on val so when ever we call sem_getvalue() it will get
latest value of val.
In addition to this, we are ensuring val is decremented before we do
unlock the sem by while loop condition.

Having sleep() in while loop will not effect the final output. IIUC

Best regards,
Naresh Kamboju
>> Please review this patch and let me know if you have any issues.
>>
>> Best regards
>> Naresh Kamboju
> --
> Thanks & Regards
> Rishi
> LTP Maintainer
> IBM, LTC, Bangalore
> Please join IRC #ltp @ irc.freenode.net