[Top][All Lists]

[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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]