Hello, > > > > I think we need to hide this into an arch_specific function. Which can > > then have a indirect function pointer for > > cell just like what you do today on PPC. > > > > But I don't understand why you need this extra check JUST for > > pfm_read_pmds() and not for all 3 accesses. > > Could you elaborate on that? > > I think that pfm_write_pmcs() and pfm_write_pmds() need this extra check too. > I did not notice that because I didn't get the problem by pfmon. > But If the user program calls them on its way, the problem happen. > > I will update the patch tomorrow.
I updated it.
Please review again!
Thanks.
Takashi Yamamoto.
------------------------------
This patch changes the check of PMU access in pfm_read_pmds(),
pfm_write_pmds(), pfm_write_pmcs().
In the cell-spe-follow mode,
we need to check whether the target SPU program is running
before pmc/pmd access.
Signed-off-by: Takashi Yamamoto <[EMAIL PROTECTED]>
---
arch/powerpc/perfmon/perfmon_cell.c | 23 ++++++++++++++++++++
include/asm-powerpc/perfmon_kern.h | 17 ++++++++++++++
perfmon/perfmon_rw.c | 41 +++++++++++++++++++++++++++++++++---
3 files changed, 78 insertions(+), 3 deletions(-)
--- a/arch/powerpc/perfmon/perfmon_cell.c
+++ b/arch/powerpc/perfmon/perfmon_cell.c
@@ -1415,6 +1415,28 @@ void pfm_cell_release_pmu(void)
}
/**
+ * pfm_cell_can_access_pmu
+ *
+ * In the per-thread & spe-follow mode,
+ * if the target spu program is running, the pmc/pmd register
+ * access is allowed.
+ **/
+int pfm_cell_can_access_pmu(struct pfm_context *ctx, int can_access)
+{
+ struct pfm_arch_context *ctx_arch = pfm_ctx_arch(ctx);
+
+ if (!ctx->flags.system && ctx->flags.not_dflt_ctxsw)
+ if (ctx_arch->arg)
+ /* the target spu program is running */
+ return 1;
+ else
+ /* the traget spu program is NOT running */
+ return 0;
+ else
+ return can_access;
+}
+
+/**
* handle_trace_buffer_interrupts
*
* This routine is for processing just the interval timer and trace buffer
@@ -1548,6 +1570,7 @@ static struct pfm_cell_platform_pmu_info
static struct pfm_arch_pmu_info pfm_cell_pmu_info = {
.pmu_style = PFM_POWERPC_PMU_CELL,
+ .can_access_pmu = pfm_cell_can_access_pmu,
.acquire_pmu = pfm_cell_acquire_pmu,
.release_pmu = pfm_cell_release_pmu,
.create_context = pfm_cell_create_context,
--- a/include/asm-powerpc/perfmon_kern.h
+++ b/include/asm-powerpc/perfmon_kern.h
@@ -120,6 +120,7 @@ struct pfm_arch_pmu_info {
void (*free_context)(struct pfm_context *ctx);
int (*acquire_pmu)(void);
void (*release_pmu)(void);
+ int (*can_access_pmu)(struct pfm_context *ctx, int can_access);
void *platform_info;
};
@@ -427,6 +428,22 @@ struct pfm_arch_context {
void *arg;
};
+/*
+ * This function returns whether the pmc/pmd register access is allowed.
+ */
+static inline int pfm_arch_can_access_pmu(
+ struct pfm_context *ctx, int can_access)
+{
+ struct pfm_arch_pmu_info *arch_info;
+
+ arch_info = pfm_pmu_info();
+
+ if (arch_info->can_access_pmu)
+ return arch_info->can_access_pmu(ctx, can_access);
+ else
+ return can_access;
+}
+
#define PFM_ARCH_CTX_SIZE sizeof(struct pfm_arch_context)
/*
* PowerPC does not need extra alignment requirements for the sampling buffer
--- a/perfmon/perfmon_rw.c
+++ b/perfmon/perfmon_rw.c
@@ -83,10 +83,22 @@ int __pfm_write_pmds(struct pfm_context
/*
* we cannot access the actual PMD registers when monitoring is masked
*/
- if (unlikely(ctx->state == PFM_CTX_LOADED))
+ if (unlikely(ctx->state == PFM_CTX_LOADED)) {
can_access_pmu = __get_cpu_var(pmu_owner) == ctx->task
|| ctx->flags.system;
+#ifdef CONFIG_PPC
+ /*
+ * Architecture depended access check.
+ *
+ * cell_spe_follow mode needs this check.
+ * Because the target SPU program may run on
+ * the other CPU's SPU.
+ */
+ can_access_pmu = pfm_arch_can_access_pmu(ctx, can_access_pmu);
+#endif
+ }
+
ret = -EINVAL;
for (i = 0; i < count; i++, req++) {
@@ -355,10 +367,22 @@ int __pfm_write_pmcs(struct pfm_context
/*
* we cannot access the actual PMC registers when monitoring is masked
*/
- if (unlikely(ctx->state == PFM_CTX_LOADED))
+ if (unlikely(ctx->state == PFM_CTX_LOADED)) {
can_access_pmu = __get_cpu_var(pmu_owner) == ctx->task
|| ctx->flags.system;
+#ifdef CONFIG_PPC
+ /*
+ * Architecture depended access check.
+ *
+ * cell_spe_follow mode needs this check.
+ * Because the target SPU program may run on
+ * the other CPU's SPU.
+ */
+ can_access_pmu = pfm_arch_can_access_pmu(ctx, can_access_pmu);
+#endif
+ }
+
ret = -EINVAL;
for (i = 0; i < count; i++, req++) {
@@ -506,7 +530,18 @@ int __pfm_read_pmds(struct pfm_context *
if (likely(ctx->state == PFM_CTX_LOADED)) {
can_access_pmu = __get_cpu_var(pmu_owner) == ctx->task
- || ctx->flags.system;
+ || ctx->flags.system;
+
+#ifdef CONFIG_PPC
+ /*
+ * Architecture depended access check.
+ *
+ * cell_spe_follow mode needs this check.
+ * Because the target SPU program may run on
+ * the other CPU's SPU.
+ */
+ can_access_pmu = pfm_arch_can_access_pmu(ctx, can_access_pmu);
+#endif
if (can_access_pmu)
pfm_arch_serialize();
cell-change_read_pmds.patch
Description: Binary data
------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________ perfmon2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
