Re: Re: [PATCH 2/6] trace: fix T_INFO_FIRST_OFFSET

[George Dunlap <george.dunlap@eu.citrix.com>]

Jan,

I've got all the ones I've acked / sent my own versions of in a 
mercurial queue, easy to patchbomb.  Let me know if you have any more 
feedback on my versions.

My only objection to the last one was the volatile stuff; if you're OK 
with it, I'll send a series with everything but the volatile stuff, and 
we can continue talking about it.

 -George

Keir Fraser wrote:
> I've lost track of all these trace patches. Please send a new set of patches
> with finalised Sign-offs and Acks as appropriate when you reach agreement.
>
>  -- Keir
>
> On 30/06/2010 17:12, "George Dunlap" <George.Dunlap@eu.citrix.com> wrote:
>
>   
>> Oops, please use this version, which used the appropraite gkprintk()
>> settings...
>>
>>  -George
>>
>> On Wed, Jun 30, 2010 at 5:10 PM, George Dunlap
>> <George.Dunlap@eu.citrix.com> wrote:
>>     
>>> Here's a version that calculates t_info_first_offset during
>>> initialization, based on the actual layout of struct t_info and
>>> NR_CPUs.
>>>
>>>  -George
>>>
>>> On Wed, Jun 30, 2010 at 4:28 PM, George Dunlap
>>> <george.dunlap@eu.citrix.com> wrote:
>>>       
>>>> Jan Beulich wrote:
>>>>         
>>>>> That part your patch doesn't address either - rather than
>>>>> sizeof(uint16_t) as the first part of the expression you'd need to
>>>>> use sizeof(struct t_info) or offsetof(struct t_info, mfn_offset).
>>>>>
>>>>>           
>>>> I was assuming that when someone changed struct t_info that they'd modify
>>>> this macro as well; I suppose then that the two complaints are really
>>>> different aspects of the same one -- that it might not be clear to the
>>>> person who adjusts struct t_info how to translate those changes into
>>>> T_INFO_FIRST_OFFSET.  I think this way is more clear.
>>>>
>>>> I suppose even better might be to calculate t_info.mfn_mfn_offset[NR_CPUS]
>>>> (or perhaps ...[num_possible_cpus]).  Hmm... let me see what I can come up
>>>> with.
>>>>         
>>>>> Btw., didn't we agree that public headers shouldn't make use of
>>>>> language extensions? struct t_info uses a variable sized array,
>>>>> which is an extension (standard only in C99).
>>>>>
>>>>>           
>>>> I'm not an expert in this.  It's lot more hassle to lay out the data the way
>>>> I'd like without it.  I'll defer judgment to Keir.
>>>>
>>>> -George
>>>>
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xensource.com
>>>> http://lists.xensource.com/xen-devel
>>>>
>>>>         
>
>
>