All of lore.kernel.org
 help / color / mirror / Atom feed
From: Manfred Spraul <manfred@colorfullife.com>
To: Rik van Riel <riel@redhat.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Davidlohr Bueso <davidlohr.bueso@hp.com>,
	hhuang@redhat.com
Subject: Re: [PATCH] ipc,sem: move restart loop to do_smart_update
Date: Sun, 26 May 2013 08:09:11 +0200	[thread overview]
Message-ID: <51A1A707.9030506@colorfullife.com> (raw)
In-Reply-To: <20130519183250.2a82d642@annuminas.surriel.com>

Hi Rik,

On 05/20/2013 12:32 AM, Rik van Riel wrote:
> @@ -751,17 +744,19 @@ static int update_queue(struct sem_array *sma, int semnum, struct list_head *pt)
>   static void do_smart_update(struct sem_array *sma, struct sembuf *sops, int nsops,
>   			int otime, struct list_head *pt)
>   {
> -	int i;
> +	int i, restart;
>   
> + again:
> +	restart = 0;
>   	if (sma->complex_count || sops == NULL) {
> -		if (update_queue(sma, -1, pt))
> +		if (update_queue(sma, -1, pt, &restart))
>   			otime = 1;
>   	}
I didn't notice it immediately, this is insufficient:
Complex operations may affect all semaphores in the array.
Thus it is not sufficient to scan the per-semaphore queues of the 
semaphores that were touched by *sops, all must be scanned.

Perhaps something like:
 > +        if (update_queue(sma, -1, pt, &restart))
 > +            sops = NULL;
 >              otime = 1;
 >      }
(untested!)

Test case:
1: semop(<key>, {{1, -1, 0}, {2, 1, 0}}, 2 <unfinished ...>
2: semop(<key>, {{2, -1, 0}}, 1 <unfinished ...>
3: semop(<key>, {{1, 1, 0}}, 1)     = 0

[3] is able to run, calls do_smart_update().
The global queue is scanned and [1] is released
Without the "sops = NULL", [2] sleeps forever.

--
     Manfred


      parent reply	other threads:[~2013-05-26  6:09 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-18 13:48 ipc/sem.c: Lockup with complex ops, comments not updated Manfred Spraul
2013-05-19 22:32 ` [PATCH] ipc,sem: move restart loop to do_smart_update Rik van Riel
2013-05-22 22:20   ` Davidlohr Bueso
2013-05-26  6:09   ` Manfred Spraul [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51A1A707.9030506@colorfullife.com \
    --to=manfred@colorfullife.com \
    --cc=akpm@linux-foundation.org \
    --cc=davidlohr.bueso@hp.com \
    --cc=hhuang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=riel@redhat.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.