Date: Thu, 9 Apr 2015 23:01:17 +0200
From: Andi Kleen <andi@firstfloor.org>
To: Kan Liang <kan.liang@intel.com>
Cc: a.p.zijlstra@chello.nl, linux-kernel@vger.kernel.org, mingo@kernel.org,
        acme@infradead.org, eranian@google.com, andi@firstfloor.org
Subject: Re: [PATCH V6 4/6] perf, x86: handle multiple records in PEBS buffer
Message-ID: <20150409210116.GX2366@two.firstfloor.org>
References: <1428597466-8154-1-git-send-email-kan.liang@intel.com>
 <1428597466-8154-5-git-send-email-kan.liang@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1428597466-8154-5-git-send-email-kan.liang@intel.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1395
Lines: 57

> +	for_each_set_bit(bit, (unsigned long *)&pebs_status, 64) {
> +
> +		if (bit >= x86_pmu.max_pebs_events)
> +			clear_bit(bit, (unsigned long *)&pebs_status);
> +		else {
> +			event = cpuc->events[bit];
> +			WARN_ON_ONCE(!event);
> +
> +			if (!event->attr.precise_ip)
> +				clear_bit(bit, (unsigned long *)&pebs_status);

Precompute the mask of non pebs events first in the caller.
Then this function would be just a & ~mask

BTW clear_bit is atomic, if you're local you should always use
__clear_bit.


> +}
> +
> +static inline void *
> +get_next_pebs_record_by_bit(void *base, void *top, int bit)
> +{
> +	void *at;
> +	u64 pebs_status;
> +
> +	if (base == NULL)
> +		return NULL;
> +
> +	for (at = base; at < top; at += x86_pmu.pebs_record_size) {
> +		struct pebs_record_nhm *p = at;
> +
> +		if (p->status & (1 << bit)) {

Use test_bit.

> +
> +			if (p->status == (1 << bit))
> +				return at;
> +
> +			/* clear non-PEBS bit and re-check */
> +			pebs_status = nonpebs_bit_clear(p->status);
> +			if (pebs_status == (1 << bit))
> +				return at;
> +		}
> +	}
> +	return NULL;

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.
--
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/