Commit
8062382c8dbe2 ("perf/x86/intel/bts: Add BTS PMU driver")
brought in a warning with the BTS buffer initialization that doesn't make
sense, but is easily tripped with (assuming KPTI is disabled):
instantly throwing:
> ------------[ cut here ]------------
> WARNING: CPU: 2 PID: 326 at arch/x86/events/intel/bts.c:86 bts_buffer_setup_aux+0x117/0x3d0
> Modules linked in:
> CPU: 2 PID: 326 Comm: perf Not tainted 5.4.0-rc8-00291-gceb9e77324fa #904
> RIP: 0010:bts_buffer_setup_aux+0x117/0x3d0
> Call Trace:
> rb_alloc_aux+0x339/0x550
> perf_mmap+0x607/0xc70
> mmap_region+0x76b/0xbd0
...
There is no comment or record anywhere that would explain the train of
thought that went into this warning, it probably tried to make sure that
the high order allocations indeed happened in the ring buffer code.
The rest of the driver handles multiple order-0 pages in the buffer just
fine. Therefore, get rid the warning. This was accidentally discovered by
Vince's perf_fuzzer.
Signed-off-by: Alexander Shishkin <[email protected]>
Fixes: 8062382c8dbe2 ("perf/x86/intel/bts: Add BTS PMU driver")
Cc: Vince Weaver <[email protected]>
---
arch/x86/events/intel/bts.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
index 38de4a7f6752..d53b4fb86d87 100644
--- a/arch/x86/events/intel/bts.c
+++ b/arch/x86/events/intel/bts.c
@@ -83,8 +83,6 @@ bts_buffer_setup_aux(struct perf_event *event, void **pages,
/* count all the high order buffers */
for (pg = 0, nbuf = 0; pg < nr_pages;) {
page = virt_to_page(pages[pg]);
- if (WARN_ON_ONCE(!PagePrivate(page) && nr_pages > 1))
- return NULL;
pg += 1 << page_private(page);
nbuf++;
}
--
2.24.0
On Thu, Dec 05, 2019 at 05:28:52PM +0300, Alexander Shishkin wrote:
> There is no comment or record anywhere that would explain the train of
> thought that went into this warning, it probably tried to make sure that
> the high order allocations indeed happened in the ring buffer code.
>
> --- a/arch/x86/events/intel/bts.c
> +++ b/arch/x86/events/intel/bts.c
> @@ -83,8 +83,6 @@ bts_buffer_setup_aux(struct perf_event *event, void **pages,
> /* count all the high order buffers */
> for (pg = 0, nbuf = 0; pg < nr_pages;) {
> page = virt_to_page(pages[pg]);
> - if (WARN_ON_ONCE(!PagePrivate(page) && nr_pages > 1))
> - return NULL;
> pg += 1 << page_private(page);
I'm thinking that because ^^^^ uses page_private(), it wants to make
sure PagePrivate().
I haven't checked the current rules, but using page_private() without
PagePrivate() seems dodgy.
Also consider:
+ __nr_pages = PagePrivate(page) ? 1 << page_private(page) : 1;
> nbuf++;
> }
> --
> 2.24.0
>