From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58665) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1G6x-0005GN-11 for qemu-devel@nongnu.org; Mon, 31 Oct 2016 13:11:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1G6t-0003nX-Q2 for qemu-devel@nongnu.org; Mon, 31 Oct 2016 13:11:02 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:42974 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c1G6t-0003my-Df for qemu-devel@nongnu.org; Mon, 31 Oct 2016 13:10:59 -0400 Received: from pps.filterd (m0098413.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u9VHAmdZ055533 for ; Mon, 31 Oct 2016 13:10:59 -0400 Received: from e31.co.us.ibm.com (e31.co.us.ibm.com [32.97.110.149]) by mx0b-001b2d01.pphosted.com with ESMTP id 26e239uxyb-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 31 Oct 2016 13:10:58 -0400 Received: from localhost by e31.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 31 Oct 2016 11:10:57 -0600 References: <1477607317-27817-1-git-send-email-duanj@linux.vnet.ibm.com> <1477607317-27817-3-git-send-email-duanj@linux.vnet.ibm.com> <20161028190657.GB2173@work-vm> <0cd200a6-3ded-0e02-1b8b-fbbf5c0bee03@linux.vnet.ibm.com> <88bb646d-39aa-e439-4b30-2c38777a4b56@linux.vnet.ibm.com> From: Jianjun Duan Date: Mon, 31 Oct 2016 10:10:43 -0700 MIME-Version: 1.0 In-Reply-To: <88bb646d-39aa-e439-4b30-2c38777a4b56@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Message-Id: Subject: Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v9 2/3] migration: migrate QTAILQ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Halil Pasic , "Dr. David Alan Gilbert" Cc: 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 On 10/31/2016 06:10 AM, Halil Pasic wrote: > > > On 10/28/2016 09:46 PM, Jianjun Duan wrote: >> >> >> On 10/28/2016 12:06 PM, Dr. David Alan Gilbert wrote: >>> * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote: >>>> Currently we cannot directly transfer a QTAILQ instance because of the >>>> limitation in the migration code. Here we introduce an approach to >>>> transfer such structures. We created VMStateInfo vmstate_info_qtailq >>>> for QTAILQ. Similar VMStateInfo can be created for other data structures >>>> such as list. >>>> >>>> This approach will be used to transfer pending_events and ccs_list in spapr >>>> state. >>>> >>>> We also create some macros in qemu/queue.h to access a QTAILQ using pointer >>>> arithmetic. This ensures that we do not depend on the implementation >>>> details about QTAILQ in the migration code. >>>> >>>> Signed-off-by: Jianjun Duan >>>> --- >>>> include/migration/vmstate.h | 20 ++++++++++++++ >>>> include/qemu/queue.h | 61 +++++++++++++++++++++++++++++++++++++++++ >>>> migration/trace-events | 4 +++ >>>> migration/vmstate.c | 67 +++++++++++++++++++++++++++++++++++++++++++++ >>>> 4 files changed, 152 insertions(+) >>>> >>>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h >>>> index d0e37b5..318a6f1 100644 >>>> --- a/include/migration/vmstate.h >>>> +++ b/include/migration/vmstate.h >>>> @@ -251,6 +251,7 @@ extern const VMStateInfo vmstate_info_timer; >>>> extern const VMStateInfo vmstate_info_buffer; >>>> extern const VMStateInfo vmstate_info_unused_buffer; >>>> extern const VMStateInfo vmstate_info_bitmap; >>>> +extern const VMStateInfo vmstate_info_qtailq; >>>> >>>> #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0) >>>> #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0) >>>> @@ -662,6 +663,25 @@ extern const VMStateInfo vmstate_info_bitmap; >>>> .offset = offsetof(_state, _field), \ >>>> } >>>> >>>> +/* For QTAILQ that need customized handling. >>>> + * Target QTAILQ needs be properly initialized. >>>> + * _type: type of QTAILQ element >>>> + * _next: name of QTAILQ entry field in QTAILQ element >>>> + * _vmsd: VMSD for QTAILQ element >>>> + * size: size of QTAILQ element >>>> + * start: offset of QTAILQ entry in QTAILQ element >>>> + */ >>>> +#define VMSTATE_QTAILQ_V(_field, _state, _version, _vmsd, _type, _next) \ >>>> +{ \ >>>> + .name = (stringify(_field)), \ >>>> + .version_id = (_version), \ >>>> + .vmsd = &(_vmsd), \ >>>> + .size = sizeof(_type), \ >>>> + .info = &vmstate_info_qtailq, \ >>>> + .offset = offsetof(_state, _field), \ >>>> + .start = offsetof(_type, _next), \ >>>> +} >>>> + >>>> /* _f : field name >>>> _f_n : num of elements field_name >>>> _n : num of elements >>>> diff --git a/include/qemu/queue.h b/include/qemu/queue.h >>>> index 342073f..16af127 100644 >>>> --- a/include/qemu/queue.h >>>> +++ b/include/qemu/queue.h >>>> @@ -438,4 +438,65 @@ struct { \ >>>> #define QTAILQ_PREV(elm, headname, field) \ >>>> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last)) >>>> >>>> +#define field_at_offset(base, offset, type) \ >>>> + ((type) (((char *) (base)) + (offset))) >>>> + >>>> +typedef struct DUMB_Q_ENTRY DUMB_Q_ENTRY; >>>> +typedef struct DUMB_Q DUMB_Q; >>>> + >>>> +struct DUMB_Q_ENTRY { >>>> + QTAILQ_ENTRY(DUMB_Q_ENTRY) next; >>>> +}; >>>> + >>>> +struct DUMB_Q { >>>> + QTAILQ_HEAD(DUMB_Q_HEAD, DUMB_Q_ENTRY) head; >>>> +}; >>> >>> OK, good we've got rid of the hard coded offsets; thanks! >>> >>>> +#define dumb_q ((DUMB_Q *) 0) >>>> +#define dumb_qh ((typeof(dumb_q->head) *) 0) >>>> +#define dumb_qe ((DUMB_Q_ENTRY *) 0) >>> >>> Note that 'dumb' and 'dummy' are completely different words; >>> this is a dummy not a dumb. >>> >> OK. >>>> +/* >>>> + * Offsets of layout of a tail queue head. >>>> + */ >>>> +#define QTAILQ_FIRST_OFFSET ((size_t) &(QTAILQ_FIRST(dumb_qh))) >>> >>> Isn't that just offsetof(dumb_qh, tqh_first) ? >> Yes. But I don't want to depend on the inside of the type if it is >> possible. QTAILQ_FIRST is a defined access macro. >> >>> >>>> +#define QTAILQ_LAST_OFFSET (offsetof(typeof(dumb_q->head), tqh_last)) >>> >>> Similarly isnt't that just offsetof(DUMB_Q_HEAD, tqh_last) ? >>> >> Similarly, DUMB_Q_HEAD may not be a type name in the future. >> >> Thanks, >> Jianjun >>>> +/* >>>> + * Raw access of elements of a tail queue >>>> + */ >>>> +#define QTAILQ_RAW_FIRST(head) \ >>>> + (*field_at_offset(head, QTAILQ_FIRST_OFFSET, void **)) >>>> +#define QTAILQ_RAW_LAST(head) \ >>>> + (*field_at_offset(head, QTAILQ_LAST_OFFSET, void ***)) >>>> + >>>> +/* >>>> + * Offsets of layout of a tail queue element. >>>> + */ >>>> +#define QTAILQ_NEXT_OFFSET ((size_t) (&QTAILQ_NEXT(dumb_qe, next))) >>>> +#define QTAILQ_PREV_OFFSET (offsetof(typeof(dumb_qe->next), tqe_prev)) >>> >>> Similar comments to the pair above. >>> >>> Dave >>> P.S. I'm out next week, so please someone else jump in. >>> > [..] > > I think this got overly complicated. Here is a little patch on > top of your stuff which gets rid of 15 lines and IMHO simplifies > things quite a bit. What do you think? > > It is based on/inspired by Dave's proposal with the dummy stuff. > > Did not address the typos though. It is unlikely the definition of QTAILQ will change, so hard-coded value probably was the most simple. Now that we want to address the potential changes, I think my code will deal with future changes better. It uses the proper q head and entry definition, and uses the provided interface whenever possible. I will fix the typo though. Thanks, Jianjun > > Cheers, > Halil > > ----------------- 8< ---------------------------- > From: Halil Pasic > Date: Mon, 31 Oct 2016 13:53:31 +0100 > Subject: [PATCH] fixup: simplify QTAILQ raw access macros > > Intended to be fixed up to [PATCH v9 2/3] migration: migrate QTAILQ. > > Signed-off-by: Halil Pasic > --- > include/qemu/queue.h | 33 +++++++++------------------------ > 1 files changed, 9 insertions(+), 24 deletions(-) > > diff --git a/include/qemu/queue.h b/include/qemu/queue.h > index 16af127..d67cb4e 100644 > --- a/include/qemu/queue.h > +++ b/include/qemu/queue.h > @@ -441,33 +441,17 @@ struct { \ > #define field_at_offset(base, offset, type) \ > ((type) (((char *) (base)) + (offset))) > > -typedef struct DUMB_Q_ENTRY DUMB_Q_ENTRY; > -typedef struct DUMB_Q DUMB_Q; > - > -struct DUMB_Q_ENTRY { > - QTAILQ_ENTRY(DUMB_Q_ENTRY) next; > -}; > - > -struct DUMB_Q { > - QTAILQ_HEAD(DUMB_Q_HEAD, DUMB_Q_ENTRY) head; > -}; > - > -#define dumb_q ((DUMB_Q *) 0) > -#define dumb_qh ((typeof(dumb_q->head) *) 0) > -#define dumb_qe ((DUMB_Q_ENTRY *) 0) > - > /* > - * Offsets of layout of a tail queue head. > + * Raw access helpers > */ > -#define QTAILQ_FIRST_OFFSET ((size_t) &(QTAILQ_FIRST(dumb_qh))) > -#define QTAILQ_LAST_OFFSET (offsetof(typeof(dumb_q->head), tqh_last)) > +typedef Q_TAILQ_HEAD(QTAILQRawHead, void,) QTAILQRawHead; > +typedef Q_TAILQ_ENTRY(void,) QTAILQRawEntry; > + > /* > * Raw access of elements of a tail queue > */ > -#define QTAILQ_RAW_FIRST(head) \ > - (*field_at_offset(head, QTAILQ_FIRST_OFFSET, void **)) > -#define QTAILQ_RAW_LAST(head) \ > - (*field_at_offset(head, QTAILQ_LAST_OFFSET, void ***)) > +#define QTAILQ_RAW_FIRST(head) (((QTAILQRawHead *)(head))->tqh_first) > +#define QTAILQ_RAW_LAST(head) (((QTAILQRawHead *)(head))->tqh_last) > > /* > * Offsets of layout of a tail queue element. > @@ -479,9 +463,10 @@ struct DUMB_Q { > * Raw access of elements of a tail entry > */ > #define QTAILQ_RAW_NEXT(elm, entry) \ > - (*field_at_offset(elm, entry + QTAILQ_NEXT_OFFSET, void **)) > + ((field_at_offset(elm, entry, QTAILQRawEntry *)->tqe_next)) > #define QTAILQ_RAW_PREV(elm, entry) \ > - (*field_at_offset(elm, entry + QTAILQ_PREV_OFFSET, void ***)) > + (field_at_offset(elm, entry, QTAILQRawEntry *)->tqe_prev) > + > /* > * Tail queue tranversal using pointer arithmetic. > */ >