All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Manfred Spraul <manfred@colorfullife.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Chris Mason <chris.mason@oracle.com>,
	Zach Brown <zach.brown@oracle.com>,
	Jens Axboe <jens.axboe@oracle.com>, Nick Piggin <npiggin@suse.de>
Subject: Re: [PATCH 2/2] ipc/sem.c: move wake_up_process out of the spinlock section
Date: Tue, 11 May 2010 14:21:49 -0700	[thread overview]
Message-ID: <20100511142149.83bb3538.akpm@linux-foundation.org> (raw)
In-Reply-To: <1272481588-1941-3-git-send-email-manfred@colorfullife.com>

On Wed, 28 Apr 2010 21:06:27 +0200
Manfred Spraul <manfred@colorfullife.com> wrote:

> The wake-up part of semtimedop() consists out of two steps:
> - the right tasks must be identified.
> - they must be woken up.
> 
> Right now, both steps run while the array spinlock is held.
> This patch reorders the code and moves the actual wake_up_process()
> behind the point where the spinlock is dropped.
> 
> The code also moves setting sem->sem_otime to one place: It does not
> make sense to set the last modify time multiple times.

ipc/sem.c: In function 'update_queue':
ipc/sem.c:545: warning: 'retval' may be used uninitialized in this function

which indeed was a bug.

--- a/ipc/sem.c~ipc-semc-move-wake_up_process-out-of-the-spinlock-section-fix-2
+++ a/ipc/sem.c
@@ -542,7 +542,7 @@ static int update_queue(struct sem_array
 	struct list_head *walk;
 	struct list_head *pending_list;
 	int offset;
-	int retval;
+	int retval = 0;
 
 	/* if there are complex operations around, then knowing the semaphore
 	 * that was modified doesn't help us. Assume that multiple semaphores
_

But I worry that the patch which you sent might not have been the one
which you tested.


  parent reply	other threads:[~2010-05-11 21:22 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-28 19:06 [PATCH 0/3] ipc/sem.c: Optimization for reducing spinlock contention Manfred Spraul
2010-04-28 19:06 ` [PATCH 1/2] ipc/sem.c: Optimize update_queue() for bulk wakeup calls Manfred Spraul
2010-04-28 19:06   ` [PATCH 2/2] ipc/sem.c: move wake_up_process out of the spinlock section Manfred Spraul
2010-04-28 19:06     ` [PATCH 3/3] [PATCH] ipc/sem.c: cacheline align the ipc spinlock for semaphores Manfred Spraul
2010-05-11 21:21     ` Andrew Morton [this message]
2010-05-12 17:34       ` [PATCH 2/2] ipc/sem.c: move wake_up_process out of the spinlock section Manfred Spraul
2010-05-12 18:18         ` Manfred Spraul

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=20100511142149.83bb3538.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=chris.mason@oracle.com \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manfred@colorfullife.com \
    --cc=npiggin@suse.de \
    --cc=zach.brown@oracle.com \
    /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.