From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH 3/3] xenpm: use new Cx statistics interface Date: Wed, 05 Mar 2014 12:05:34 -0500 Message-ID: <5317595E.2090006@oracle.com> References: <53170BB40200007800121204@nat28.tlf.novell.com> <53170C8A020000780012121A@nat28.tlf.novell.com> <53174725.4010500@oracle.com> <5317569C02000078001213E4@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WLFEK-0002ru-UK for xen-devel@lists.xenproject.org; Wed, 05 Mar 2014 17:03:41 +0000 In-Reply-To: <5317569C02000078001213E4@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Keir Fraser , Ian Campbell , Ian Jackson , Donald D Dugger , Jun Nakajima , xen-devel List-Id: xen-devel@lists.xenproject.org On 03/05/2014 10:53 AM, Jan Beulich wrote: >>>> On 05.03.14 at 16:47, Boris Ostrovsky wrote: >> On 03/05/2014 05:37 AM, Jan Beulich wrote: >>> @@ -331,7 +346,7 @@ void pxstat_func(int argc, char *argv[]) >>> } >>> >>> static uint64_t usec_start, usec_end; >>> -static struct xc_cx_stat *cxstat, *cxstat_start, *cxstat_end; >>> +static struct xc_cx_stat_v2 *cxstat, *cxstat_start, *cxstat_end; >>> static struct xc_px_stat *pxstat, *pxstat_start, *pxstat_end; >>> static int *avgfreq; >>> static uint64_t *sum, *sum_cx, *sum_px; >>> @@ -482,25 +497,26 @@ static void signal_int_handler(int signo >>> /* print out CC? and PC? */ >>> for ( i = 0; i < socket_nr; i++ ) >>> { >>> + unsigned int n; >>> uint64_t res; >>> + >>> for ( j = 0; j <= info.max_cpu_index; j++ ) >>> { >>> if ( cpu_to_socket[j] == socket_ids[i] ) >>> break; >>> } >>> printf("\nSocket %d\n", socket_ids[i]); >>> - res = cxstat_end[j].pc2 - cxstat_start[j].pc2; >>> - printf("\tPC2\t%"PRIu64" ms\t%.2f%%\n", res / 1000000UL, >>> - 100UL * res / (double)sum_cx[j]); >>> - res = cxstat_end[j].pc3 - cxstat_start[j].pc3; >>> - printf("\tPC3\t%"PRIu64" ms\t%.2f%%\n", res / 1000000UL, >>> - 100UL * res / (double)sum_cx[j]); >>> - res = cxstat_end[j].pc6 - cxstat_start[j].pc6; >>> - printf("\tPC6\t%"PRIu64" ms\t%.2f%%\n", res / 1000000UL, >>> - 100UL * res / (double)sum_cx[j]); >>> - res = cxstat_end[j].pc7 - cxstat_start[j].pc7; >>> - printf("\tPC7\t%"PRIu64" ms\t%.2f%%\n", res / 1000000UL, >>> - 100UL * res / (double)sum_cx[j]); >>> + for ( n = 0; n < MAX_PKG_RESIDENCIES; ++n ) >>> + { >>> + if ( n >= cxstat_end[j].nr_pc ) >>> + continue; >>> + res = cxstat_end[j].pc[n]; >>> + if ( n < cxstat_start[j].nr_pc ) >>> + res -= cxstat_start[j].pc[n]; >> Is it possible to have cxstat_end[j].nr_pc != cxstat_start[j].nr_pc ? > Yes - see the previous patch: It bumps the count only if the > respective hw_res field was non-zero. You mean this? + +#define PUT_xC(what, n) do { \ + if ( stat->nr_##what >= n && \ + copy_to_guest_offset(stat->what, n - 1, &hw_res.what##n, 1) ) \ + return -EFAULT; \ + if ( hw_res.what##n ) \ + nr_##what = n; \ + } while ( 0 ) +#define PUT_PC(n) PUT_xC(pc, n) This reminds me of another question I had about this patch: this fragment appears to assume that you call it in order. In other words, will it work as intended if your call sequence is PUT_PC(10) .. PUT_PC(1) -boris > > But even if the current implementation didn't allow for this, I'd > still consider it good practice to cope with the possibility. >