Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ

2016-10-21 Thread Jianjun Duan 


On 10/21/2016 11:51 AM, Dr. David Alan Gilbert wrote:
> * Jianjun Duan (du...@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.
> 

I am working on it.

Thanks,
Jianjun
> Dave
> 
>>
>> Thanks,
>> Jianjun
>>> Cheers,
>>> Halil
>>>
>>
> --
> Dr.

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ

2016-10-21 Thread Dr. David Alan Gilbert 
* Jianjun Duan (du...@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 / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ

2016-10-17 Thread Jianjun Duan 


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.

Thanks,
Jianjun
> Cheers,
> Halil
>

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ

2016-10-15 Thread Halil Pasic 


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;

Cheers,
Halil

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ

2016-10-14 Thread Jianjun Duan 


On 10/14/2016 11:39 AM, Paolo Bonzini wrote:
> 
>>> Another possibility is a macro like
>>>
>>> #define field_at_offset(base, offset, type) \
>>>((type) (((char *) (base)) + (offset)))
>>>
>>> so that you can do
>>>
>>>*field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET) = NULL;
>>>*field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET) =
>>>*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET);
>>>**field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = (elm);
>>>*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) =
>>> field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET);
>>
>> The thing is that we don't know type at all. So only the offset values
>> is used. This is intended for QTAILQ of any type.
> 
> Sure, my code is identical to yours---just with more macros for readability,
> and a little more type-safe.  Another possibility:
> 
> #define QTAILQ_RAW_NEXT(elm, entry) \
>(*field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET))
> #define QTAILQ_RAW_PREV(elm, entry) \
>(*field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET))
> #define QTAILQ_RAW_LAST(head) \
>(*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET))
> 
> QTAILQ_RAW_NEXT(elm, entry) = NULL;
> QTAILQ_RAW_PREV(elm, entry) = QTAILQ_RAW_LAST(head);
> *QTAILQ_RAW_LAST(head) = (elm);
> QTAILQ_RAW_LAST(head) = _RAW_NEXT(elm, entry);
> 

You are right. The readability can be approved.

Thanks,
Jianjun
> Thanks,
> 
> Paolo
> 
>> Thanks,
>> Jianjun
>>
>>>
>>> or something like that (note that I've always used the same type for
>>> next and last, by the way).
>>>
>>> Paolo
>>>
>>
>>
>

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ

2016-10-14 Thread Paolo Bonzini 

> > Another possibility is a macro like
> > 
> > #define field_at_offset(base, offset, type) \
> >((type) (((char *) (base)) + (offset)))
> > 
> > so that you can do
> > 
> >*field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET) = NULL;
> >*field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET) =
> >*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET);
> >**field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = (elm);
> >*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) =
> > field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET);
> 
> The thing is that we don't know type at all. So only the offset values
> is used. This is intended for QTAILQ of any type.

Sure, my code is identical to yours---just with more macros for readability,
and a little more type-safe.  Another possibility:

#define QTAILQ_RAW_NEXT(elm, entry) \
   (*field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET))
#define QTAILQ_RAW_PREV(elm, entry) \
   (*field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET))
#define QTAILQ_RAW_LAST(head) \
   (*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET))

QTAILQ_RAW_NEXT(elm, entry) = NULL;
QTAILQ_RAW_PREV(elm, entry) = QTAILQ_RAW_LAST(head);
*QTAILQ_RAW_LAST(head) = (elm);
QTAILQ_RAW_LAST(head) = _RAW_NEXT(elm, entry);

Thanks,

Paolo

> Thanks,
> Jianjun
> 
> > 
> > or something like that (note that I've always used the same type for
> > next and last, by the way).
> > 
> > Paolo
> > 
> 
>

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ

2016-10-14 Thread Jianjun Duan 


