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

[Rishikesh K Rajak]

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 :-(
> 
> After my investigation found synchronization is missing between the
> child processes.
> Made a patch to fix this issue.
> 
> 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.
> 
> 
> After applying this patch I have tested this test case 1000 times continuously.
> All the times test case reported as Test Pass :-)
> 
> 
> Signed-off-by: Naresh Kamboju < naresh.kernel@xxxxxxxxx >

Looks good to me though i needed few clarification below.

Acked-By: Rishikesh K Rajak <risrajak@xxxxxxxxxxxxxxxxxx>

> ---
>  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.

> +       do {
>                 sem_getvalue(sem_1, &val);
>                 //printf("val = %d\n", val);
>         } while (val != 1);
> -       */
> +
>         c_3 = fork();
>         if (c_3 == 0)
>         {
> @@ -191,13 +190,15 @@ int main()
>         }
>         fprintf(stderr, "P: child_3: %d forked\n", c_3);
> 
> +        /* Step 3 Implementation */
>         /* Make sure child 3 has been waiting for the lock */
> -       /*do {
> -               sleep(1);
> +       do {
>                 sem_getvalue(sem_1, &val);
>                 //printf("val = %d\n", val);
>         } while (val != 0);
> -       */
> +
> +       /* Synchronization required before release the lock */
> +       sleep(1);
>         /* Ok, let's release the lock */
>         fprintf(stderr, "P: release lock\n");
>         sem_post(sem);
> 
> 
> Test script to test 1000 times:
> /*****************************************************/
> #!/bin/sh
> 
> for ((  i = 0 ;  i < 1000;  i++  ))
> 
> do
> 
> ./8-1.test >> /tmp/sem-post-8-1.log
> done
> /*****************************************************/
> 
> Refs:
> http://ltp.cvs.sourceforge.net/viewvc/ltp/ltp/testcases/open_posix_testsuite/conformance/interfaces/sem_post/8-1.c?view=log
> 
> Please review this patch and let me know if you have any issues.
> 
> Best regards
> Naresh Kamboju


> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev

> _______________________________________________
> Ltp-list mailing list
> Ltp-list@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/ltp-list


-- 
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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/