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;
Subject: Re: [PATCH] smp: generic ipi_raise tracepoint
To:     Nadav Amit <namit@vmware.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Peter Zijlstra <peterz@infradead.org>
References: <02b57f31-fc0c-df35-57bf-e0a297a7e6c4@gmail.com>
 <40AA62E7-3AFB-44DC-B4A6-F89E5C914620@vmware.com>
From:   Wojciech Kudla <wk.kernel@gmail.com>
Message-ID: <767faf00-0dc0-698a-910f-100608d10d81@gmail.com>
Date:   Thu, 21 May 2020 21:11:36 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <40AA62E7-3AFB-44DC-B4A6-F89E5C914620@vmware.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 21/05/2020 20:00, Nadav Amit wrote:

>> -	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)
> 

Yes, my motivation for conditional logic was performance-driven.
Thanks for pointing out the implicit check.

> 
> 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.
> 

Thank you for the review, I agree this may not be optimal. Let's just stop here.
There's a different patch I submitted today that follows Peter's suggestions
about smp function calls being much more sensible target for new tracepoints.

http://lkml.kernel.org/r/e6141d56-1da1-6c00-198f-cbe4385327ff@gmail.com

Thanks,
W.