[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] rng: switch request queue to QSIMPLEQ
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] rng: switch request queue to QSIMPLEQ |
Date: |
Fri, 4 Mar 2016 10:27:57 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
On 04/03/2016 10:19, Ladi Prosek wrote:
> On Fri, Mar 4, 2016 at 10:12 AM, Paolo Bonzini <address@hidden> wrote:
>>
>>
>> On 04/03/2016 09:04, Ladi Prosek wrote:
>>>>>>> + QSIMPLEQ_INIT(&s->requests);
>>>>>>> }
>>>>>
>>>>> This init here isn't necessary, the accessors for the queue will take
>>>>> care of this.
>>> We are basically purging the queue here and we want to leave it in a
>>> consistent state. Without the QSIMPLEQ_INIT the queue head would
>>> become a pair of dangling pointers. Let me know if I misunderstood
>>> your comment.
>>
>> It wouldn't, check out QSIMPLEQ_REMOVE_HEAD:
>>
>> #define QSIMPLEQ_REMOVE_HEAD(head, field) do {
>> if (((head)->sqh_first = (head)->sqh_first->field.sqe_next) == NULL)
>> (head)->sqh_last = &(head)->sqh_first;
>> } while (/*CONSTCOND*/0)
>>
>> The queue would become { NULL, &s->requests.sqh_first }. So the
>> QSIMPLEQ_INIT is indeed redundant.
>
> Right, but we're not running QSIMPLEQ_REMOVE_HEAD in this function. We
> iterate the queue and free all elements without writing anything to
> the head or to the next ptr. This is the only "write" we do in
> rng_backend_free_requests.
Ah, sorry, I was convinced that rng_backend_free_request did the remove,
but now I remember checking it yesterday (after making the same
reasoning as Amit) and indeed it doesn't. :)
So the patch is okay. It's just a slightly unusual use of
QSIMPLEQ_FOREACH_SAFE.
Paolo
Re: [Qemu-devel] [PATCH] rng: switch request queue to QSIMPLEQ, Amit Shah, 2016/03/04