From: Nadav Amit <namit@vmware.com>
To: Wojciech Kudla <wk.kernel@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH] smp: generic ipi_raise tracepoint
Date: Thu, 21 May 2020 19:00:54 +0000 [thread overview]
Message-ID: <40AA62E7-3AFB-44DC-B4A6-F89E5C914620@vmware.com> (raw)
In-Reply-To: <02b57f31-fc0c-df35-57bf-e0a297a7e6c4@gmail.com>
> On May 20, 2020, at 6:17 AM, Wojciech Kudla <wk.kernel@gmail.com> wrote:
>
> Preliminary discussion: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2020%2F5%2F13%2F1327&data=02%7C01%7Cnamit%40vmware.com%7Ceb1fce63ca4644ab29ad08d7fcc022df%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637255774462316114&sdata=eKrYH1vLDaEk4QyN4ZLQQRCk%2BtVdGLq7K6xYn1s%2BjJo%3D&reserved=0
> This patch avoids introducing arch-specific trace points by leveraging
> existing definition for ipi_raise.
>
> Issues to address in potential future work:
> - make ipi reason available on generic smp code level (possible?)
> - addition of ipi_entry/ipi_exit tracepoints in generic smp code
>
> Signed-off-by: Wojciech Kudla <wk.kernel@gmail.com>
> ---
> kernel/smp.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/smp.c b/kernel/smp.c
> index 7dbcb402c2fc..df6982a1d3f2 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -23,6 +23,11 @@
>
> #include "smpboot.h"
>
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/ipi.h>
> +
> +static const char *ipi_reason_missing __tracepoint_string = "";
> +
> enum {
> CSD_FLAG_LOCK = 0x01,
> CSD_FLAG_SYNCHRONOUS = 0x02,
> @@ -34,6 +39,7 @@ struct call_function_data {
> cpumask_var_t cpumask_ipi;
> };
>
> +
Unneeded redundant new line.
> static DEFINE_PER_CPU_ALIGNED(struct call_function_data, cfd_data);
>
> static DEFINE_PER_CPU_SHARED_ALIGNED(struct llist_head, call_single_queue);
> @@ -176,8 +182,12 @@ static int generic_exec_single(int cpu, call_single_data_t *csd,
> * locking and barrier primitives. Generic code isn't really
> * equipped to do the right thing...
> */
> - if (llist_add(&csd->llist, &per_cpu(call_single_queue, cpu)))
> + if (llist_add(&csd->llist, &per_cpu(call_single_queue, cpu))) {
> + if (trace_ipi_raise_enabled())
Why do you need this check? trace_ipi_raise() will do the same check before
actual tracing:
if (static_key_false(&__tracepoint_##name.key)
I guess you do it in order to avoid evaluation of cpumask_of() if tracing is
disabled, but it seems to me that the macro would only evaluate/call
cpumask_of() if tracing is indeed enabled.
> + trace_ipi_raise(cpumask_of(cpu), ipi_reason_missing);
> +
> arch_send_call_function_single_ipi(cpu);
> + }
In general, I think there are too many trace-points. They look benign(i.e.,
free), but can cause worse code to be generated as they behave as a memory
clobber. Many times the same result can be achieved with a probe.
next prev parent reply other threads:[~2020-05-21 19:00 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-20 13:17 [PATCH] smp: generic ipi_raise tracepoint Wojciech Kudla
2020-05-20 13:33 ` Peter Zijlstra
2020-05-20 13:42 ` Wojciech Kudla
2020-05-20 13:51 ` Peter Zijlstra
2020-05-21 10:45 ` kbuild test robot
2020-05-21 19:00 ` Nadav Amit [this message]
2020-05-21 20:11 ` Wojciech Kudla
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=40AA62E7-3AFB-44DC-B4A6-F89E5C914620@vmware.com \
--to=namit@vmware.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=wk.kernel@gmail.com \
/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).