Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1491418 > unrolled thread

Re: [RFC PATCH 6/6] perf: Disable IRQs in address filter sync path

Started byAlexander Shishkin <alexander.shishkin@linux.intel.com>
First post2016-09-26 18:30 +0200
Last post2016-10-04 18:50 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [RFC PATCH 6/6] perf: Disable IRQs in address filter sync path Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-09-26 18:30 +0200
    Re: [RFC PATCH 6/6] perf: Disable IRQs in address filter sync path Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-10-04 18:50 +0200

#1491418 — Re: [RFC PATCH 6/6] perf: Disable IRQs in address filter sync path

FromAlexander Shishkin <alexander.shishkin@linux.intel.com>
Date2016-09-26 18:30 +0200
SubjectRe: [RFC PATCH 6/6] perf: Disable IRQs in address filter sync path
Message-ID<slGX0-67A-21@gated-at.bofh.it>
Alexander Shishkin <alexander.shishkin@linux.intel.com> writes:

> If PMU callbacks are executed in hardirq context, the address filter
> sync code needs to disable interrupts when taking its spinlock to be
> consistent with the rest of its users. This may happen if the PMU is
> used in AUX sampling.

Hi Mathieu,

I've been meaning to CC you on this series and forgot. My concern was
that on PMUs that run PMIs in hardirq context this patch should be
required already now. Is this the case for you?

>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> ---
>  kernel/events/core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index f6582df1c9..047c495c94 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -2568,16 +2568,17 @@ static int perf_event_stop(struct perf_event *event, int restart)
>  void perf_event_addr_filters_sync(struct perf_event *event)
>  {
>  	struct perf_addr_filters_head *ifh = perf_event_addr_filters(event);
> +	unsigned long flags;
>  
>  	if (!has_addr_filter(event))
>  		return;
>  
> -	raw_spin_lock(&ifh->lock);
> +	raw_spin_lock_irqsave(&ifh->lock, flags);
>  	if (event->addr_filters_gen != event->hw.addr_filters_gen) {
>  		event->pmu->addr_filters_sync(event);
>  		event->hw.addr_filters_gen = event->addr_filters_gen;
>  	}
> -	raw_spin_unlock(&ifh->lock);
> +	raw_spin_unlock_irqrestore(&ifh->lock, flags);
>  }
>  EXPORT_SYMBOL_GPL(perf_event_addr_filters_sync);
>  
> -- 
> 2.9.3

[toc] | [next] | [standalone]


#1495523

FromMathieu Poirier <mathieu.poirier@linaro.org>
Date2016-10-04 18:50 +0200
Message-ID<soB4J-5is-5@gated-at.bofh.it>
In reply to#1491418
On 26 September 2016 at 10:18, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> Alexander Shishkin <alexander.shishkin@linux.intel.com> writes:
>
>> If PMU callbacks are executed in hardirq context, the address filter
>> sync code needs to disable interrupts when taking its spinlock to be
>> consistent with the rest of its users. This may happen if the PMU is
>> used in AUX sampling.
>
> Hi Mathieu,
>
> I've been meaning to CC you on this series and forgot. My concern was
> that on PMUs that run PMIs in hardirq context this patch should be
> required already now. Is this the case for you?

At this time cross triggers haven't been implemented and as such, PMIs
aren't an issue.  On the flip side I can see the value of your code
when we get around to do the implementation.

Mathieu

>
>>
>> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> ---
>>  kernel/events/core.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>> index f6582df1c9..047c495c94 100644
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -2568,16 +2568,17 @@ static int perf_event_stop(struct perf_event *event, int restart)
>>  void perf_event_addr_filters_sync(struct perf_event *event)
>>  {
>>       struct perf_addr_filters_head *ifh = perf_event_addr_filters(event);
>> +     unsigned long flags;
>>
>>       if (!has_addr_filter(event))
>>               return;
>>
>> -     raw_spin_lock(&ifh->lock);
>> +     raw_spin_lock_irqsave(&ifh->lock, flags);
>>       if (event->addr_filters_gen != event->hw.addr_filters_gen) {
>>               event->pmu->addr_filters_sync(event);
>>               event->hw.addr_filters_gen = event->addr_filters_gen;
>>       }
>> -     raw_spin_unlock(&ifh->lock);
>> +     raw_spin_unlock_irqrestore(&ifh->lock, flags);
>>  }
>>  EXPORT_SYMBOL_GPL(perf_event_addr_filters_sync);
>>
>> --
>> 2.9.3

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web