From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: Re: [PATCH 2/6] trace: fix T_INFO_FIRST_OFFSET Date: Wed, 30 Jun 2010 19:12:51 +0100 Message-ID: <4C2B8923.8020700@eu.citrix.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: "xen-devel@lists.xensource.com" , Jan Beulich List-Id: xen-devel@lists.xenproject.org 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" wrote: > > >> Oops, please use this version, which used the appropraite gkprintk() >> settings... >> >> -George >> >> On Wed, Jun 30, 2010 at 5:10 PM, George Dunlap >> 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 >>> 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 >>>> >>>> > > >