From: Palmer Dabbelt <palmer@sifive.com>
To: anup@brainfault.org
Cc: Christoph Hellwig <hch@infradead.org>,
aou@eecs.berkeley.edu, atish.patra@wdc.com,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
tglx@linutronix.de
Subject: Re: [PATCH] RISC-V: Show IPI stats
Date: Mon, 01 Oct 2018 09:42:37 -0700 (PDT) [thread overview]
Message-ID: <mhng-34a3263b-002e-4301-ae67-47e19c44449f@palmer-si-x1c4> (raw)
In-Reply-To: <CAAhSdy0wd7LN1tb5oRkgYbxeR=Yb3LomUtTcym1dhyO6wa+a9g@mail.gmail.com>
On Fri, 28 Sep 2018 23:09:56 PDT (-0700), anup@brainfault.org wrote:
> On Sat, Sep 29, 2018 at 7:15 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>>
>> On Mon, 10 Sep 2018 06:46:59 PDT (-0700), Christoph Hellwig wrote:
>> > On Fri, Sep 07, 2018 at 06:14:29PM +0530, Anup Patel wrote:
>> >> This patch provides arch_show_interrupts() implementation to
>> >> show IPI stats via /proc/interrupts.
>> >>
>> >> Now the contents of /proc/interrupts" will look like below:
>> >> CPU0 CPU1 CPU2 CPU3
>> >> 8: 17 7 6 14 SiFive PLIC 8 virtio0
>> >> 10: 10 10 9 11 SiFive PLIC 10 ttyS0
>> >> IPI0: 170 673 251 79 Rescheduling interrupts
>> >> IPI1: 1 12 27 1 Function call interrupts
>> >> IPI2: 0 0 0 0 CPU wake-up interrupts
>> >>
>> >> Signed-off-by: Anup Patel <anup@brainfault.org>
>> >
>> > Thanks, this looks pretty sensible to me. Maybe we want to also show
>> > timer interrupts if we do this?
>>
>> IIRC we used to have some issue where the timer interrupt ID in
>> /proc/interrupts aliased with a possible PLIC interrupt ID, but that was back
>> when we had a big mess of chained interrupt drivers that didn't really talk to
>> each other. I think at some point I might have just removed the timer
>> interrupt from /proc/interrupts as a hack, but now that our interrupt
>> controller mess is sorted out it'd be better to have it.
>
> If we have irqdomain and irqchip for HLIC then showing timer interrupts
> becomes cleaner. My RISCV INTC patch achieves this.
>
>>
>> I'm fine taking this without the timer interrupts, as something is better than
>> nothing.
>
> Sure.
>
>>
>> >> --- a/arch/riscv/kernel/irq.c
>> >> +++ b/arch/riscv/kernel/irq.c
>> >> @@ -8,6 +8,7 @@
>> >> #include <linux/interrupt.h>
>> >> #include <linux/irqchip.h>
>> >> #include <linux/irqdomain.h>
>> >> +#include <linux/seq_file.h>
>> >>
>> >> /*
>> >> * Possible interrupt causes:
>> >> @@ -24,6 +25,14 @@
>> >> */
>> >> #define INTERRUPT_CAUSE_FLAG (1UL << (__riscv_xlen - 1))
>> >>
>> >> +int arch_show_interrupts(struct seq_file *p, int prec)
>> >> +{
>> >> +#ifdef CONFIG_SMP
>> >> + show_ipi_stats(p, prec);
>> >> +#endif
>> >> + return 0;
>> >> +}
>> >
>> > If we don't also add timer stats I'd just move arch_show_interrupts
>> > to smp.c and make it conditional. If we don't this split might make
>> > more sense.
>>
>> Makes sense, but I think timer interrupts are more interesting to see than IPIs
>> so we'll eventually pipe them through. Might just be my workloads, though :)
>>
>> >> +static const char *ipi_names[IPI_MAX] = {
>> >> + [IPI_RESCHEDULE] = "Rescheduling interrupts",
>> >> + [IPI_CALL_FUNC] = "Function call interrupts",
>> >> + [IPI_CALL_WAKEUP] = "CPU wake-up interrupts",
>> >> +};
>> >
>> > No need for the explicit array size. Also please use a few tabs to
>> > align this nicely:
>> >
>> > static const char *ipi_names[] = {
>> > [IPI_RESCHEDULE] = "Rescheduling interrupts",
>> > [IPI_CALL_FUNC] = "Function call interrupts",
>> > [IPI_CALL_WAKEUP] = "CPU wake-up interrupts",
>> > };
>>
>> I don't see a v2 of this, was there one? If not then I'll just clean up
>> ipi_names and drop this on for-next.
>>
>> Thanks for the patch!
>
> I will send v3 patch because IPI_CALL_WAKEUP is not going to be available.
>
> I will base v3 patch upon v5 of Atish's patchset. Atish will take v3 of this
> patch and make it part of his v6 patchset so that it will be easy for you
> to apply.
Thanks!
prev parent reply other threads:[~2018-10-01 16:42 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-07 12:44 [PATCH] RISC-V: Show IPI stats Anup Patel
2018-09-10 13:46 ` Christoph Hellwig
2018-09-12 4:33 ` Anup Patel
2018-09-29 1:45 ` Palmer Dabbelt
2018-09-29 6:09 ` Anup Patel
2018-10-01 16:42 ` Palmer Dabbelt [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=mhng-34a3263b-002e-4301-ae67-47e19c44449f@palmer-si-x1c4 \
--to=palmer@sifive.com \
--cc=anup@brainfault.org \
--cc=aou@eecs.berkeley.edu \
--cc=atish.patra@wdc.com \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).