Re: [PATCH 01/10] perf, tools: Factor out scale conversion code

From	Arnaldo Carvalho de Melo <acme@kernel.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH 01/10] perf, tools: Factor out scale conversion code
Date	2016-10-14 18:30 +0200
Message-ID	<ssdwS-3Da-7@gated-at.bofh.it> (permalink)
References	(1 earlier) <srVzY-qJ-35@gated-at.bofh.it> <sscKu-36Y-1@gated-at.bofh.it> <sscUa-3ay-13@gated-at.bofh.it> <ssdnc-3zJ-3@gated-at.bofh.it> <ssdnc-3zJ-5@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

Em Fri, Oct 14, 2016 at 09:15:16AM -0700, Andi Kleen escreveu:
> On Fri, Oct 14, 2016 at 01:08:42PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Oct 14, 2016 at 08:45:15AM -0700, Andi Kleen escreveu:
> > > > So if we can't convert the scale because of an allocation failure
> > > > related to locale issues we silently trow it away and do no scale at
> > > > all?
> > 
> > > That is right. If your machine is thrashing to death in a OOM this
> > > is your smallest problem.
> > 
> > Please keep it was before, i.e. return an error value, and bail out. It
> > was like that before, why introduce these kinds of silent "do something
> > else in an unlikely case" handling?
> 
> Ok. I will fix it.
> 
> But just for the record I don't think this fine grained memory error
> handling makes any sense for perf. It is needed in the kernel, but
> it's not appropiate for user programs:
> 
> - Usually when you're out of memory then every thing afterward
> that needs memory will fail too, so there's no sane way to continue,
> - When you run out of memory in user space you usually get killed
> at some point anyways because the OOM killer kicks in.
> - The only exception is that you run out of VA space, but then the point
> above applies.
> - These error paths are all untested and most likely a significant
> fraction of them is broken because untested code is often broken.
> 
> What most user space does is to just have malloc wrappers that
> exit when you run out of memory with an error message. That's nearly
> always the right strategy for user programs.

I disagree, and I usually try not to differentiate that much if I'm
programming for the kernel or for userspace, in fact, I try as much as
possible to reduce the gap of writing for the kernel or writing for
userspace.

I tools/ specifically, I try to use the same constructs, list.h, rbtree,
WARN_, pr_, etc, not panic()'ing, etc.

In this specific case, doing a:

   printf("life is hard, I give up"); exit(1);

would be preferrable to silently not doing what was asked for, namely do
the scaling, with that said, please keep the original way of handling it
and just return an error when what was asked for can't be done.

Thanks,

- Arnaldo

Thread

Support Intel uncore event lists Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
  [PATCH 07/10] perf, tools: Collapse identically named events in perf stat Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
    Re: [PATCH 07/10] perf, tools: Collapse identically named events in  perf stat Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:00 +0200
    Re: [PATCH 07/10] perf, tools: Collapse identically named events in  perf stat Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:30 +0200
      Re: [PATCH 07/10] perf, tools: Collapse identically named events in  perf stat Andi Kleen <andi@firstfloor.org> - 2016-10-17 18:40 +0200
        Re: [PATCH 07/10] perf, tools: Collapse identically named events in  perf stat Jiri Olsa <jolsa@redhat.com> - 2016-10-17 19:30 +0200
          Re: [PATCH 07/10] perf, tools: Collapse identically named events in  perf stat Andi Kleen <ak@linux.intel.com> - 2016-10-17 20:20 +0200
    Re: [PATCH 07/10] perf, tools: Collapse identically named events in  perf stat Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:30 +0200
  [PATCH 08/10] perf, tools: Expand PMU events by prefix match Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
    Re: [PATCH 08/10] perf, tools: Expand PMU events by prefix match Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:40 +0200
    Re: [PATCH 08/10] perf, tools: Expand PMU events by prefix match Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:50 +0200
      Re: [PATCH 08/10] perf, tools: Expand PMU events by prefix match Andi Kleen <andi@firstfloor.org> - 2016-10-17 19:20 +0200
        Re: [PATCH 08/10] perf, tools: Expand PMU events by prefix match Jiri Olsa <jolsa@redhat.com> - 2016-10-17 19:30 +0200
  [PATCH 02/10] perf, tools: Only print Using CPUID message once Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
    Re: [PATCH 02/10] perf, tools: Only print Using CPUID message once Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-14 17:50 +0200
    [tip:perf/core] perf pmu: Only print Using CPUID message once tip-bot for Andi Kleen <tipbot@zytor.com> - 2016-10-24 21:10 +0200
  [PATCH 09/10] perf, tools: Support DividedBy header in JSON event list Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
    Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON  event list Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:50 +0200
      Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON  event list Andi Kleen <andi@firstfloor.org> - 2016-10-17 18:30 +0200
        Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON  event list Jiri Olsa <jolsa@redhat.com> - 2016-10-17 19:50 +0200
        Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON  event list Jiri Olsa <jolsa@redhat.com> - 2016-10-17 19:50 +0200
          Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON  event list Andi Kleen <andi@firstfloor.org> - 2016-10-17 20:30 +0200
  [PATCH 04/10] perf, tools: Support per pmu json aliases Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
    Re: [PATCH 04/10] perf, tools: Support per pmu json aliases Jiri Olsa <jolsa@redhat.com> - 2016-10-14 14:40 +0200
  [PATCH 05/10] perf, tools: Support event aliases for non cpu// pmus Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
    Re: [PATCH 05/10] perf, tools: Support event aliases for non cpu//  pmus Jiri Olsa <jolsa@redhat.com> - 2016-10-17 11:40 +0200
    Re: [PATCH 05/10] perf, tools: Support event aliases for non cpu//  pmus Jiri Olsa <jolsa@redhat.com> - 2016-10-17 12:30 +0200
  [PATCH 01/10] perf, tools: Factor out scale conversion code Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
    Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-14 17:40 +0200
      Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Andi Kleen <andi@firstfloor.org> - 2016-10-14 17:50 +0200
        Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-14 18:20 +0200
          Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Andi Kleen <ak@linux.intel.com> - 2016-10-14 18:20 +0200
            Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-14 18:30 +0200
              Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-14 18:40 +0200
              Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Andi Kleen <andi@firstfloor.org> - 2016-10-14 18:40 +0200
  [PATCH 10/10] perf, tools, stat: Output generic dividedby metric Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
  [PATCH 06/10] perf, tools: Add debug support for outputing alias string Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
  Re: Support Intel uncore event lists Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:10 +0200

csiph-web