Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200901144324.1071694-1-maz@kernel.org> <20200901144324.1071694-4-maz@kernel.org>
 <CAKfTPtDjPpri5Gt6kLeFp_B_zJUZ5DYXEqtJ+0VKohU-y9bFEQ@mail.gmail.com> <353f13b0dcc6c7ea1b44012d9632a0cc@kernel.org>
In-Reply-To: <353f13b0dcc6c7ea1b44012d9632a0cc@kernel.org>
From:   Vincent Guittot <vincent.guittot@linaro.org>
Date:   Mon, 19 Oct 2020 17:43:16 +0200
Message-ID: <CAKfTPtBjw5ay_BcPmEXpHOHbNxNZuYmdCoHQM53u3c+RXnKONg@mail.gmail.com>
Subject: Re: [PATCH v3 03/16] arm64: Allow IPIs to be handled as normal interrupts
To:     Marc Zyngier <maz@kernel.org>
Cc:     LAK <linux-arm-kernel@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Will Deacon <will@kernel.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Russell King <linux@arm.linux.org.uk>,
        Thomas Gleixner <tglx@linutronix.de>,
        Jason Cooper <jason@lakedaemon.net>,
        Sumit Garg <sumit.garg@linaro.org>,
        Valentin Schneider <Valentin.Schneider@arm.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Gregory Clement <gregory.clement@bootlin.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Saravana Kannan <saravanak@google.com>,
        Android Kernel Team <kernel-team@android.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, 19 Oct 2020 at 15:04, Marc Zyngier <maz@kernel.org> wrote:
>
> Hi Vincent,
>
> On 2020-10-19 13:42, Vincent Guittot wrote:
> > Hi Marc,
> >
> > On Tue, 1 Sep 2020 at 16:44, Marc Zyngier <maz@kernel.org> wrote:
> >>
> >> In order to deal with IPIs as normal interrupts, let's add
> >> a new way to register them with the architecture code.
> >>
> >> set_smp_ipi_range() takes a range of interrupts, and allows
> >> the arch code to request them as if the were normal interrupts.
> >> A standard handler is then called by the core IRQ code to deal
> >> with the IPI.
> >>
> >> This means that we don't need to call irq_enter/irq_exit, and
> >> that we don't need to deal with set_irq_regs either. So let's
> >> move the dispatcher into its own function, and leave handle_IPI()
> >> as a compatibility function.
> >>
> >> On the sending side, let's make use of ipi_send_mask, which
> >> already exists for this purpose.
> >>
> >> One of the major difference is that we end up, in some cases
> >> (such as when performing IRQ time accounting on the scheduler
> >> IPI), end up with nested irq_enter()/irq_exit() pairs.
> >> Other than the (relatively small) overhead, there should be
> >> no consequences to it (these pairs are designed to nest
> >> correctly, and the accounting shouldn't be off).
> >
> > While rebasing on mainline, I have faced a performance regression for
> > the benchmark:
> > perf bench sched pipe
> > on my arm64 dual quad core (hikey) and my 2 nodes x 112 CPUS (thx2)
> >
> > The regression comes from:
> > commit: d3afc7f12987 ("arm64: Allow IPIs to be handled as normal
> > interrupts")
>
> That's interesting, as this patch doesn't really change anything (most
> of the potential overhead comes in later). The only potential overhead
> I can see is that the scheduler_ipi() call is now wrapped around
> irq_enter()/irq_exit().
>
> >
> >           v5.9              + this patch
> > hikey :   48818(+/- 0.31)   37503(+/- 0.15%)  -23.2%
> > thx2  :  132410(+/- 1.72)  122646(+/- 1.92%)   -7.4%
> >
> > By + this patch,  I mean merging branch from this patch. Whereas
> > merging the previous:
> > commit: 83cfac95c018 ("genirq: Allow interrupts to be excluded from
> > /proc/interrupts")
> >  It doesn't show any regression
>
> Since you are running perf, can you spot where the overhead occurs?

hmm... Difficult to say because tracing the bench decreases a lot the
result. I have pasted the perf reports.

With this patch :

# Samples: 634  of event 'cpu-clock'
# Event count (approx.): 158500000
#
# Overhead  Command     Shared Object       Symbol
# ........  ..........  ..................  ..................................
#
    31.86%  sched-pipe  [kernel.kallsyms]   [k] _raw_spin_unlock_irqrestore
     8.68%  sched-pipe  [kernel.kallsyms]   [k] _raw_spin_unlock_irq
     6.31%  sched-pipe  [kernel.kallsyms]   [k] __schedule
     5.21%  sched-pipe  [kernel.kallsyms]   [k] schedule
     4.73%  sched-pipe  [kernel.kallsyms]   [k] pipe_read
     3.31%  sched-pipe  [kernel.kallsyms]   [k] el0_svc_common.constprop.3
     2.84%  sched-pipe  [kernel.kallsyms]   [k] ww_mutex_lock_interruptible
     2.52%  sched-pipe  [kernel.kallsyms]   [k] init_wait_entry
     2.37%  sched-pipe  [kernel.kallsyms]   [k] mutex_unlock
     2.21%  sched-pipe  [kernel.kallsyms]   [k] new_sync_read
     1.89%  sched-pipe  [kernel.kallsyms]   [k] new_sync_write
     1.74%  sched-pipe  [kernel.kallsyms]   [k] security_file_permission
     1.74%  sched-pipe  [kernel.kallsyms]   [k] vfs_read
     1.58%  sched-pipe  [kernel.kallsyms]   [k] __my_cpu_offset
     1.26%  sched-pipe  libpthread-2.24.so  [.] 0x0000000000010a2c
     1.10%  sched-pipe  [kernel.kallsyms]   [k] mutex_lock
     1.10%  sched-pipe  [kernel.kallsyms]   [k] vfs_write

After reverting this patch which gives a result similar to v5.9:

# Samples: 659  of event 'cpu-clock'
# Event count (approx.): 164750000
#
# Overhead  Command     Shared Object       Symbol
# ........  ..........  ..................  ...............................
#
    29.29%  sched-pipe  [kernel.kallsyms]   [k] _raw_spin_unlock_irqrestore
    21.40%  sched-pipe  [kernel.kallsyms]   [k] _raw_spin_unlock_irq
     4.86%  sched-pipe  [kernel.kallsyms]   [k] pipe_read
     4.55%  sched-pipe  [kernel.kallsyms]   [k] ww_mutex_lock_interruptible
     2.88%  sched-pipe  [kernel.kallsyms]   [k] __schedule
     2.88%  sched-pipe  [kernel.kallsyms]   [k] _raw_spin_lock_irqsave
     2.88%  sched-pipe  [kernel.kallsyms]   [k] schedule
     2.12%  sched-pipe  [kernel.kallsyms]   [k] new_sync_read
     1.82%  sched-pipe  [kernel.kallsyms]   [k] mutex_lock
     1.67%  sched-pipe  [kernel.kallsyms]   [k] el0_svc_common.constprop.3
     1.67%  sched-pipe  [kernel.kallsyms]   [k] pipe_write
     1.21%  sched-pipe  [kernel.kallsyms]   [k] rw_verify_area
     1.21%  sched-pipe  [kernel.kallsyms]   [k] security_file_permission
     1.06%  sched-pipe  [kernel.kallsyms]   [k] fsnotify

I have only put symbol with overhead above 1%

so _raw_spin_unlock_irq, schedule and __schedule seem the most
impacted but i can't get any conclusion

I can sent you perf.data files if you want


>
> Thanks,
>
>          M.
> --
> Jazz is not dead. It just smells funny...