Re: [Xen-devel] Re: [PATCH 2/6] trace: fix T_INFO_FIRST_OFFSET

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx
> > > > http://lists.xensource.com/xen-devel
> > > >
> > > >         
>   
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel