From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42659) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxevF-0004XR-TU for qemu-devel@nongnu.org; Fri, 21 Oct 2016 14:52:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxevB-0003jV-1O for qemu-devel@nongnu.org; Fri, 21 Oct 2016 14:52:05 -0400 Date: Fri, 21 Oct 2016 19:51:52 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20161021185152.GJ2039@work-vm> References: <1476394254-7987-1-git-send-email-duanj@linux.vnet.ibm.com> <1476394254-7987-3-git-send-email-duanj@linux.vnet.ibm.com> <20161014104447.GC2030@work-vm> <4b63338c-cf70-ce4d-1854-afc23c38b88f@linux.vnet.ibm.com> <20161014170412.GE2030@work-vm> <2a8ea35b-8335-1132-002b-c45e891a57e4@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2a8ea35b-8335-1132-002b-c45e891a57e4@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jianjun Duan Cc: Halil Pasic , veroniabahaa@gmail.com, peter.maydell@linaro.org, mdroth@linux.vnet.ibm.com, mst@redhat.com, quintela@redhat.com, mark.cave-ayland@ilande.co.uk, qemu-devel@nongnu.org, mreitz@redhat.com, blauwirbel@gmail.com, amit.shah@redhat.com, qemu-ppc@nongnu.org, kraxel@redhat.com, kwolf@redhat.com, dmitry@daynix.com, pbonzini@redhat.com, rth@twiddle.net, leon.alrae@imgtec.com, aurelien@aurel32.net, david@gibson.dropbear.id.au * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote: > > > On 10/15/2016 05:48 AM, Halil Pasic wrote: > > > > > > On 10/14/2016 07:18 PM, Jianjun Duan wrote: > >>>>>> +/* > >>>>>>>>>> + * Offsets of layout of a tail queue head. > >>>>>>>>>> + */ > >>>>>>>>>> +#define QTAILQ_FIRST_OFFSET 0 > >>>>>>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *)) > >>>>>>>>>> + > >>>>>>>>>> +/* > >>>>>>>>>> + * Offsets of layout of a tail queue element. > >>>>>>>>>> + */ > >>>>>>>>>> +#define QTAILQ_NEXT_OFFSET 0 > >>>>>>>>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *)) > >>>>>>>>>> + > >>>>>>>>>> +/* > >>>>>>>>>> + * Tail queue tranversal using pointer arithmetic. > >>>>>>>>>> + */ > >>>>>>>>>> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \ > >>>>>>>>>> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \ > >>>>>>>>>> + (elm); \ > >>>>>>>>>> + (elm) = \ > >>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET))) > >>>>>>>>>> +/* > >>>>>>>>>> + * Tail queue insertion using pointer arithmetic. > >>>>>>>>>> + */ > >>>>>>>>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ > >>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \ > >>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \ > >>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \ > >>>>>>>>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \ > >>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \ > >>>>>>>>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \ > >>>>>>>>>> +} while (/*CONSTCOND*/0) > >>>>>>>> > >>>>>>>> I wonder if there's a simpler way to do this; I'm not sure this works, but something like: > >>>>>>>> > >>>>>>>> struct QTAILQDummy { > >>>>>>>> char dummy; > >>>>>>>> }; > >>>>>>>> > >>>>>>>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy) > >>>>>>>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry; > >>>>>>>> > >>>>>>>> #define QTAILQ_RAW_FOREACH(elm, head, entry) \ > >>>>>>>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \ > >>>>>>>> (elm); \ > >>>>>>>> (elm) = \ > >>>>>>>> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next > >>>>>>>> > >>>>>>>> and then I think elm gets declared as a struct QTAILQDummy. > >>>>>>>> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations. > >>>>>>>> > >>>>>>>> Would that work? > >>>>>>>> > >>>>>> It is intended for QTAILQ of any type. So type is not available. > >>>> > >>>> I think it might be possible to do it generally. > >>>> > >> If we have type, then we can use what is there already, and don't need a > >> pointer arithmetic based approach. Inside put/get, we only get type > >> layout info from vmsd, which is all about size and offset. This macro > >> is used inside put/get, so I am not sure how we can directly use type > >> here. > >> > > > > Dave's approach seems perfectly sane to me. > > > > Jianjun have you actually tried to make it work before writing this? > > Your argument does not work, because what you need from vmsd for > > QTAILQ_RAW_FOREACH is only .start which corresponds to the entry > > parameter of the macro. Dave still does the pointer arithmetic to > > get a pointer (char*) to the anonymous struct holding tqe_next > > and tqe_prev. Now since no arithmetic is done wit tqe_next > > and tqe_prev, only dereferencing, their pointer type does not matter > > all that much so we can do the and follow the pointer. Same goes > > for the head. > > > > Actually the QTAILQDummy is not necessary in my opinion since we can > > probably (did not try it out myself) do: > > > > Q_TAILQ_HEAD(QTAILQRawHead, void,) > > typedef Q_TAILQ_ENTRY(void,) QTAILQRawEntry; > > > > Now I see. I thought Dave was using QTAILQDummy as an example. If you have a new version with either that or Paolo's suggestion, it would be good; I'd like to see this set go in because I've now got a small pile of patches built on top of it using my WITH_TMP macro. Dave > > Thanks, > Jianjun > > Cheers, > > Halil > > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK