| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <52A6AD61.4050408@linux.vnet.ibm.com>
Date: Tue, 10 Dec 2013 11:27:53 +0530
From: Anshuman Khandual <khandual@...ux.vnet.ibm.com>
To: Michael Ellerman <mpe@...erman.id.au>
CC: linuxppc-dev@...abs.org, linux-kernel@...r.kernel.org,
mikey@...ling.org, ak@...ux.intel.com, eranian@...gle.com,
acme@...stprotocols.net, sukadev@...ux.vnet.ibm.com,
mingo@...nel.org
Subject: Re: [PATCH V4 08/10] powerpc, perf: Enable SW filtering in branch
stack sampling framework
On 12/09/2013 11:51 AM, Michael Ellerman wrote:
> This code was already in need of some unindentation, and now it's just
> ridiculous.
>
> To start with at the beginning of this routine we have:
>
> while (..) {
> if (!val)
> break;
> else {
> // Bulk of the logic
> ...
> }
> }
>
> That should almost always become:
>
> while (..) {
> if (!val)
> break;
>
> // Bulk of the logic
> ...
> }
>
>
> But in this case that's not enough. Please send a precursor patch which moves
> this logic out into a helper function.
Hey Michael,
I believe this patch should be able to take care of this.
commit d66d729715cabe0cfd8e34861a6afa8ad639ddf3
Author: Anshuman Khandual <khandual@...ux.vnet.ibm.com>
Date: Tue Dec 10 11:10:06 2013 +0530
power, perf: Clean up BHRB processing
This patch cleans up some indentation problem and re-organizes the
BHRB processing code with an additional helper function.
Signed-off-by: Anshuman Khandual <khandual@...ux.vnet.ibm.com>
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 29b89e8..9ae96c5 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -400,11 +400,20 @@ static __u64 power_pmu_bhrb_to(u64 addr)
return target - (unsigned long)&instr + addr;
}
+void update_branch_entry(struct cpu_hw_events *cpuhw, int u_index, u64 from, u64 to, int pred)
+{
+ cpuhw->bhrb_entries[u_index].from = from;
+ cpuhw->bhrb_entries[u_index].to = to;
+ cpuhw->bhrb_entries[u_index].mispred = pred;
+ cpuhw->bhrb_entries[u_index].predicted = ~pred;
+ return;
+}
+
/* Processing BHRB entries */
void power_pmu_bhrb_read(struct cpu_hw_events *cpuhw)
{
u64 val;
- u64 addr;
+ u64 addr, tmp;
int r_index, u_index, pred;
r_index = 0;
@@ -415,62 +424,54 @@ void power_pmu_bhrb_read(struct cpu_hw_events *cpuhw)
if (!val)
/* Terminal marker: End of valid BHRB entries */
break;
- else {
- addr = val & BHRB_EA;
- pred = val & BHRB_PREDICTION;
- if (!addr)
- /* invalid entry */
- continue;
+ addr = val & BHRB_EA;
+ pred = val & BHRB_PREDICTION;
- /* Branches are read most recent first (ie. mfbhrb 0 is
- * the most recent branch).
- * There are two types of valid entries:
- * 1) a target entry which is the to address of a
- * computed goto like a blr,bctr,btar. The next
- * entry read from the bhrb will be branch
- * corresponding to this target (ie. the actual
- * blr/bctr/btar instruction).
- * 2) a from address which is an actual branch. If a
- * target entry proceeds this, then this is the
- * matching branch for that target. If this is not
- * following a target entry, then this is a branch
- * where the target is given as an immediate field
- * in the instruction (ie. an i or b form branch).
- * In this case we need to read the instruction from
- * memory to determine the target/to address.
+ if (!addr)
+ /* invalid entry */
+ continue;
+
+ /* Branches are read most recent first (ie. mfbhrb 0 is
+ * the most recent branch).
+ * There are two types of valid entries:
+ * 1) a target entry which is the to address of a
+ * computed goto like a blr,bctr,btar. The next
+ * entry read from the bhrb will be branch
+ * corresponding to this target (ie. the actual
+ * blr/bctr/btar instruction).
+ * 2) a from address which is an actual branch. If a
+ * target entry proceeds this, then this is the
+ * matching branch for that target. If this is not
+ * following a target entry, then this is a branch
+ * where the target is given as an immediate field
+ * in the instruction (ie. an i or b form branch).
+ * In this case we need to read the instruction from
+ * memory to determine the target/to address.
+ */
+ if (val & BHRB_TARGET) {
+ /* Target branches use two entries
+ * (ie. computed gotos/XL form)
*/
+ tmp = addr;
+ /* Get from address in next entry */
+ val = read_bhrb(r_index++);
+ addr = val & BHRB_EA;
if (val & BHRB_TARGET) {
- /* Target branches use two entries
- * (ie. computed gotos/XL form)
- */
- cpuhw->bhrb_entries[u_index].to = addr;
- cpuhw->bhrb_entries[u_index].mispred = pred;
- cpuhw->bhrb_entries[u_index].predicted = ~pred;
-
- /* Get from address in next entry */
- val = read_bhrb(r_index++);
- addr = val & BHRB_EA;
- if (val & BHRB_TARGET) {
- /* Shouldn't have two targets in a
- row.. Reset index and try again */
- r_index--;
- addr = 0;
- }
- cpuhw->bhrb_entries[u_index].from = addr;
- } else {
- /* Branches to immediate field
- (ie I or B form) */
- cpuhw->bhrb_entries[u_index].from = addr;
- cpuhw->bhrb_entries[u_index].to =
- power_pmu_bhrb_to(addr);
- cpuhw->bhrb_entries[u_index].mispred = pred;
- cpuhw->bhrb_entries[u_index].predicted = ~pred;
+ /* Shouldn't have two targets in a
+ row.. Reset index and try again */
+ r_index--;
+ addr = 0;
}
- u_index++;
-
+ update_branch_entry(cpuhw, u_index, addr, tmp, pred);
+ } else {
+ /* Branches to immediate field
+ (ie I or B form) */
+ tmp = power_pmu_bhrb_to(addr);
+ update_branch_entry(cpuhw, u_index, addr, tmp, pred);
}
+ u_index++;
}
cpuhw->bhrb_stack.nr = u_index;
return;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists