From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36510) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SUG1U-0000s2-6f for qemu-devel@nongnu.org; Tue, 15 May 2012 07:34:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SUG1S-0001sX-2c for qemu-devel@nongnu.org; Tue, 15 May 2012 07:34:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:61090) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SUG1R-0001sL-QZ for qemu-devel@nongnu.org; Tue, 15 May 2012 07:34:33 -0400 Message-ID: <4FB23F3E.4000700@redhat.com> Date: Tue, 15 May 2012 13:34:22 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <87vcjx1y7k.fsf@rho.meyering.net> In-Reply-To: <87vcjx1y7k.fsf@rho.meyering.net> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] posix-aio: don't set aiocb->ret/active outside critical section List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jim Meyering Cc: Paolo Bonzini , Anthony Liguori , qemu-devel@nongnu.org, Stefan Hajnoczi , Frediano Ziglio Am 15.05.2012 13:27, schrieb Jim Meyering: > > Move code that sets aiocb->ret and aiocb->active into critical section. > All other accesses are lock-guarded. Spotted by coverity. > > Signed-off-by: Jim Meyering > --- > I've included enough context to show one of the > guarded uses in the following function. > > posix-aio-compat.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/posix-aio-compat.c b/posix-aio-compat.c > index 68361f5..45511d4 100644 > --- a/posix-aio-compat.c > +++ b/posix-aio-compat.c > @@ -421,37 +421,37 @@ static void spawn_thread(void) > { > cur_threads++; > new_threads++; > /* If there are threads being created, they will spawn new workers, so > * we don't spend time creating many threads in a loop holding a mutex or > * starving the current vcpu. > * > * If there are no idle threads, ask the main thread to create one, so we > * inherit the correct affinity instead of the vcpu affinity. > */ > if (!pending_threads) { > qemu_bh_schedule(new_thread_bh); > } > } > > static void qemu_paio_submit(struct qemu_paiocb *aiocb) > { > + mutex_lock(&lock); > aiocb->ret = -EINPROGRESS; > aiocb->active = 0; > - mutex_lock(&lock); > if (idle_threads == 0 && cur_threads < max_threads) > spawn_thread(); > QTAILQ_INSERT_TAIL(&request_list, aiocb, node); > mutex_unlock(&lock); > cond_signal(&cond); > } This is just silencing coverity, not really fixing a bug, right? aiocb is inserted into request_lists only afterwards, and before it is in the list, other threads can't find it. I'm just wondering why coverity doesn't complain about the callers of qemu_paio_submit(), they access aiocb as well... Kevin