From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38548) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abm1m-0002yH-Ap for qemu-devel@nongnu.org; Fri, 04 Mar 2016 04:28:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1abm1h-0001Nq-BS for qemu-devel@nongnu.org; Fri, 04 Mar 2016 04:28:06 -0500 Received: from mail-wm0-x231.google.com ([2a00:1450:400c:c09::231]:37022) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abm1h-0001Nj-1h for qemu-devel@nongnu.org; Fri, 04 Mar 2016 04:28:01 -0500 Received: by mail-wm0-x231.google.com with SMTP id p65so21923868wmp.0 for ; Fri, 04 Mar 2016 01:28:00 -0800 (PST) Sender: Paolo Bonzini References: <1457010971-24771-1-git-send-email-lprosek@redhat.com> <20160304062723.GH15443@grmbl.mre> <56D95173.9050108@redhat.com> From: Paolo Bonzini Message-ID: <56D9551D.3070607@redhat.com> Date: Fri, 4 Mar 2016 10:27:57 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] rng: switch request queue to QSIMPLEQ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ladi Prosek Cc: Amit Shah , pagupta , qemu-devel@nongnu.org On 04/03/2016 10:19, Ladi Prosek wrote: > On Fri, Mar 4, 2016 at 10:12 AM, Paolo Bonzini 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