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


Groups > linux.kernel > #1739322

Re: [PATCH v6 2/2] tracing: Add support for preempt and irq enable/disable events

From Joel Fernandes <joelaf@google.com>
Newsgroups linux.kernel
Subject Re: [PATCH v6 2/2] tracing: Add support for preempt and irq enable/disable events
Date 2017-09-26 01:00 +0200
Message-ID <utKw1-4oF-5@gated-at.bofh.it> (permalink)
References (3 earlier) <usAS5-ZW-15@gated-at.bofh.it> <utx5M-3Sb-5@gated-at.bofh.it> <utyEx-4YG-3@gated-at.bofh.it> <utyXV-55N-21@gated-at.bofh.it> <utzhg-5f9-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, Sep 25, 2017 at 3:52 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 25 Sep 2017 12:32:23 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
>
>
>> > You mean you want to trace all calls to preempt and irq off even if
>> > preempt and irqs are already off?
>>
>> Sure, why not? This stuff naturally nests, and who is to say its not a
>> useful thing to trace all of them?
>>
>> By also tracing the nested sections you can, for instance, see how much
>> you'd really win by getting rid of the outer one. If, for instance, the
>> outer only accounts for 1% of the time, while the inner ones are
>> interlinked and span the other 99%, there's more work to do than if that
>> were not the case.
>
> If we do this we need a field to record if the preemption or irqs were
> toggled by that call. Something that filters could easily be added to
> only show what this patch set has.

I request that we please not do this for my patchset, there are a
number of reasons in my mind:

1. trace_preempt_off in existing code is only called the first time
preempt is turned off. This is the definition of "preempt off", its
the first time Preemption is actually turned off, and has nothing much
to do with going into a deeper preempt count. Whether the count
increases or not, preempt is already off and that's confirmed by the
first preempt off event.

This is how I read it in the comments in sched/core.c as well:
"
 * If the value passed in is equal to the current preempt count
 * then we just disabled preemption."

This is how I based this patchset as well, againt its not my usecase
and it can be a future patch if its useful to track this.

2. This stuff is already a bit trace heavy, I prefer not to generate
event every time the preempt count increases. Ofcourse filters but
still then we have the filtering overhead for a usecase that I am not
really targetting with this patchset.

3. It will complicate the patch more, apart from adding filters as
Steven suggested, it would also mean we change how
preempt_latency_start in sched/core.c works.

Do you mind if we please keep this as a 'future' usecase for a future
patch? Its not my usecase at all for this patchset and not what I was
intending.

I will reply to Peter's other comments on the other email shortly.

thanks!

- Joel

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v6 2/2] tracing: Add support for preempt and irq enable/disable events Joel Fernandes <joelaf@google.com> - 2017-09-22 04:00 +0200
  Re: [PATCH v6 2/2] tracing: Add support for preempt and irq  enable/disable events Peter Zijlstra <peterz@infradead.org> - 2017-09-22 11:10 +0200
    Re: [PATCH v6 2/2] tracing: Add support for preempt and irq  enable/disable events Joel Fernandes <joelaf@google.com> - 2017-09-22 20:30 +0200
      Re: [PATCH v6 2/2] tracing: Add support for preempt and irq  enable/disable events Peter Zijlstra <peterz@infradead.org> - 2017-09-25 10:40 +0200
        Re: [PATCH v6 2/2] tracing: Add support for preempt and irq  enable/disable events Steven Rostedt <rostedt@goodmis.org> - 2017-09-25 12:20 +0200
          Re: [PATCH v6 2/2] tracing: Add support for preempt and irq  enable/disable events Peter Zijlstra <peterz@infradead.org> - 2017-09-25 12:40 +0200
            Re: [PATCH v6 2/2] tracing: Add support for preempt and irq  enable/disable events Steven Rostedt <rostedt@goodmis.org> - 2017-09-25 13:00 +0200
              Re: [PATCH v6 2/2] tracing: Add support for preempt and irq  enable/disable events Joel Fernandes <joelaf@google.com> - 2017-09-26 01:00 +0200
                Re: [PATCH v6 2/2] tracing: Add support for preempt and irq  enable/disable events Steven Rostedt <rostedt@goodmis.org> - 2017-09-26 04:50 +0200
        Re: [PATCH v6 2/2] tracing: Add support for preempt and irq  enable/disable events Joel Fernandes <joelaf@google.com> - 2017-09-26 02:30 +0200

csiph-web