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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 20 Feb 2017 13:42:03 -0700
From:   Mathieu Poirier <mathieu.poirier@...aro.org>
To:     Alexander Shishkin <alexander.shishkin@...ux.intel.com>
Cc:     Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Ingo Molnar <mingo@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Vince Weaver <vince@...ter.net>,
        Stephane Eranian <eranian@...gle.com>,
        Arnaldo Carvalho de Melo <acme@...radead.org>,
        Will Deacon <will.deacon@....com>
Subject: Re: [PATCH 2/4] perf: Keep AUX flags in the output handle

On 20 February 2017 at 06:33, Alexander Shishkin
<alexander.shishkin@...ux.intel.com> wrote:
> From: Will Deacon <will.deacon@....com>
>
> In preparation for adding more flags to perf AUX records, introduce a
> separate API for setting the flags for a session, rather than appending
> more bool arguments to perf_aux_output_end. This allows to set each
> flag at the time a corresponding condition is detected, instead of
> tracking it in each driver's private state.
>
> Cc: Mathieu Poirier <mathieu.poirier@...aro.org>
> Signed-off-by: Will Deacon <will.deacon@....com>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> ---
>  arch/x86/events/intel/bts.c                      | 16 +++++------
>  arch/x86/events/intel/pt.c                       | 17 ++++++------
>  arch/x86/events/intel/pt.h                       |  1 -
>  drivers/hwtracing/coresight/coresight-etb10.c    |  7 +++--
>  drivers/hwtracing/coresight/coresight-etm-perf.c |  9 +++----
>  drivers/hwtracing/coresight/coresight-priv.h     |  2 --
>  drivers/hwtracing/coresight/coresight-tmc-etf.c  |  7 +++--

For the CS files:
Acked-by: Mathieu Poirier <mathieu.poirier@...aro.org>

