[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] posix-aio: don't set aiocb->ret/active outside
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH] posix-aio: don't set aiocb->ret/active outside critical section |
Date: |
Tue, 15 May 2012 13:34:22 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1 |
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 <address@hidden>
> ---
> 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