From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Borkmann Subject: [PATCH bpf-next 2/3] tools, perf: use smp_{rmb,mb} barriers instead of {rmb,mb} Date: Wed, 17 Oct 2018 16:41:55 +0200 Message-ID: <20181017144156.16639-3-daniel@iogearbox.net> References: <20181017144156.16639-1-daniel@iogearbox.net> Cc: peterz@infradead.org, paulmck@linux.vnet.ibm.com, will.deacon@arm.com, acme@redhat.com, yhs@fb.com, john.fastabend@gmail.com, netdev@vger.kernel.org, Daniel Borkmann To: alexei.starovoitov@gmail.com Return-path: Received: from www62.your-server.de ([213.133.104.62]:36528 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727363AbeJQWjm (ORCPT ); Wed, 17 Oct 2018 18:39:42 -0400 In-Reply-To: <20181017144156.16639-1-daniel@iogearbox.net> Sender: netdev-owner@vger.kernel.org List-ID: Switch both rmb()/mb() barriers to more lightweight smp_rmb()/smp_mb() ones. When walking the perf ring buffer they pair the following way, quoting kernel/events/ring_buffer.c: Since the mmap() consumer (userspace) can run on a different CPU: kernel user if (LOAD ->data_tail) { LOAD ->data_head (A) smp_rmb() (C) STORE $data LOAD $data smp_wmb() (B) smp_mb() (D) STORE ->data_head STORE ->data_tail } Where A pairs with D, and B pairs with C. In our case (A) is a control dependency that separates the load of the ->data_tail and the stores of $data. In case ->data_tail indicates there is no room in the buffer to store $data we do not. D needs to be a full barrier since it separates the data READ from the tail WRITE. For B a WMB is sufficient since it separates two WRITEs, and for C an RMB is sufficient since it separates two READs. Currently, on x86-64, perf uses LFENCE and MFENCE which is overkill as we can do more lightweight in particular given this is fast-path. According to Peter rmb()/mb() were added back then via a94d342b9cb0 ("tools/perf: Add required memory barriers") at a time where kernel still supported chips that needed it, but nowadays support for these has been ditched completely, therefore we can fix them up as well. Signed-off-by: Daniel Borkmann Cc: Peter Zijlstra Cc: "Paul E. McKenney" Cc: Will Deacon Cc: Arnaldo Carvalho de Melo --- tools/perf/util/mmap.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h index 05a6d47..de6dc2e 100644 --- a/tools/perf/util/mmap.h +++ b/tools/perf/util/mmap.h @@ -73,7 +73,8 @@ static inline u64 perf_mmap__read_head(struct perf_mmap *mm) { struct perf_event_mmap_page *pc = mm->base; u64 head = READ_ONCE(pc->data_head); - rmb(); + + smp_rmb(); return head; } @@ -84,7 +85,7 @@ static inline void perf_mmap__write_tail(struct perf_mmap *md, u64 tail) /* * ensure all reads are done before we write the tail out. */ - mb(); + smp_mb(); pc->data_tail = tail; } -- 2.9.5