linux-kernel - Re: [LTP] [PATCH] Synchronization required before release the lock: sem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f5a7b3811003020708wdd0710fv4c3e1dcf1992935c@mail.gmail.com>
Date:	Tue, 2 Mar 2010 20:38:20 +0530
From:	naresh kamboju <naresh.kernel@...il.com>
To:	Rishikesh K Rajak <risrajak@...ux.vnet.ibm.com>
Cc:	linux-kernel@...r.kernel.org,
	ltp-list <ltp-list@...ts.sourceforge.net>
Subject: Re: [LTP] [PATCH] Synchronization required before release the lock: 
	sem_post/8-1.c

On Tue, Mar 2, 2010 at 2:20 PM, Rishikesh K Rajak
<risrajak@...ux.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@...il.com >
>
> Looks good to me though i needed few clarification below.
>
> Acked-By: Rishikesh K Rajak <risrajak@...ux.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/