Date: Thu, 27 Aug 2015 19:54:31 +0200
From: Jiri Olsa <jolsa@redhat.com>
To: Kan Liang <kan.liang@intel.com>
Cc: acme@kernel.org, jolsa@kernel.org, a.p.zijlstra@chello.nl, luto@kernel.org,
        mingo@redhat.com, eranian@google.com, ak@linux.intel.com,
        mark.rutland@arm.com, adrian.hunter@intel.com, namhyung@kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH V5 1/7] perf,tools: introduce generic FEAT for CPU
 attributes
Message-ID: <20150827175431.GC25932@krava.redhat.com>
References: <1440494288-20448-1-git-send-email-kan.liang@intel.com>
 <1440494288-20448-2-git-send-email-kan.liang@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1440494288-20448-2-git-send-email-kan.liang@intel.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1319
Lines: 49

On Tue, Aug 25, 2015 at 05:18:02AM -0400, Kan Liang wrote:

SNIP

>  
> +static int process_cpu_attributes(struct perf_file_section *section __maybe_unused,
> +				  struct perf_header *ph, int fd,
> +				  void *data __maybe_unused)
> +{
> +	ssize_t ret;
> +	u32 tag_id;
> +	u64 nr;
> +
> +
> +	ret = readn(fd, &tag_id, sizeof(tag_id));
> +	if (ret != sizeof(tag_id))
> +		return -1;
> +
> +	if (ph->needs_swap)
> +		nr = bswap_32(tag_id);
> +
> +	if (tag_id >= PERF_HEADER_CPU_ATTR_MAX) {
> +		pr_debug("The number of cpu attributes is not expected. "
> +			 "You may need to upgrade the perf tool.\n");
> +		return -1;
> +	}
> +
> +	ret = readn(fd, &nr, sizeof(nr));
> +	if (ret != sizeof(nr))
> +		return -1;
> +
> +	if (ph->needs_swap)
> +		nr = bswap_64(nr);
> +
> +	ph->env.cpu_attr[tag_id] = nr;

this should be in the loop right? process it PERF_HEADER_CPU_ATTR_MAX times

I understand it's enough for now when there's only single attr,
but could you please put it into the loop, so the next time
we add cpu attribute we dont need to add this logic?

thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/