2.6.5-rc2-mm2 ipc hang fix

2.6.5-rc2-mm2 ipc hang fix

[badari]

Hi Andrew,

I ran into ipc hang while trying to shutdown database. Problem seems
to be due to missing sem_unlock() in find_undo(). Here is the patch
to fix the problem.

Please apply.

Thanks,
Badari

--- linux/ipc/sem.c     2004-03-26 05:19:22.833959160 -0800
+++ linux.new/ipc/sem.c 2004-03-26 05:19:57.047757872 -0800
@@ -972,8 +972,10 @@ static struct sem_undo *find_undo(int se
        if(sma==NULL)
                goto out;
        un = ERR_PTR(-EIDRM);
-       if (sem_checkid(sma,semid))
+       if (sem_checkid(sma,semid)) {
+               sem_unlock(sma);
                goto out_unlock;
+       }
        nsems = sma->sem_nsems;
        sem_unlock(sma);

Re: 2.6.5-rc2-mm2 ipc hang fix

[Manfred Spraul]

badari wrote:

>--- linux/ipc/sem.c     2004-03-26 05:19:22.833959160 -0800
>+++ linux.new/ipc/sem.c 2004-03-26 05:19:57.047757872 -0800
>@@ -972,8 +972,10 @@ static struct sem_undo *find_undo(int se
>        if(sma==NULL)
>                goto out;
>        un = ERR_PTR(-EIDRM);
>-       if (sem_checkid(sma,semid))
>+       if (sem_checkid(sma,semid)) {
>+               sem_unlock(sma);
>                goto out_unlock;
>+       }
>        nsems = sma->sem_nsems;
>        sem_unlock(sma);
>  
>
[snip]

> out_unlock:
>         unlock_semundo();
> out:
>         return un;
> }

Thanks for finding the bug - out_unlock unlocks the wrong spinlock, 
that's why I didn't notice it while searching for the bug.
But I think your fix is wrong: the "goto out_unlock" must be replaced 
with "goto out": the semundo spinlock is not held.

--
    Manfred

Re: 2.6.5-rc2-mm2 ipc hang fix

[Badari Pulavarty]

[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]

On Thursday 25 March 2004 09:57 pm, Manfred Spraul wrote:
> badari wrote:
> >--- linux/ipc/sem.c     2004-03-26 05:19:22.833959160 -0800
> >+++ linux.new/ipc/sem.c 2004-03-26 05:19:57.047757872 -0800
> >@@ -972,8 +972,10 @@ static struct sem_undo *find_undo(int se
> >        if(sma==NULL)
> >                goto out;
> >        un = ERR_PTR(-EIDRM);
> >-       if (sem_checkid(sma,semid))
> >+       if (sem_checkid(sma,semid)) {
> >+               sem_unlock(sma);
> >                goto out_unlock;
> >+       }
> >        nsems = sma->sem_nsems;
> >        sem_unlock(sma);
>
> [snip]
>
> > out_unlock:
> >         unlock_semundo();
> > out:
> >         return un;
> > }
>
> Thanks for finding the bug - out_unlock unlocks the wrong spinlock,
> that's why I didn't notice it while searching for the bug.
> But I think your fix is wrong: the "goto out_unlock" must be replaced
> with "goto out": the semundo spinlock is not held.

Yes. You are correct. semundo lock is not held. 
Here is the updated patch.

Thanks,
Badari

[-- Attachment #2: undo.patch --]
[-- Type: text/x-diff, Size: 397 bytes --]

--- linux/ipc/sem.c	2004-03-26 05:19:22.833959160 -0800
+++ linux.new/ipc/sem.c	2004-03-26 20:59:46.496258680 -0800
@@ -972,8 +972,10 @@ static struct sem_undo *find_undo(int se
 	if(sma==NULL)
 		goto out;
 	un = ERR_PTR(-EIDRM);
-	if (sem_checkid(sma,semid))
-		goto out_unlock;
+	if (sem_checkid(sma,semid)) {
+		sem_unlock(sma);
+		goto out;
+	}
 	nsems = sma->sem_nsems;
 	sem_unlock(sma);
 

Re: 2.6.5-rc2-mm2 ipc hang fix (final version)

[Badari Pulavarty]

[-- Attachment #1: Type: text/plain, Size: 130 bytes --]

Hi,

Here is the final version. I missed a compile warning before.
out_unlock label is no longer needed.

Thanks,
Badari


[-- Attachment #2: undo.patch --]
[-- Type: text/x-diff, Size: 562 bytes --]

--- linux/ipc/sem.c	2004-03-26 05:19:22.833959160 -0800
+++ linux.new/ipc/sem.c	2004-03-26 21:18:28.424699632 -0800
@@ -972,8 +972,10 @@ static struct sem_undo *find_undo(int se
 	if(sma==NULL)
 		goto out;
 	un = ERR_PTR(-EIDRM);
-	if (sem_checkid(sma,semid))
-		goto out_unlock;
+	if (sem_checkid(sma,semid)) {
+		sem_unlock(sma);
+		goto out;
+	}
 	nsems = sma->sem_nsems;
 	sem_unlock(sma);
 
@@ -1004,7 +1006,6 @@ static struct sem_undo *find_undo(int se
 	sma->undo = new;
 	sem_unlock(sma);
 	un = new;
-out_unlock:
 	unlock_semundo();
 out:
 	return un;