Re: [PATCH 06/14] perf, core: always switch pmu specific data during context switch

2014-02-05 Thread Yan, Zheng
On 02/06/2014 02:35 AM, Stephane Eranian wrote:
> On Wed, Feb 5, 2014 at 6:55 PM, Peter Zijlstra  wrote:
>> On Wed, Feb 05, 2014 at 06:19:27PM +0100, Stephane Eranian wrote:
>>> On Fri, Jan 3, 2014 at 6:48 AM, Yan, Zheng  wrote:
 If two tasks were both forked from the same parent task, Events in their 
 perf
 task contexts can be the same. Perf core optimizes context switch oout in 
 this
 case.

 Previous patch inroduces pmu specific data. The data is task specific, so 
 we
 should switch the data even when context switch is optimized out.

>>> Reviwed-by: Stephane Eranian 
>>
>> You should look again.. that xchg() is an atomic op and a total waste of
>> time since the assignment back onto ctx->task_ctx_data is non-atomic.
>>
>> Complete fail there.
>>
> I admit, it was not clear to me why the xchg().

Sorry. I forget why I used xhcg(), maybe save a few lines of code.

Regards
Yan, Zheng

> 
 Signed-off-by: Yan, Zheng 
 ---
  kernel/events/core.c | 2 ++
  1 file changed, 2 insertions(+)

 diff --git a/kernel/events/core.c b/kernel/events/core.c
 index b6650ab..d6d8dea 100644
 --- a/kernel/events/core.c
 +++ b/kernel/events/core.c
 @@ -2319,6 +2319,8 @@ static void perf_event_context_sched_out(struct 
 task_struct *task, int ctxn,
 next->perf_event_ctxp[ctxn] = ctx;
 ctx->task = next;
 next_ctx->task = task;
 +   ctx->task_ctx_data = xchg(&next_ctx->task_ctx_data,
 + ctx->task_ctx_data);
 do_switch = 0;

 perf_event_sync_stat(ctx, next_ctx);
 --
 1.8.4.2


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


Re: [PATCH 06/14] perf, core: always switch pmu specific data during context switch

2014-02-05 Thread Stephane Eranian
On Wed, Feb 5, 2014 at 6:55 PM, Peter Zijlstra  wrote:
> On Wed, Feb 05, 2014 at 06:19:27PM +0100, Stephane Eranian wrote:
>> On Fri, Jan 3, 2014 at 6:48 AM, Yan, Zheng  wrote:
>> > If two tasks were both forked from the same parent task, Events in their 
>> > perf
>> > task contexts can be the same. Perf core optimizes context switch oout in 
>> > this
>> > case.
>> >
>> > Previous patch inroduces pmu specific data. The data is task specific, so 
>> > we
>> > should switch the data even when context switch is optimized out.
>> >
>> Reviwed-by: Stephane Eranian 
>
> You should look again.. that xchg() is an atomic op and a total waste of
> time since the assignment back onto ctx->task_ctx_data is non-atomic.
>
> Complete fail there.
>
I admit, it was not clear to me why the xchg().

>> > Signed-off-by: Yan, Zheng 
>> > ---
>> >  kernel/events/core.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/kernel/events/core.c b/kernel/events/core.c
>> > index b6650ab..d6d8dea 100644
>> > --- a/kernel/events/core.c
>> > +++ b/kernel/events/core.c
>> > @@ -2319,6 +2319,8 @@ static void perf_event_context_sched_out(struct 
>> > task_struct *task, int ctxn,
>> > next->perf_event_ctxp[ctxn] = ctx;
>> > ctx->task = next;
>> > next_ctx->task = task;
>> > +   ctx->task_ctx_data = xchg(&next_ctx->task_ctx_data,
>> > + ctx->task_ctx_data);
>> > do_switch = 0;
>> >
>> > perf_event_sync_stat(ctx, next_ctx);
>> > --
>> > 1.8.4.2
>> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 06/14] perf, core: always switch pmu specific data during context switch

2014-02-05 Thread Peter Zijlstra
On Wed, Feb 05, 2014 at 06:19:27PM +0100, Stephane Eranian wrote:
> On Fri, Jan 3, 2014 at 6:48 AM, Yan, Zheng  wrote:
> > If two tasks were both forked from the same parent task, Events in their 
> > perf
> > task contexts can be the same. Perf core optimizes context switch oout in 
> > this
> > case.
> >
> > Previous patch inroduces pmu specific data. The data is task specific, so we
> > should switch the data even when context switch is optimized out.
> >
> Reviwed-by: Stephane Eranian 

You should look again.. that xchg() is an atomic op and a total waste of
time since the assignment back onto ctx->task_ctx_data is non-atomic.

Complete fail there.

> > Signed-off-by: Yan, Zheng 
> > ---
> >  kernel/events/core.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index b6650ab..d6d8dea 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -2319,6 +2319,8 @@ static void perf_event_context_sched_out(struct 
> > task_struct *task, int ctxn,
> > next->perf_event_ctxp[ctxn] = ctx;
> > ctx->task = next;
> > next_ctx->task = task;
> > +   ctx->task_ctx_data = xchg(&next_ctx->task_ctx_data,
> > + ctx->task_ctx_data);
> > do_switch = 0;
> >
> > perf_event_sync_stat(ctx, next_ctx);
> > --
> > 1.8.4.2
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 06/14] perf, core: always switch pmu specific data during context switch

2014-02-05 Thread Stephane Eranian
On Fri, Jan 3, 2014 at 6:48 AM, Yan, Zheng  wrote:
> If two tasks were both forked from the same parent task, Events in their perf
> task contexts can be the same. Perf core optimizes context switch oout in this
> case.
>
> Previous patch inroduces pmu specific data. The data is task specific, so we
> should switch the data even when context switch is optimized out.
>
Reviwed-by: Stephane Eranian 
> Signed-off-by: Yan, Zheng 
> ---
>  kernel/events/core.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index b6650ab..d6d8dea 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -2319,6 +2319,8 @@ static void perf_event_context_sched_out(struct 
> task_struct *task, int ctxn,
> next->perf_event_ctxp[ctxn] = ctx;
> ctx->task = next;
> next_ctx->task = task;
> +   ctx->task_ctx_data = xchg(&next_ctx->task_ctx_data,
> + ctx->task_ctx_data);
> do_switch = 0;
>
> perf_event_sync_stat(ctx, next_ctx);
> --
> 1.8.4.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 06/14] perf, core: always switch pmu specific data during context switch

2014-01-02 Thread Yan, Zheng
If two tasks were both forked from the same parent task, Events in their perf
task contexts can be the same. Perf core optimizes context switch oout in this
case.

Previous patch inroduces pmu specific data. The data is task specific, so we
should switch the data even when context switch is optimized out.

Signed-off-by: Yan, Zheng 
---
 kernel/events/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index b6650ab..d6d8dea 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2319,6 +2319,8 @@ static void perf_event_context_sched_out(struct 
task_struct *task, int ctxn,
next->perf_event_ctxp[ctxn] = ctx;
ctx->task = next;
next_ctx->task = task;
+   ctx->task_ctx_data = xchg(&next_ctx->task_ctx_data,
+ ctx->task_ctx_data);
do_switch = 0;
 
perf_event_sync_stat(ctx, next_ctx);
-- 
1.8.4.2

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