Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220901110032.9226-1-adrian.hunter@intel.com>
 <20220901110032.9226-6-adrian.hunter@intel.com> <dadd6179-4867-211a-ad6e-30fcd66a8e0a@linux.intel.com>
 <1a7aaa51-1858-bb59-834c-7e8f6a58bc39@intel.com>
In-Reply-To: <1a7aaa51-1858-bb59-834c-7e8f6a58bc39@intel.com>
From:   Namhyung Kim <namhyung@kernel.org>
Date:   Thu, 1 Sep 2022 18:34:00 -0700
Message-ID: <CAM9d7chiutXEZfpQbayJ3bgraLZ_YFGC15yDn7sQBT4asdEfjA@mail.gmail.com>
Subject: Re: [PATCH 5/5] perf intel-pt: Support itrace option flag d+e to log
 on error
To:     Adrian Hunter <adrian.hunter@intel.com>
Cc:     Andi Kleen <ak@linux.intel.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Jiri Olsa <jolsa@redhat.com>, Ian Rogers <irogers@google.com>,
        linux-kernel <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Sep 1, 2022 at 9:29 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 1/09/22 17:31, Andi Kleen wrote:
> >
> > On 9/1/2022 4:00 AM, Adrian Hunter wrote:

[SNIP]
> >> +
> >> +static void log_buf__dump(struct log_buf *b)
> >> +{
> >> +    if (!b->buf)
> >> +        return;
> >> +
> >> +    fflush(f);
> >> +    fprintf(b->backend, "Dumping debug log buffer (first line may be sliced)\n");
> >
> >
> > Should be easy to skip the first line, no?
>
> Not as easy as typing " (first line may be sliced)" ;-)
>
> Still not sure it is worth having the extra complication, but here
> is the change as a separate patch:
>
> From: Adrian Hunter <adrian.hunter@intel.com>
> Date: Thu, 1 Sep 2022 19:01:33 +0300
> Subject: [PATCH] perf intel-pt: Remove first line of log dumped on error
>
> Instead of printing "(first line may be sliced)", always remove the
> first line of the debug log when dumping on error.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  .../perf/util/intel-pt-decoder/intel-pt-log.c | 27 ++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-log.c b/tools/perf/util/intel-pt-decoder/intel-pt-log.c
> index ea96dcae187a7..6cc465d1f7a9e 100644
> --- a/tools/perf/util/intel-pt-decoder/intel-pt-log.c
> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-log.c
> @@ -143,16 +143,37 @@ static FILE *log_buf__open(struct log_buf *b, FILE *backend, unsigned int sz)
>         return file;
>  }
>
> +static bool remove_first_line(const char **p, size_t *n)
> +{
> +       for (; *n && **p != '\n'; ++*p, --*n)
> +               ;
> +       if (*n) {
> +               *p += 1;
> +               *n -= 1;
> +               return true;
> +       }
> +       return false;
> +}
> +
> +static void write_lines(const char *p, size_t n, FILE *fp, bool *remove_first)
> +{
> +       if (*remove_first)
> +               *remove_first = !remove_first_line(&p, &n);
> +       fwrite(p, n, 1, fp);
> +}
> +
>  static void log_buf__dump(struct log_buf *b)
>  {
> +       bool remove_first = true;

Isn't it only required when the buf is wrapped?

Thanks,
Namhyung


> +
>         if (!b->buf)
>                 return;
>
>         fflush(f);
> -       fprintf(b->backend, "Dumping debug log buffer (first line may be sliced)\n");
> +       fprintf(b->backend, "Dumping debug log buffer\n");
>         if (b->wrapped)
> -               fwrite(b->buf + b->head, b->buf_sz - b->head, 1, b->backend);
> -       fwrite(b->buf, b->head, 1, b->backend);
> +               write_lines(b->buf + b->head, b->buf_sz - b->head, b->backend, &remove_first);
> +       write_lines(b->buf, b->head, b->backend, &remove_first);
>         fprintf(b->backend, "End of debug log buffer dump\n");
>
>         b->head = 0;
> --
> 2.34.1
>