Kevin Wolf wrote:
> 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 <meyer...@redhat.com>
>> ---
>> 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
Yes. Given your explanation, it's obvious, now.
> 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...
It warned only about the setting of aiocb->ret (not about ->active),
apparently because it failed to deduce that the lock is not actually
required here, while several other ->ret accesses *are* guarded.