>  include/linux/coresight.h                        |  2 +-
>  include/linux/perf_event.h                       |  8 +++---
>  kernel/events/ring_buffer.c                      | 34 ++++++++++++++++--------
>  10 files changed, 55 insertions(+), 48 deletions(-)
>
> diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
> index 982c9e31da..8ae8c5ce3a 100644
> --- a/arch/x86/events/intel/bts.c
> +++ b/arch/x86/events/intel/bts.c
> @@ -63,7 +63,6 @@ struct bts_buffer {
>         unsigned int    cur_buf;
>         bool            snapshot;
>         local_t         data_size;
> -       local_t         lost;
>         local_t         head;
>         unsigned long   end;
>         void            **data_pages;
> @@ -199,7 +198,8 @@ static void bts_update(struct bts_ctx *bts)
>                         return;
>
>                 if (ds->bts_index >= ds->bts_absolute_maximum)
> -                       local_inc(&buf->lost);
> +                       perf_aux_output_flag(&bts->handle,
> +                                            PERF_AUX_FLAG_TRUNCATED);
>
>                 /*
>                  * old and head are always in the same physical buffer, so we
> @@ -276,7 +276,7 @@ static void bts_event_start(struct perf_event *event, int flags)
>         return;
>
>  fail_end_stop:
> -       perf_aux_output_end(&bts->handle, 0, false);
> +       perf_aux_output_end(&bts->handle, 0);
>
>  fail_stop:
>         event->hw.state = PERF_HES_STOPPED;
> @@ -319,9 +319,8 @@ static void bts_event_stop(struct perf_event *event, int flags)
>                                 bts->handle.head =
>                                         local_xchg(&buf->data_size,
>                                                    buf->nr_pages << PAGE_SHIFT);
> -
> -                       perf_aux_output_end(&bts->handle, local_xchg(&buf->data_size, 0),
> -                                           !!local_xchg(&buf->lost, 0));
> +                       perf_aux_output_end(&bts->handle,
> +                                           local_xchg(&buf->data_size, 0));
>                 }
>
>                 cpuc->ds->bts_index = bts->ds_back.bts_buffer_base;
> @@ -484,8 +483,7 @@ int intel_bts_interrupt(void)
>         if (old_head == local_read(&buf->head))
>                 return handled;
>
> -       perf_aux_output_end(&bts->handle, local_xchg(&buf->data_size, 0),
> -                           !!local_xchg(&buf->lost, 0));
> +       perf_aux_output_end(&bts->handle, local_xchg(&buf->data_size, 0));
>
>         buf = perf_aux_output_begin(&bts->handle, event);
>         if (buf)
> @@ -500,7 +498,7 @@ int intel_bts_interrupt(void)
>                          * cleared handle::event
>                          */
>                         barrier();
> -                       perf_aux_output_end(&bts->handle, 0, false);
> +                       perf_aux_output_end(&bts->handle, 0);
>                 }
>         }
>
> diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
> index d92a60ef08..a2d0050fde 100644
> --- a/arch/x86/events/intel/pt.c
> +++ b/arch/x86/events/intel/pt.c
> @@ -823,7 +823,8 @@ static void pt_handle_status(struct pt *pt)
>                  */
>                 if (!pt_cap_get(PT_CAP_topa_multiple_entries) ||
>                     buf->output_off == sizes(TOPA_ENTRY(buf->cur, buf->cur_idx)->size)) {
> -                       local_inc(&buf->lost);
> +                       perf_aux_output_flag(&pt->handle,
> +                                            PERF_AUX_FLAG_TRUNCATED);
>                         advance++;
>                 }
>         }
> @@ -916,8 +917,10 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
>
>         /* can't stop in the middle of an output region */
>         if (buf->output_off + handle->size + 1 <
> -           sizes(TOPA_ENTRY(buf->cur, buf->cur_idx)->size))
> +           sizes(TOPA_ENTRY(buf->cur, buf->cur_idx)->size)) {
> +               perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>                 return -EINVAL;
> +       }
>
>
>         /* single entry ToPA is handled by marking all regions STOP=1 INT=1 */
> @@ -1269,8 +1272,7 @@ void intel_pt_interrupt(void)
>
>         pt_update_head(pt);
>
> -       perf_aux_output_end(&pt->handle, local_xchg(&buf->data_size, 0),
> -                           local_xchg(&buf->lost, 0));
> +       perf_aux_output_end(&pt->handle, local_xchg(&buf->data_size, 0));
>
>         if (!event->hw.state) {
>                 int ret;
> @@ -1285,7 +1287,7 @@ void intel_pt_interrupt(void)
>                 /* snapshot counters don't use PMI, so it's safe */
>                 ret = pt_buffer_reset_markers(buf, &pt->handle);
>                 if (ret) {
> -                       perf_aux_output_end(&pt->handle, 0, true);
> +                       perf_aux_output_end(&pt->handle, 0);
>                         return;
>                 }
>
> @@ -1357,7 +1359,7 @@ static void pt_event_start(struct perf_event *event, int mode)
>         return;
>
>  fail_end_stop:
> -       perf_aux_output_end(&pt->handle, 0, true);
> +       perf_aux_output_end(&pt->handle, 0);
>  fail_stop:
>         hwc->state = PERF_HES_STOPPED;
>  }
> @@ -1398,8 +1400,7 @@ static void pt_event_stop(struct perf_event *event, int mode)
>                         pt->handle.head =
>                                 local_xchg(&buf->data_size,
>                                            buf->nr_pages << PAGE_SHIFT);
> -               perf_aux_output_end(&pt->handle, local_xchg(&buf->data_size, 0),
> -                                   local_xchg(&buf->lost, 0));
> +               perf_aux_output_end(&pt->handle, local_xchg(&buf->data_size, 0));
>         }
>  }
>
> diff --git a/arch/x86/events/intel/pt.h b/arch/x86/events/intel/pt.h
> index fa81f8d8ed..25fa9710f4 100644
> --- a/arch/x86/events/intel/pt.h
> +++ b/arch/x86/events/intel/pt.h
> @@ -144,7 +144,6 @@ struct pt_buffer {
>         size_t                  output_off;
>         unsigned long           nr_pages;
>         local_t                 data_size;
> -       local_t                 lost;
>         local64_t               head;
>         bool                    snapshot;
>         unsigned long           stop_pos, intr_pos;
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index d7325c6534..82c8ddcf09 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -321,7 +321,7 @@ static int etb_set_buffer(struct coresight_device *csdev,
>
>  static unsigned long etb_reset_buffer(struct coresight_device *csdev,
>                                       struct perf_output_handle *handle,
> -                                     void *sink_config, bool *lost)
> +                                     void *sink_config)
>  {
>         unsigned long size = 0;
>         struct cs_buffers *buf = sink_config;
> @@ -343,7 +343,6 @@ static unsigned long etb_reset_buffer(struct coresight_device *csdev,
>                  * resetting parameters here and squaring off with the ring
>                  * buffer API in the tracer PMU is fine.
>                  */
> -               *lost = !!local_xchg(&buf->lost, 0);
>                 size = local_xchg(&buf->data_size, 0);
>         }
>
> @@ -385,7 +384,7 @@ static void etb_update_buffer(struct coresight_device *csdev,
>                         (unsigned long)write_ptr);
>
>                 write_ptr &= ~(ETB_FRAME_SIZE_WORDS - 1);
> -               local_inc(&buf->lost);
> +               perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>         }
>
>         /*
> @@ -396,7 +395,7 @@ static void etb_update_buffer(struct coresight_device *csdev,
>          */
>         status = readl_relaxed(drvdata->base + ETB_STATUS_REG);
>         if (status & ETB_STATUS_RAM_FULL) {
> -               local_inc(&buf->lost);
> +               perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>                 to_read = capacity;
>                 read_ptr = write_ptr;
>         } else {
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index 70eaa74dc2..47ea0eee67 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -301,7 +301,8 @@ static void etm_event_start(struct perf_event *event, int flags)
>         return;
>
>  fail_end_stop:
> -       perf_aux_output_end(handle, 0, true);
> +       perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
> +       perf_aux_output_end(handle, 0);
>  fail:
>         event->hw.state = PERF_HES_STOPPED;
>         goto out;
> @@ -309,7 +310,6 @@ static void etm_event_start(struct perf_event *event, int flags)
>
>  static void etm_event_stop(struct perf_event *event, int mode)
>  {
> -       bool lost;
>         int cpu = smp_processor_id();
>         unsigned long size;
>         struct coresight_device *sink, *csdev = per_cpu(csdev_src, cpu);
> @@ -347,10 +347,9 @@ static void etm_event_stop(struct perf_event *event, int mode)
>                         return;
>
>                 size = sink_ops(sink)->reset_buffer(sink, handle,
> -                                                   event_data->snk_config,
> -                                                   &lost);
> +                                                   event_data->snk_config);
>
> -               perf_aux_output_end(handle, size, lost);
> +               perf_aux_output_end(handle, size);
>         }
>
>         /* Disabling the path make its elements available to other sessions */
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index ef9d8e93e3..5f662d8205 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -76,7 +76,6 @@ enum cs_mode {
>   * @nr_pages:  max number of pages granted to us
>   * @offset:    offset within the current buffer
>   * @data_size: how much we collected in this run
> - * @lost:      other than zero if we had a HW buffer wrap around
>   * @snapshot:  is this run in snapshot mode
>   * @data_pages:        a handle the ring buffer
>   */
> @@ -85,7 +84,6 @@ struct cs_buffers {
>         unsigned int            nr_pages;
>         unsigned long           offset;
>         local_t                 data_size;
> -       local_t                 lost;
>         bool                    snapshot;
>         void                    **data_pages;
>  };
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index 1549436e24..aec61a6d5c 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -329,7 +329,7 @@ static int tmc_set_etf_buffer(struct coresight_device *csdev,
>
>  static unsigned long tmc_reset_etf_buffer(struct coresight_device *csdev,
>                                           struct perf_output_handle *handle,
> -                                         void *sink_config, bool *lost)
> +                                         void *sink_config)
>  {
>         long size = 0;
>         struct cs_buffers *buf = sink_config;
> @@ -350,7 +350,6 @@ static unsigned long tmc_reset_etf_buffer(struct coresight_device *csdev,
>                  * resetting parameters here and squaring off with the ring
>                  * buffer API in the tracer PMU is fine.
>                  */
> -               *lost = !!local_xchg(&buf->lost, 0);
>                 size = local_xchg(&buf->data_size, 0);
>         }
>
> @@ -389,7 +388,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>          */
>         status = readl_relaxed(drvdata->base + TMC_STS);
>         if (status & TMC_STS_FULL) {
> -               local_inc(&buf->lost);
> +               perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>                 to_read = drvdata->size;
>         } else {
>                 to_read = CIRC_CNT(write_ptr, read_ptr, drvdata->size);
> @@ -434,7 +433,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>                         read_ptr -= drvdata->size;
>                 /* Tell the HW */
>                 writel_relaxed(read_ptr, drvdata->base + TMC_RRP);
> -               local_inc(&buf->lost);
> +               perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>         }
>
>         cur = buf->cur;
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 2a5982c37d..035c16c9a5 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -201,7 +201,7 @@ struct coresight_ops_sink {
>                           void *sink_config);
>         unsigned long (*reset_buffer)(struct coresight_device *csdev,
>                                       struct perf_output_handle *handle,
> -                                     void *sink_config, bool *lost);
> +                                     void *sink_config);
>         void (*update_buffer)(struct coresight_device *csdev,
>                               struct perf_output_handle *handle,
>                               void *sink_config);
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index a39564314e..cdbaa88dc8 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -812,6 +812,7 @@ struct perf_output_handle {
>         struct ring_buffer              *rb;
>         unsigned long                   wakeup;
>         unsigned long                   size;
> +       u64                             aux_flags;
>         union {
>                 void                    *addr;
>                 unsigned long           head;
> @@ -860,10 +861,11 @@ perf_cgroup_from_task(struct task_struct *task, struct perf_event_context *ctx)
>  extern void *perf_aux_output_begin(struct perf_output_handle *handle,
>                                    struct perf_event *event);
>  extern void perf_aux_output_end(struct perf_output_handle *handle,
> -                               unsigned long size, bool truncated);
> +                               unsigned long size);
>  extern int perf_aux_output_skip(struct perf_output_handle *handle,
>                                 unsigned long size);
>  extern void *perf_get_aux(struct perf_output_handle *handle);
> +extern void perf_aux_output_flag(struct perf_output_handle *handle, u64 flags);
>
>  extern int perf_pmu_register(struct pmu *pmu, const char *name, int type);
>  extern void perf_pmu_unregister(struct pmu *pmu);
> @@ -1278,8 +1280,8 @@ static inline void *
>  perf_aux_output_begin(struct perf_output_handle *handle,
>                       struct perf_event *event)                         { return NULL; }
>  static inline void
> -perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
> -                   bool truncated)                                     { }
> +perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
> +                                                                       { }
>  static inline int
>  perf_aux_output_skip(struct perf_output_handle *handle,
>                      unsigned long size)                                { return -EINVAL; }
> diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> index 6415807169..f3ebafe060 100644
> --- a/kernel/events/ring_buffer.c
> +++ b/kernel/events/ring_buffer.c
> @@ -297,6 +297,19 @@ ring_buffer_init(struct ring_buffer *rb, long watermark, int flags)
>                 rb->paused = 1;
>  }
>
> +void perf_aux_output_flag(struct perf_output_handle *handle, u64 flags)
> +{
> +       /*
> +        * OVERWRITE is determined by perf_aux_output_end() and can't
> +        * be passed in directly.
> +        */
> +       if (WARN_ON_ONCE(flags & PERF_AUX_FLAG_OVERWRITE))
> +               return;
> +
> +       handle->aux_flags |= flags;
> +}
> +EXPORT_SYMBOL_GPL(perf_aux_output_flag);
> +
>  /*
>   * This is called before hardware starts writing to the AUX area to
>   * obtain an output handle and make sure there's room in the buffer.
> @@ -360,6 +373,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
>         handle->event = event;
>         handle->head = aux_head;
>         handle->size = 0;
> +       handle->aux_flags = 0;
>
>         /*
>          * In overwrite mode, AUX data stores do not depend on aux_tail,
> @@ -409,34 +423,32 @@ EXPORT_SYMBOL_GPL(perf_aux_output_begin);
>   * of the AUX buffer management code is that after pmu::stop(), the AUX
>   * transaction must be stopped and therefore drop the AUX reference count.
>   */
> -void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
> -                        bool truncated)
> +void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
>  {
>         struct ring_buffer *rb = handle->rb;
> -       bool wakeup = truncated;
> +       bool wakeup = !!handle->aux_flags;
>         unsigned long aux_head;
> -       u64 flags = 0;
> -
> -       if (truncated)
> -               flags |= PERF_AUX_FLAG_TRUNCATED;
>
>         /* in overwrite mode, driver provides aux_head via handle */
>         if (rb->aux_overwrite) {
> -               flags |= PERF_AUX_FLAG_OVERWRITE;
> +               handle->aux_flags |= PERF_AUX_FLAG_OVERWRITE;
>
>                 aux_head = handle->head;
>                 local_set(&rb->aux_head, aux_head);
>         } else {
> +               handle->aux_flags &= ~PERF_AUX_FLAG_OVERWRITE;
> +
>                 aux_head = local_read(&rb->aux_head);
>                 local_add(size, &rb->aux_head);
>         }
>
> -       if (size || flags) {
> +       if (size || handle->aux_flags) {
>                 /*
>                  * Only send RECORD_AUX if we have something useful to communicate
>                  */
>
> -               perf_event_aux_event(handle->event, aux_head, size, flags);
> +               perf_event_aux_event(handle->event, aux_head, size,
> +                                    handle->aux_flags);
>         }
>
>         aux_head = rb->user_page->aux_head = local_read(&rb->aux_head);
> @@ -447,7 +459,7 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
>         }
>
>         if (wakeup) {
> -               if (truncated)
> +               if (handle->aux_flags & PERF_AUX_FLAG_TRUNCATED)
>                         handle->event->pending_disable = 1;
>                 perf_output_wakeup(handle);
>         }
> --
> 2.11.0
>

Powered by blists - more mailing lists