Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 26 Nov 2019 10:50:27 +0100
From:   Jiri Olsa <jolsa@redhat.com>
To:     Changbin Du <changbin.du@gmail.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Namhyung Kim <namhyung@kernel.org>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 2/2] perf: add support for logging debug messages to
 file
Message-ID: <20191126095027.GC32367@krava>
References: <20191125151446.10948-1-changbin.du@gmail.com>
 <20191125151446.10948-3-changbin.du@gmail.com>
MIME-Version: 1.0
In-Reply-To: <20191125151446.10948-3-changbin.du@gmail.com>
User-Agent: Mutt/1.12.1 (2019-06-15)
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Nov 25, 2019 at 11:14:46PM +0800, Changbin Du wrote:
> When in TUI mode, it is impossible to show all the debug messages to
> console. This make it hard to debug perf issues using debug messages.
> This patch adds support for logging debug messages to file to resolve
> this problem.
>=20
> The usage is:
> perf -debug verbose=3D2,file=3D~/perf.log COMMAND
>=20
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
>=20
> ---
> v5: doc default log path.
> v4: fix another segfault.
> v3: fix a segfault issue.
> ---
>  tools/perf/Documentation/perf.txt | 15 +++++++----
>  tools/perf/util/debug.c           | 44 ++++++++++++++++++++++++++++---
>  2 files changed, 50 insertions(+), 9 deletions(-)
>=20
> diff --git a/tools/perf/Documentation/perf.txt b/tools/perf/Documentation=
/perf.txt
> index fd8d790f68a7..a47933b53fbe 100644
> --- a/tools/perf/Documentation/perf.txt
> +++ b/tools/perf/Documentation/perf.txt
> @@ -16,15 +16,20 @@ OPTIONS
>  =09Setup debug variable (see list below) in value
>  =09range (0, 10). Use like:
>  =09  --debug verbose   # sets verbose =3D 1
> -=09  --debug verbose=3D2 # sets verbose =3D 2
> +=09  --debug verbose=3D2,file=3D~/perf.log
> +=09                    # sets verbose =3D 2 and save log to file
> =20
>  =09List of debug variables allowed to set:
> -=09  verbose=3Dlevel=09=09- general debug messages
> -=09  ordered-events=3Dlevel=09- ordered events object debug messages
> -=09  data-convert=3Dlevel=09- data convert command debug messages
> -=09  stderr=09=09- write debug output (option -v) to stderr
> +=09  verbose=3Dlevel         - general debug messages
> +=09  ordered-events=3Dlevel  - ordered events object debug messages
> +=09  data-convert=3Dlevel    - data convert command debug messages
> +=09  stderr                - write debug output (option -v) to stderr
> +=09                          in browser mode

hum, why is this changed in this patch?

jirka

>  =09  perf-event-open=09- Print perf_event_open() arguments and
>  =09                          return value in browser mode
> +=09  file[=3Dpath]           - write debug output to log file, default
> +=09                          'perf.log' (stderr and file options are
> +=09                          exclusive)
> =20
>  --buildid-dir::
>  =09Setup buildid cache directory. It has higher priority than
> diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
> index 929da46ece92..21bc889976bc 100644
> --- a/tools/perf/util/debug.c
> +++ b/tools/perf/util/debug.c
> @@ -6,6 +6,7 @@
>  #include <stdarg.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <errno.h>
>  #include <sys/wait.h>
>  #include <api/debug.h>
>  #include <linux/kernel.h>
> @@ -27,7 +28,7 @@ int verbose;
>  int debug_peo_args;
>  bool dump_trace =3D false, quiet =3D false;
>  int debug_ordered_events;
> -static bool redirect_to_stderr;
> +static FILE *log_file;
>  int debug_data_convert;
> =20
>  int veprintf(int level, int var, const char *fmt, va_list args)
> @@ -35,8 +36,10 @@ int veprintf(int level, int var, const char *fmt, va_l=
ist args)
>  =09int ret =3D 0;
> =20
>  =09if (var >=3D level) {
> -=09=09if (use_browser >=3D 1 && !redirect_to_stderr)
> +=09=09if (use_browser >=3D 1 && !log_file)
>  =09=09=09ui_helpline__vshow(fmt, args);
> +=09=09else if (log_file)
> +=09=09=09ret =3D vfprintf(log_file, fmt, args);
>  =09=09else
>  =09=09=09ret =3D vfprintf(stderr, fmt, args);
>  =09}
> @@ -198,6 +201,24 @@ static int str2loglevel(const char *vstr)
>  =09return v;
>  }
> =20
> +static void flush_log(void)
> +{
> +=09if (log_file)
> +=09=09fflush(log_file);
> +}
> +
> +static void set_log_output(FILE *f)
> +{
> +=09if (f =3D=3D log_file)
> +=09=09return;
> +
> +=09if (log_file && log_file !=3D stderr)
> +=09=09fclose(log_file);
> +
> +=09log_file =3D f;
> +=09atexit(flush_log);
> +}
> +
>  int perf_debug_option(const char *str)
>  {
>  =09char *sep, *vstr;
> @@ -219,10 +240,25 @@ int perf_debug_option(const char *str)
>  =09=09else if (!strcmp(opt, "data-convert"))
>  =09=09=09debug_data_convert =3D str2loglevel(vstr);
>  =09=09else if (!strcmp(opt, "stderr"))
> -=09=09=09redirect_to_stderr =3D true;
> +=09=09=09set_log_output(stderr);
>  =09=09else if (!strcmp(opt, "perf-event-open"))
>  =09=09=09debug_peo_args =3D true;
> -=09=09else {
> +=09=09else if (!strcmp(opt, "file")) {
> +=09=09=09FILE *f;
> +
> +=09=09=09if (!vstr)
> +=09=09=09=09vstr =3D (char *)"perf.log";
> +
> +=09=09=09f =3D fopen(vstr, "a");
> +=09=09=09if (!f) {
> +=09=09=09=09pr_err("Can not create log file: %s\n",
> +=09=09=09=09       strerror(errno));
> +=09=09=09=09free(dstr);
> +=09=09=09=09return -1;
> +=09=09=09}
> +=09=09=09fprintf(f, "\n=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3Dperf log=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D\n");
> +=09=09=09set_log_output(f);
> +=09=09} else {
>  =09=09=09fprintf(stderr, "unkown debug option '%s'\n", opt);
>  =09=09=09free(dstr);
>  =09=09=09return -1;
> --=20
> 2.20.1
>=20