On 10/14/2016 10:04 AM, Dr. David Alan Gilbert wrote:
> * Jianjun Duan (du...@linux.vnet.ibm.com) wrote:
>>
>>
>> On 10/14/2016 03:44 AM, Dr. David Alan Gilbert wrote:
>>> * Jianjun Duan (du...@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| 32 
  migration/trace-events  |  4 +++
  migration/vmstate.c | 59 
 +
  4 files changed, 115 insertions(+)

 diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
 index d0e37b5..4dd0aed 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
 + * _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 = _info_qtailq,\
 +.flags= VMS_LINKED,  \
>>>
>>> Hang on - you killed VMS_LINKED off in the previous patch; so this
>>> doesn't work.
>>> (Adding a test to tests/test-vmstate.c would have found that)
>>>
>>
>> Thanks for pointing it out. Since the actual application of this patch
>> is split off, my test didn't go through this path.
> 
> Yes, but you could add a new entry into test-vmstate.c for this.

OK.
> 
 +.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..d672ae0 100644
 --- a/include/qemu/queue.h
 +++ b/include/qemu/queue.h
 @@ -438,4 +438,36 @@ struct {  
   \
  #define QTAILQ_PREV(elm, headname, field) \
  (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
  
 +/*
 + * 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 {

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ

2016-10-14 Thread Dr. David Alan Gilbert 
* Jianjun Duan (du...@linux.vnet.ibm.com) wrote:
> 
> 
> On 10/14/2016 03:44 AM, Dr. David Alan Gilbert wrote:
> > * Jianjun Duan (du...@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| 32 
> >>  migration/trace-events  |  4 +++
> >>  migration/vmstate.c | 59 
> >> +
> >>  4 files changed, 115 insertions(+)
> >>
> >> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> >> index d0e37b5..4dd0aed 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
> >> + * _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 = _info_qtailq,\
> >> +.flags= VMS_LINKED,  \
> > 
> > Hang on - you killed VMS_LINKED off in the previous patch; so this
> > doesn't work.
> > (Adding a test to tests/test-vmstate.c would have found that)
> > 
> 
> Thanks for pointing it out. Since the actual application of this patch
> is split off, my test didn't go through this path.

Yes, but you could add a new entry into test-vmstate.c for this.

> >> +.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..d672ae0 100644
> >> --- a/include/qemu/queue.h
> >> +++ b/include/qemu/queue.h
> >> @@ -438,4 +438,36 @@ struct {  
> >>   \
> >>  #define QTAILQ_PREV(elm, headname, field) \
> >>  (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
> >>  
> >> +/*
> >> + * 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) +

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ

2016-10-14 Thread Jianjun Duan 


On 10/14/2016 03:44 AM, Dr. David Alan Gilbert wrote:
> * Jianjun Duan (du...@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| 32 
>>  migration/trace-events  |  4 +++
>>  migration/vmstate.c | 59 
>> +
>>  4 files changed, 115 insertions(+)
>>
>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>> index d0e37b5..4dd0aed 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
>> + * _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 = _info_qtailq,\
>> +.flags= VMS_LINKED,  \
> 
> Hang on - you killed VMS_LINKED off in the previous patch; so this
> doesn't work.
> (Adding a test to tests/test-vmstate.c would have found that)
> 

Thanks for pointing it out. Since the actual application of this patch
is split off, my test didn't go through this path.
>> +.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..d672ae0 100644
>> --- a/include/qemu/queue.h
>> +++ b/include/qemu/queue.h
>> @@ -438,4 +438,36 @@ struct {
>> \
>>  #define QTAILQ_PREV(elm, headname, field) \
>>  (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
>>  
>> +/*
>> + * 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 *)

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ

2016-10-14 Thread Jianjun Duan 
I need to double check my code. My build passed and migration test also
succeeded.


On 10/14/2016 04:07 AM, Paolo Bonzini wrote:
> 
> 
> On 14/10/2016 12:44, Dr. David Alan Gilbert wrote:
 +#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.
> 
> Another possibility is a macro like
> 
> #define field_at_offset(base, offset, type) \
>((type) (((char *) (base)) + (offset)))
> 
> so that you can do
> 
>*field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET) = NULL;
>*field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET) =
>*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET);
>**field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = (elm);
>*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) =
> field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET);

The thing is that we don't know type at all. So only the offset values
is used. This is intended for QTAILQ of any type.

Thanks,
Jianjun

> 
> or something like that (note that I've always used the same type for
> next and last, by the way).
> 
> Paolo
>