[01/17] block/nvme: Avoid further processing if trace event not enabled
diff mbox series

Message ID 20200625184838.28172-2-philmd@redhat.com
State New
Headers show
Series
  • block/nvme: Various cleanups required to use multiple queues
Related show

Commit Message

Philippe Mathieu-Daudé June 25, 2020, 6:48 p.m. UTC
Avoid further processing if TRACE_NVME_SUBMIT_COMMAND_RAW is
not enabled.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 block/nvme.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Stefan Hajnoczi June 26, 2020, 10:36 a.m. UTC | #1
On Thu, Jun 25, 2020 at 08:48:22PM +0200, Philippe Mathieu-Daudé wrote:
> Avoid further processing if TRACE_NVME_SUBMIT_COMMAND_RAW is
> not enabled.

Why?

This saves 8 trace events, each with 8 arguments. I guess the intent is
to improve performance. Did you measure an improvement?
Philippe Mathieu-Daudé June 26, 2020, 2:02 p.m. UTC | #2
On 6/26/20 12:36 PM, Stefan Hajnoczi wrote:
> On Thu, Jun 25, 2020 at 08:48:22PM +0200, Philippe Mathieu-Daudé wrote:
>> Avoid further processing if TRACE_NVME_SUBMIT_COMMAND_RAW is
>> not enabled.
> 
> Why?
> 
> This saves 8 trace events, each with 8 arguments. I guess the intent is
> to improve performance. Did you measure an improvement?

No testing, I just tried to outsmart the compiler :/
Stefan Hajnoczi June 29, 2020, 1:02 p.m. UTC | #3
On Fri, Jun 26, 2020 at 04:02:43PM +0200, Philippe Mathieu-Daudé wrote:
> On 6/26/20 12:36 PM, Stefan Hajnoczi wrote:
> > On Thu, Jun 25, 2020 at 08:48:22PM +0200, Philippe Mathieu-Daudé wrote:
> >> Avoid further processing if TRACE_NVME_SUBMIT_COMMAND_RAW is
> >> not enabled.
> > 
> > Why?
> > 
> > This saves 8 trace events, each with 8 arguments. I guess the intent is
> > to improve performance. Did you measure an improvement?
> 
> No testing, I just tried to outsmart the compiler :/

Usually performance patches are accompanied with benchmark results.
Otherwise it's easy to modify code without making a difference or
accidentally introducing performance regressions. But this is a small
change and I'm not worried.

Please do explicitly state in the commit description that this is
intended as a performance optimization. I wasn't exactly sure about the
reason for this change.

Stefan

Patch
diff mbox series

diff --git a/block/nvme.c b/block/nvme.c
index eb2f54dd9d..1e5b40f61c 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -367,6 +367,9 @@  static void nvme_trace_command(const NvmeCmd *cmd)
 {
     int i;
 
+    if (!trace_event_get_state_backends(TRACE_NVME_SUBMIT_COMMAND_RAW)) {
+        return;
+    }
     for (i = 0; i < 8; ++i) {
         uint8_t *cmdp = (uint8_t *)cmd + i * 8;
         trace_nvme_submit_command_raw(cmdp[0], cmdp[1], cmdp[2], cmdp[3],