Hi Peter, Josh,
Found following bug. This bug can not be seen with this fix:
https://lkml.org/lkml/2018/5/10/280.
Here unwind_next_frame+0x463 is pointing at: "*ip = regs->ip;" in
deref_stack_iret_regs().
[ 2505.084076] BUG: KASAN: stack-out-of-bounds in
unwind_next_frame+0x463/0x850
[ 2505.084079] Read of size 8 at addr ffff8803d3d87970 by task
vhost-2815/2848
[ 2505.084083] CPU: 3 PID: 2848 Comm: vhost-2815 Not tainted 4.18.0-rc3+ #13
[ 2505.084084] Hardware name: Hewlett-Packard HP Z440 Workstation/212B,
BIOS M60 v02.34 05/18/2017
[ 2505.084085] Call Trace:
[ 2505.084087] <NMI>
[ 2505.084091] dump_stack+0x71/0xac
[ 2505.084096] print_address_description+0x65/0x22e
[ 2505.084099] ? unwind_next_frame+0x463/0x850
[ 2505.084101] kasan_report.cold.6+0x241/0x2fd
[ 2505.084104] unwind_next_frame+0x463/0x850
[ 2505.084109] ? native_iret+0x7/0x7
[ 2505.084111] ? deref_stack_reg+0xd0/0xd0
[ 2505.084115] __unwind_start+0x1c0/0x3c0
[ 2505.084117] ? unwind_next_frame+0x850/0x850
[ 2505.084121] ? perf_output_begin_forward+0x2df/0x460
[ 2505.084124] ? native_iret+0x7/0x7
[ 2505.084128] perf_callchain_kernel+0x19b/0x280
[ 2505.084131] ? arch_perf_update_userpage+0x1a0/0x1a0
[ 2505.084134] ? native_iret+0x7/0x7
[ 2505.084137] get_perf_callchain+0x1f7/0x3d0
[ 2505.084140] ? put_callchain_buffers+0x50/0x50
[ 2505.084143] perf_prepare_sample+0x805/0x990
[ 2505.084146] ? perf_output_sample+0xb90/0xb90
[ 2505.084151] ? cyc2ns_read_begin.part.2+0x67/0x90
[ 2505.084154] perf_event_output_forward+0x80/0x100
[ 2505.084157] ? perf_prepare_sample+0x990/0x990
[ 2505.084159] ? sched_clock+0x5/0x10
[ 2505.084161] ? perf_adjust_period+0x117/0x270
[ 2505.084163] ? __perf_event_account_interrupt+0x132/0x190
[ 2505.084166] __perf_event_overflow+0xaa/0x190
[ 2505.084169] __intel_pmu_pebs_event+0x349/0x3e0
[ 2505.084172] ? setup_pebs_sample_data+0x890/0x890
[ 2505.084175] ? stack_access_ok+0x35/0x80
[ 2505.084178] ? native_iret+0x7/0x7
[ 2505.084181] ? native_iret+0x7/0x7
[ 2505.084186] intel_pmu_drain_pebs_nhm+0x3c4/0x590
[ 2505.084189] ? __intel_pmu_pebs_event+0x3e0/0x3e0
[ 2505.084192] ? ktime_get_mono_fast_ns+0xdb/0x120
[ 2505.084194] ? intel_pmu_lbr_read+0x2e/0x7a0
[ 2505.084198] ? watchdog_overflow_callback+0x83/0xb0
[ 2505.084201] ? intel_bts_interrupt+0x7d/0x1a0
[ 2505.084203] intel_pmu_handle_irq+0x200/0x670
[ 2505.084206] ? intel_pmu_save_and_restart+0x80/0x80
[ 2505.084212] ? cyc2ns_read_begin.part.2+0x67/0x90
[ 2505.084214] ? native_sched_clock+0x75/0xf0
[ 2505.084217] ? cyc2ns_read_begin.part.2+0x90/0x90
[ 2505.084220] ? cyc2ns_read_begin.part.2+0x90/0x90
[ 2505.084223] perf_event_nmi_handler+0x40/0x60
[ 2505.084225] nmi_handle+0x73/0x150
[ 2505.084228] default_do_nmi+0x57/0x110
[ 2505.084231] do_nmi+0x141/0x1a0
[ 2505.084233] end_repeat_nmi+0x16/0x50
[ 2505.084236] RIP: 0010:deref_stack_reg+0x76/0xd0
[ 2505.084237] Code: c7 40 04 00 f2 f2 f2 65 48 8b 04 25 28 00 00 00 48
89 44 24 58 31 c0 e8 48 fe ff ff 31 d2 84 c0 74 23 48 89 ef 48 8d 74 24
20 <e8> 75 ff ff ff 48 8b 6c 24 20 4c 89 e7 e8 18 d3 32 00 ba 01 00 00
[ 2505.084263] RSP: 0018:ffff8803d3d87970 EFLAGS: 00000202
[ 2505.084266] RAX: 0000000000000001 RBX: 1ffff1007a7b0f2e RCX:
ffffffffa8075985
[ 2505.084267] RDX: 0000000000000000 RSI: ffff8803d3d87990 RDI:
ffff8803d3d87e20
[ 2505.084268] RBP: ffff8803d3d87e20 R08: fffffbfff54f23db R09:
fffffbfff54f23da
[ 2505.084270] R10: fffffbfff54f23da R11: ffffffffaa791ed1 R12:
ffff8803d3d87b10
[ 2505.084271] R13: 0000000000000002 R14: ffff8803d3d87b18 R15:
ffff8803d3d87b00
[ 2505.084274] ? stack_access_ok+0x35/0x80
[ 2505.084277] ? deref_stack_reg+0x76/0xd0
[ 2505.084279] ? deref_stack_reg+0x76/0xd0
[ 2505.084280] </NMI>
[ 2505.084281] <IRQ>
[ 2505.084284] ? __read_once_size_nocheck.constprop.7+0x10/0x10
[ 2505.084286] ? deref_stack_reg+0xd0/0xd0
[ 2505.084288] ? __orc_find+0x6f/0xc0
[ 2505.084291] unwind_next_frame+0x514/0x850
[ 2505.084295] ? __kfree_skb_flush+0x3c/0x50
[ 2505.084296] ? __kfree_skb_flush+0x3c/0x50
[ 2505.084299] ? deref_stack_reg+0xd0/0xd0
[ 2505.084305] ? vhost_worker+0x147/0x1e0 [vhost]
[ 2505.084309] ? is_module_text_address+0xa/0x11
[ 2505.084312] ? kernel_text_address+0x4c/0x110
[ 2505.084316] __save_stack_trace+0x82/0x100
[ 2505.084318] ? __kfree_skb_flush+0x3c/0x50
[ 2505.084320] save_stack+0x32/0xb0
[ 2505.084323] ? __kasan_slab_free+0x125/0x170
[ 2505.084326] ? kmem_cache_free_bulk+0x1af/0x3c0
[ 2505.084328] ? __kfree_skb_flush+0x3c/0x50
[ 2505.084331] ? net_rx_action+0x44b/0x630
[ 2505.084333] ? __do_softirq+0x114/0x383
[ 2505.084335] ? irq_exit+0x138/0x140
[ 2505.084337] ? do_IRQ+0x9a/0xe0
[ 2505.084339] ? common_interrupt+0xf/0xf
[ 2505.084345] ? iotlb_access_ok+0x260/0x260 [vhost]
[ 2505.084348] ? handle_rx+0x14a/0xe30 [vhost_net]
[ 2505.084353] ? vhost_worker+0x147/0x1e0 [vhost]
[ 2505.084357] ? kthread+0x1a0/0x1c0
[ 2505.084359] ? ret_from_fork+0x35/0x40
[ 2505.084362] ? skb_release_data+0x1fe/0x2d0
[ 2505.084381] ? ixgbe_update_itr.isra.63+0x170/0x2a0 [ixgbe]
[ 2505.084396] ? ixgbe_write_eitr+0x78/0xb0 [ixgbe]
[ 2505.084411] ? ixgbe_poll+0x26c4/0x2850 [ixgbe]
[ 2505.084414] __kasan_slab_free+0x125/0x170
[ 2505.084417] kmem_cache_free_bulk+0x1af/0x3c0
[ 2505.084419] ? __kfree_skb_flush+0x3c/0x50
[ 2505.084421] __kfree_skb_flush+0x3c/0x50
[ 2505.084424] net_rx_action+0x44b/0x630
[ 2505.084427] ? napi_complete_done+0x190/0x190
[ 2505.084430] __do_softirq+0x114/0x383
[ 2505.084432] irq_exit+0x138/0x140
[ 2505.084435] do_IRQ+0x9a/0xe0
[ 2505.084437] common_interrupt+0xf/0xf
[ 2505.084438] </IRQ>
[ 2505.084444] RIP: 0010:vq_iotlb_prefetch+0x0/0xe0 [vhost]
[ 2505.084444] Code: ff 48 89 dd e9 38 ff ff ff 48 8b 6c 24 10 e9 2e ff
ff ff 48 83 c4 30 31 c0 5b 5d 41 5c 41 5d 41 5e 41 5f c3 66 0f 1f 44 00
00 <0f> 1f 44 00 00 41 54 55 31 ed 53 48 89 fb 48 81 c7 30 45 00 00 e8
[ 2505.084470] RSP: 0018:ffff880355137b08 EFLAGS: 00000282 ORIG_RAX:
ffffffffffffffdb
[ 2505.084473] RAX: ffff88034fe24f58 RBX: ffff880373b845c8 RCX:
ffffffffc11b8fcd
[ 2505.084474] RDX: dffffc0000000000 RSI: 0000000000000008 RDI:
ffff880373b800a0
[ 2505.084475] RBP: 0000000000000000 R08: ffffed006aa26f57 R09:
ffffed006aa26f56
[ 2505.084477] R10: ffffed006aa26f56 R11: ffff880355137ab7 R12:
ffff880373b80000
[ 2505.084478] R13: 0000000000000000 R14: ffff880373b80000 R15:
ffff880373b800a0
[ 2505.084482] ? handle_rx+0x12d/0xe30 [vhost_net]
[ 2505.084486] handle_rx+0x14a/0xe30 [vhost_net]
[ 2505.084490] ? __update_load_avg_cfs_rq.isra.36+0x28/0x2a0
[ 2505.084492] ? update_load_avg+0x921/0xa30
[ 2505.084496] ? rb_erase_cached+0x83c/0x8a0
[ 2505.084499] ? peek_head_len+0x390/0x390 [vhost_net]
[ 2505.084502] ? speculative_store_bypass_update+0x210/0x210
[ 2505.084504] ? pick_next_entity+0xf2/0x1e0
[ 2505.084507] ? __list_add_valid+0x2d/0x70
[ 2505.084510] ? __switch_to+0x58f/0x600
[ 2505.084513] ? compat_start_thread+0x60/0x60
[ 2505.084516] ? finish_task_switch+0x101/0x3e0
[ 2505.084520] ? switch_mm_irqs_off+0x2c0/0x6d0
[ 2505.084522] ? __schedule+0x432/0xdf0
[ 2505.084529] vhost_worker+0x147/0x1e0 [vhost]
[ 2505.084534] ? vhost_dev_init+0x4e0/0x4e0 [vhost]
[ 2505.084537] ? __kthread_parkme+0xcc/0x100
[ 2505.084539] ? parse_args.cold.14+0xc4/0xc4
[ 2505.084545] ? vhost_dev_init+0x4e0/0x4e0 [vhost]
[ 2505.084547] kthread+0x1a0/0x1c0
[ 2505.084550] ? kthread_create_worker_on_cpu+0xc0/0xc0
[ 2505.084552] ret_from_fork+0x35/0x40
[ 2505.084555] The buggy address belongs to the page:
[ 2505.084557] page:ffffea000f4f61c0 count:1 mapcount:0
mapping:0000000000000000 index:0x0
[ 2505.084559] flags: 0x17ffffc0000800(reserved)
[ 2505.084563] raw: 0017ffffc0000800 ffffea000f4f61c8 ffffea000f4f61c8
0000000000000000
[ 2505.084565] raw: 0000000000000000 0000000000000000 00000001ffffffff
0000000000000000
[ 2505.084566] page dumped because: kasan: bad access detected
[ 2505.084567] Memory state around the buggy address:
[ 2505.084569] ffff8803d3d87800: 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00
[ 2505.084570] ffff8803d3d87880: 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00
[ 2505.084572] >ffff8803d3d87900: 00 00 00 00 00 00 00 00 00 00 00 00 00
00 f1 f1
[ 2505.084573]
^
[ 2505.084575] ffff8803d3d87980: f1 f1 00 f2 f2 f2 00 00 00 00 00 00 00
00 00 00
[ 2505.084576] ffff8803d3d87a00: 00 00 00 00 00 00 f1 f1 f1 f1 00 f2 f2
f2 00 00
[ 2505.084577]
==================================================================
[ 2505.084578] Disabling lock debugging due to kernel taint
[ 2508.883975] WARNING: stack going in the wrong direction?
ip=pktgen_xmit+0x4a9/0x1e30 [pktgen]
-Prashant
On Thu, Jul 19, 2018 at 01:33:54PM +0900, Prashant Bhole wrote:
> Hi Peter, Josh,
>
> Found following bug. This bug can not be seen with this fix:
> https://lkml.org/lkml/2018/5/10/280.
Peter, care to clean that up and submit it?
--
Josh
On Thu, Jul 19, 2018 at 10:33:47AM -0500, Josh Poimboeuf wrote:
> On Thu, Jul 19, 2018 at 01:33:54PM +0900, Prashant Bhole wrote:
> > Hi Peter, Josh,
> >
> > Found following bug. This bug can not be seen with this fix:
> > https://lkml.org/lkml/2018/5/10/280.
>
> Peter, care to clean that up and submit it?
Ah, thanks for the prod. Yes I'll go clean that up.
On Thu, Jul 19, 2018 at 07:43:11PM +0200, Peter Zijlstra wrote:
> On Thu, Jul 19, 2018 at 10:33:47AM -0500, Josh Poimboeuf wrote:
> > On Thu, Jul 19, 2018 at 01:33:54PM +0900, Prashant Bhole wrote:
> > > Hi Peter, Josh,
> > >
> > > Found following bug. This bug can not be seen with this fix:
> > > https://lkml.org/lkml/2018/5/10/280.
> >
> > Peter, care to clean that up and submit it?
>
> Ah, thanks for the prod. Yes I'll go clean that up.
Here goes; Ingo could you stuff in perf/urgent ?
---
Subject: perf/x86/intel: Fix unwind errors from PEBS entries (mk-II)
From: Peter Zijlstra <[email protected]>
Date: Thu, 10 May 2018 15:48:41 +0200
Vince reported the perf_fuzzer giving various unwinder warnings and
Josh reported:
On Sun, May 06, 2018 at 06:49:35PM -0500, Josh Poimboeuf wrote:
> Deja vu. Most of these are related to perf PEBS, similar to the
> following issue:
>
> b8000586c90b ("perf/x86/intel: Cure bogus unwind from PEBS entries")
>
> This is basically the ORC version of that. setup_pebs_sample_data() is
> assembling a franken-pt_regs which ORC isn't happy about. RIP is
> inconsistent with some of the other registers (like RSP and RBP).
And where the previous unwinder only needed BP,SP ORC also requires
IP. But we cannot spoof IP because then the sample will get displaced,
entirely negating the point of PEBS.
So cure the whole thing differently by doing the unwind early; this
does however require a means to communicate we did the unwind early.
We (ab)use an unused sample_type bit for this, which we set on events
that fill out the data->callchain before the normal
perf_prepare_sample().
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Debugged-by: Josh Poimboeuf <[email protected]>
Tested-by: Josh Poimboeuf <[email protected]>
Tested-by: Prashant Bhole <[email protected]>
Reported-by: Vince Weaver <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
arch/x86/events/intel/core.c | 3 +++
arch/x86/events/intel/ds.c | 25 +++++++++++--------------
include/linux/perf_event.h | 1 +
include/uapi/linux/perf_event.h | 2 ++
kernel/events/core.c | 6 ++++--
5 files changed, 21 insertions(+), 16 deletions(-)
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -2997,6 +2997,9 @@ static int intel_pmu_hw_config(struct pe
}
if (x86_pmu.pebs_aliases)
x86_pmu.pebs_aliases(event);
+
+ if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN)
+ event->attr.sample_type |= __PERF_SAMPLE_CALLCHAIN_EARLY;
}
if (needs_branch_stack(event)) {
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -1186,16 +1186,20 @@ static void setup_pebs_sample_data(struc
}
/*
+ * We must however always use iregs for the unwinder to stay sane; the
+ * record BP,SP,IP can point into thin air when the record is from a
+ * previous PMI context or an (I)RET happend between the record and
+ * PMI.
+ */
+ if (sample_type & PERF_SAMPLE_CALLCHAIN)
+ data->callchain = perf_callchain(event, iregs);
+
+ /*
* We use the interrupt regs as a base because the PEBS record does not
* contain a full regs set, specifically it seems to lack segment
* descriptors, which get used by things like user_mode().
*
* In the simple case fix up only the IP for PERF_SAMPLE_IP.
- *
- * We must however always use BP,SP from iregs for the unwinder to stay
- * sane; the record BP,SP can point into thin air when the record is
- * from a previous PMI context or an (I)RET happend between the record
- * and PMI.
*/
*regs = *iregs;
@@ -1214,15 +1218,8 @@ static void setup_pebs_sample_data(struc
regs->si = pebs->si;
regs->di = pebs->di;
- /*
- * Per the above; only set BP,SP if we don't need callchains.
- *
- * XXX: does this make sense?
- */
- if (!(sample_type & PERF_SAMPLE_CALLCHAIN)) {
- regs->bp = pebs->bp;
- regs->sp = pebs->sp;
- }
+ regs->bp = pebs->bp;
+ regs->sp = pebs->sp;
#ifndef CONFIG_X86_32
regs->r8 = pebs->r8;
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1130,6 +1130,7 @@ extern void perf_callchain_kernel(struct
extern struct perf_callchain_entry *
get_perf_callchain(struct pt_regs *regs, u32 init_nr, bool kernel, bool user,
u32 max_stack, bool crosstask, bool add_mark);
+extern struct perf_callchain_entry *perf_callchain(struct perf_event *event, struct pt_regs *regs);
extern int get_callchain_buffers(int max_stack);
extern void put_callchain_buffers(void);
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -143,6 +143,8 @@ enum perf_event_sample_format {
PERF_SAMPLE_PHYS_ADDR = 1U << 19,
PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
+
+ __PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
};
/*
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6343,7 +6343,7 @@ static u64 perf_virt_to_phys(u64 virt)
static struct perf_callchain_entry __empty_callchain = { .nr = 0, };
-static struct perf_callchain_entry *
+struct perf_callchain_entry *
perf_callchain(struct perf_event *event, struct pt_regs *regs)
{
bool kernel = !event->attr.exclude_callchain_kernel;
@@ -6382,7 +6382,9 @@ void perf_prepare_sample(struct perf_eve
if (sample_type & PERF_SAMPLE_CALLCHAIN) {
int size = 1;
- data->callchain = perf_callchain(event, regs);
+ if (!(sample_type & __PERF_SAMPLE_CALLCHAIN_EARLY))
+ data->callchain = perf_callchain(event, regs);
+
size += data->callchain->nr;
header->size += size * sizeof(u64);
Hi Peter,
I love your patch! Perhaps something to improve:
[auto build test WARNING on tip/perf/core]
[also build test WARNING on v4.18-rc5 next-20180719]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Peter-Zijlstra/perf-x86-intel-Fix-unwind-errors-from-PEBS-entries-mk-II/20180720-061741
config: i386-randconfig-x008-201828 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/linux/perf_event.h:17:0,
from include/linux/trace_events.h:10,
from include/trace/trace_events.h:20,
from include/trace/define_trace.h:96,
from include/trace/events/mce.h:75,
from arch/x86/kernel/cpu/mcheck/mce.c:63:
>> include/uapi/linux/perf_event.h:147:39: warning: left shift count >= width of type [-Wshift-count-overflow]
__PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
^~
vim +147 include/uapi/linux/perf_event.h
118
119 /*
120 * Bits that can be set in attr.sample_type to request information
121 * in the overflow packets.
122 */
123 enum perf_event_sample_format {
124 PERF_SAMPLE_IP = 1U << 0,
125 PERF_SAMPLE_TID = 1U << 1,
126 PERF_SAMPLE_TIME = 1U << 2,
127 PERF_SAMPLE_ADDR = 1U << 3,
128 PERF_SAMPLE_READ = 1U << 4,
129 PERF_SAMPLE_CALLCHAIN = 1U << 5,
130 PERF_SAMPLE_ID = 1U << 6,
131 PERF_SAMPLE_CPU = 1U << 7,
132 PERF_SAMPLE_PERIOD = 1U << 8,
133 PERF_SAMPLE_STREAM_ID = 1U << 9,
134 PERF_SAMPLE_RAW = 1U << 10,
135 PERF_SAMPLE_BRANCH_STACK = 1U << 11,
136 PERF_SAMPLE_REGS_USER = 1U << 12,
137 PERF_SAMPLE_STACK_USER = 1U << 13,
138 PERF_SAMPLE_WEIGHT = 1U << 14,
139 PERF_SAMPLE_DATA_SRC = 1U << 15,
140 PERF_SAMPLE_IDENTIFIER = 1U << 16,
141 PERF_SAMPLE_TRANSACTION = 1U << 17,
142 PERF_SAMPLE_REGS_INTR = 1U << 18,
143 PERF_SAMPLE_PHYS_ADDR = 1U << 19,
144
145 PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
146
> 147 __PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
148 };
149
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Peter,
I love your patch! Yet something to improve:
[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.18-rc5 next-20180719]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Peter-Zijlstra/perf-x86-intel-Fix-unwind-errors-from-PEBS-entries-mk-II/20180720-061741
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
In file included from include/linux/perf_event.h:17:0,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from arch/powerpc/kernel/ptrace.c:32:
>> include/uapi/linux/perf_event.h:147:39: error: left shift count >= width of type [-Werror=shift-count-overflow]
__PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
^~
cc1: all warnings being treated as errors
vim +147 include/uapi/linux/perf_event.h
118
119 /*
120 * Bits that can be set in attr.sample_type to request information
121 * in the overflow packets.
122 */
123 enum perf_event_sample_format {
124 PERF_SAMPLE_IP = 1U << 0,
125 PERF_SAMPLE_TID = 1U << 1,
126 PERF_SAMPLE_TIME = 1U << 2,
127 PERF_SAMPLE_ADDR = 1U << 3,
128 PERF_SAMPLE_READ = 1U << 4,
129 PERF_SAMPLE_CALLCHAIN = 1U << 5,
130 PERF_SAMPLE_ID = 1U << 6,
131 PERF_SAMPLE_CPU = 1U << 7,
132 PERF_SAMPLE_PERIOD = 1U << 8,
133 PERF_SAMPLE_STREAM_ID = 1U << 9,
134 PERF_SAMPLE_RAW = 1U << 10,
135 PERF_SAMPLE_BRANCH_STACK = 1U << 11,
136 PERF_SAMPLE_REGS_USER = 1U << 12,
137 PERF_SAMPLE_STACK_USER = 1U << 13,
138 PERF_SAMPLE_WEIGHT = 1U << 14,
139 PERF_SAMPLE_DATA_SRC = 1U << 15,
140 PERF_SAMPLE_IDENTIFIER = 1U << 16,
141 PERF_SAMPLE_TRANSACTION = 1U << 17,
142 PERF_SAMPLE_REGS_INTR = 1U << 18,
143 PERF_SAMPLE_PHYS_ADDR = 1U << 19,
144
145 PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
146
> 147 __PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
148 };
149
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Thu, Jul 19, 2018 at 11:19:54PM +0200, Peter Zijlstra wrote:
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -143,6 +143,8 @@ enum perf_event_sample_format {
> PERF_SAMPLE_PHYS_ADDR = 1U << 19,
>
> PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
> +
> + __PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
Given the kbuild test robot errors for building on 32-bit, looks like
the 63 needs to be changed to 31 so it fits in an int?
--
Josh
On Mon, Jul 23, 2018 at 08:30:06AM -0500, Josh Poimboeuf wrote:
> On Thu, Jul 19, 2018 at 11:19:54PM +0200, Peter Zijlstra wrote:
> > --- a/include/uapi/linux/perf_event.h
> > +++ b/include/uapi/linux/perf_event.h
> > @@ -143,6 +143,8 @@ enum perf_event_sample_format {
> > PERF_SAMPLE_PHYS_ADDR = 1U << 19,
> >
> > PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
> > +
> > + __PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
>
> Given the kbuild test robot errors for building on 32-bit, looks like
> the 63 needs to be changed to 31 so it fits in an int?
I made it 1ULL << 63. The actual field (perf_event_attr::sample_type is
u64).
On Mon, Jul 23, 2018 at 04:14:27PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 23, 2018 at 08:30:06AM -0500, Josh Poimboeuf wrote:
> > On Thu, Jul 19, 2018 at 11:19:54PM +0200, Peter Zijlstra wrote:
> > > --- a/include/uapi/linux/perf_event.h
> > > +++ b/include/uapi/linux/perf_event.h
> > > @@ -143,6 +143,8 @@ enum perf_event_sample_format {
> > > PERF_SAMPLE_PHYS_ADDR = 1U << 19,
> > >
> > > PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
> > > +
> > > + __PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
> >
> > Given the kbuild test robot errors for building on 32-bit, looks like
> > the 63 needs to be changed to 31 so it fits in an int?
>
> I made it 1ULL << 63. The actual field (perf_event_attr::sample_type is
> u64).
Ok, did you post this somewhere?
--
Josh
On Thu, Jul 19, 2018 at 2:21 PM Peter Zijlstra <[email protected]> wrote:
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -143,6 +143,8 @@ enum perf_event_sample_format {
> PERF_SAMPLE_PHYS_ADDR = 1U << 19,
>
> PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
> +
> + __PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
> };
The above change makes sparse unhappy :-( Sparse reports the following
complaint about __PERF_SAMPLE_CALLCHAIN_EARLY:
./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits
from constant value (8000000000000000 becomes 0)
Is the above change portable? Isn't a compiler allowed to use 'int' to
represent an enumeration type?
Fubo.
On Mon, Aug 06, 2018 at 08:35:07AM -0700, Fubo Chen wrote:
> On Thu, Jul 19, 2018 at 2:21 PM Peter Zijlstra <[email protected]> wrote:
> > --- a/include/uapi/linux/perf_event.h
> > +++ b/include/uapi/linux/perf_event.h
> > @@ -143,6 +143,8 @@ enum perf_event_sample_format {
> > PERF_SAMPLE_PHYS_ADDR = 1U << 19,
> >
> > PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
> > +
> > + __PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
> > };
>
> The above change makes sparse unhappy :-( Sparse reports the following
> complaint about __PERF_SAMPLE_CALLCHAIN_EARLY:
I thought I changed that to 1ULL before commit.
On Mon, Aug 6, 2018 at 8:42 AM Peter Zijlstra <[email protected]> wrote:
>
> On Mon, Aug 06, 2018 at 08:35:07AM -0700, Fubo Chen wrote:
> > On Thu, Jul 19, 2018 at 2:21 PM Peter Zijlstra <[email protected]> wrote:
> > > --- a/include/uapi/linux/perf_event.h
> > > +++ b/include/uapi/linux/perf_event.h
> > > @@ -143,6 +143,8 @@ enum perf_event_sample_format {
> > > PERF_SAMPLE_PHYS_ADDR = 1U << 19,
> > >
> > > PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
> > > +
> > > + __PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
> > > };
> >
> > The above change makes sparse unhappy :-( Sparse reports the following
> > complaint about __PERF_SAMPLE_CALLCHAIN_EARLY:
>
> I thought I changed that to 1ULL before commit.
The sparse complaint was reported for code with the "1ULL << 63".
Thanks,
Fubo.
On Mon, Aug 06, 2018 at 09:54:23AM -0700, Fubo Chen wrote:
> On Mon, Aug 6, 2018 at 8:42 AM Peter Zijlstra <[email protected]> wrote:
> >
> > On Mon, Aug 06, 2018 at 08:35:07AM -0700, Fubo Chen wrote:
> > > On Thu, Jul 19, 2018 at 2:21 PM Peter Zijlstra <[email protected]> wrote:
> > > > --- a/include/uapi/linux/perf_event.h
> > > > +++ b/include/uapi/linux/perf_event.h
> > > > @@ -143,6 +143,8 @@ enum perf_event_sample_format {
> > > > PERF_SAMPLE_PHYS_ADDR = 1U << 19,
> > > >
> > > > PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
> > > > +
> > > > + __PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
> > > > };
> > >
> > > The above change makes sparse unhappy :-( Sparse reports the following
> > > complaint about __PERF_SAMPLE_CALLCHAIN_EARLY:
> >
> > I thought I changed that to 1ULL before commit.
>
> The sparse complaint was reported for code with the "1ULL << 63".
Ah ok.. and I think I see what you mean. The C standard says that enums
shall be 'int'. However C++ standard says any integer type that fits the
largest value.
I suppose GCC uses the C++ definition and I suspect many other compilers
will too.
On Mon, Aug 6, 2018 at 11:04 AM Peter Zijlstra <[email protected]> wrote:
> On Mon, Aug 06, 2018 at 09:54:23AM -0700, Fubo Chen wrote:
> > On Mon, Aug 6, 2018 at 8:42 AM Peter Zijlstra <[email protected]> wrote:
> > > On Mon, Aug 06, 2018 at 08:35:07AM -0700, Fubo Chen wrote:
> > > > On Thu, Jul 19, 2018 at 2:21 PM Peter Zijlstra <[email protected]> wrote:
> > > > > --- a/include/uapi/linux/perf_event.h
> > > > > +++ b/include/uapi/linux/perf_event.h
> > > > > @@ -143,6 +143,8 @@ enum perf_event_sample_format {
> > > > > PERF_SAMPLE_PHYS_ADDR = 1U << 19,
> > > > >
> > > > > PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
> > > > > +
> > > > > + __PERF_SAMPLE_CALLCHAIN_EARLY = 1UL << 63,
> > > > > };
> > > >
> > > > The above change makes sparse unhappy :-( Sparse reports the following
> > > > complaint about __PERF_SAMPLE_CALLCHAIN_EARLY:
> > >
> > > I thought I changed that to 1ULL before commit.
> >
> > The sparse complaint was reported for code with the "1ULL << 63".
>
> Ah ok.. and I think I see what you mean. The C standard says that enums
> shall be 'int'. However C++ standard says any integer type that fits the
> largest value.
>
> I suppose GCC uses the C++ definition and I suspect many other compilers
> will too.
Do you think the patch below is sufficient to suppress the sparse warning?
---
include/uapi/linux/perf_event.h | 2 +-
tools/include/uapi/linux/perf_event.h | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index eeb787b1c53c..0f0f43599b74 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -144,8 +144,8 @@ enum perf_event_sample_format {
PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
- __PERF_SAMPLE_CALLCHAIN_EARLY = 1ULL << 63,
};
+#define __PERF_SAMPLE_CALLCHAIN_EARLY (1ULL << 63)
/*
* values to program into branch_sample_type when PERF_SAMPLE_BRANCH is set
diff --git a/tools/include/uapi/linux/perf_event.h
b/tools/include/uapi/linux/perf_event.h
index eeb787b1c53c..3db800bc8d2e 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -143,9 +143,8 @@ enum perf_event_sample_format {
PERF_SAMPLE_PHYS_ADDR = 1U << 19,
PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
-
- __PERF_SAMPLE_CALLCHAIN_EARLY = 1ULL << 63,
};
+#define __PERF_SAMPLE_CALLCHAIN_EARLY (1ULL << 63)
/*
* values to program into branch_sample_type when PERF_SAMPLE_BRANCH is set
--
Fubo.
On Mon, Aug 06, 2018 at 02:28:18PM -0700, Fubo Chen wrote:
> Do you think the patch below is sufficient to suppress the sparse warning?
Why would I want to make the code ugly to supress it?
On Mon, Aug 6, 2018 at 3:30 PM Peter Zijlstra <[email protected]> wrote:
>
> On Mon, Aug 06, 2018 at 02:28:18PM -0700, Fubo Chen wrote:
> > Do you think the patch below is sufficient to suppress the sparse warning?
>
> Why would I want to make the code ugly to supress it?
There are many kernel developers who use sparse to verify the
correctness of endianness annotations (__be32, __le32, ...). When
compiling kernel code with sparse every warning that is reported by
sparse should be analyzed. Most kernel developers consider it annoying
having to deal with false positive warnings. So I think that is useful
to suppress false positive sparse warnings if it is possible to
suppress false positives with a reasonable effort.
Thanks,
Fubo.
--
Fubo.
On Mon, Aug 06, 2018 at 04:04:40PM -0700, Fubo Chen wrote:
> On Mon, Aug 6, 2018 at 3:30 PM Peter Zijlstra <[email protected]> wrote:
> >
> > On Mon, Aug 06, 2018 at 02:28:18PM -0700, Fubo Chen wrote:
> > > Do you think the patch below is sufficient to suppress the sparse warning?
> >
> > Why would I want to make the code ugly to supress it?
>
> There are many kernel developers who use sparse to verify the
> correctness of endianness annotations (__be32, __le32, ...). When
> compiling kernel code with sparse every warning that is reported by
> sparse should be analyzed. Most kernel developers consider it annoying
> having to deal with false positive warnings. So I think that is useful
> to suppress false positive sparse warnings if it is possible to
> suppress false positives with a reasonable effort.
Last time I used sparse there were a metric ton of warnings. I really
can't be bothered about one more. Maybe fix sparse if you're bothered?