Re: [PATCH v6 1/3] perf/core: use rb trees for pinned/flexible groups

[Alexey Budankov]

On 30.08.2017 11:30, Alexey Budankov wrote:
> On 29.08.2017 16:51, Alexander Shishkin wrote:
>> Alexey Budankov <alexey.budankov@xxxxxxxxxxxxxxx> writes:
>>
>>> Now I figured that not all indexed events are always located under 
>>> the root with the same cpu, and it depends on the order of insertion
>>> e.g. with insertion order 01,02,03,14,15,16 we get this:
>>>
>>>      02
>>>     /  \
>>>    01  14
>>>       /  \
>>>      03  15
>>>            \
>>>            16
>>>
>>> and it is unclear how to iterate cpu==0 part of tree in this case.
>>
>> Using this example, rb_next() should take you through the nodes in this
>> order (assuming you start with 01): 01, 02, 03, 14, etc. So you iterate
>> while event->cpu==cpu using rb_next() and you should be fine.
> 
> Well, indeed we get the most left leaf (03) in rb_next() for the case above.
> 
>>
>>> Iterating cpu specific subtree like this:
>>>
>>> #define for_each_group_event(event, group, cpu, pmu, field)	 \
>>> 	for (event = rb_entry_safe(group_first(group, cpu, pmu), \
>>> 				   typeof(*event), field);	 \
>>> 	     event && event->cpu == cpu && event->pmu == pmu;	 \
>>> 	     event = rb_entry_safe(rb_next(&event->field),	 \
>>> 				   typeof(*event), field))
>>
>> Afaict, this assumes that you are also ordering on event->pmu, which
>> should be reflected in your _less function. And also assuming that
>> group_first() is doing the right thing. Can we see the code?
> 
> I didn't do ordering by PMU for this patch set. Yet more I implemented 
> groups_first() like this:
> 
> static struct perf_event *
> perf_event_groups_first(struct perf_event_groups *groups, int cpu)
> {
> 	struct perf_event *node_event = NULL;
> 	struct rb_node *node = NULL;
> 
> 	node = groups->tree.rb_node;
> 
> 	while (node) {
> 		node_event = container_of(node,
> 				struct perf_event, group_node);
> 
> 		if (cpu < node_event->cpu) {
> 			node = node->rb_left;
> 		} else if (cpu > node_event->cpu) {
> 			node = node->rb_right;
> 		} else {
> 			node = node->rb_left;
> 		}
> 	}
> 
> 	return node_event;
> }
> 
> and it doesn't work as expected for case above with cpu == 1.
> 
> I corrected the code above to this:
> 
> static struct perf_event *
> perf_event_groups_first(struct perf_event_groups *groups, int cpu)
> {
> 	struct perf_event *node_event = NULL, *match = NULL;
> 	struct rb_node *node = NULL;
> 
> 	node = groups->tree.rb_node;
> 
> 	while (node) {
> 		node_event = container_of(node,
> 				struct perf_event, group_node);
> 
> 		if (cpu < node_event->cpu) {
> 			node = node->rb_left;
> 		} else if (cpu > node_event->cpu) {
> 			node = node->rb_right;
> 		} else {
> 			match = node_event;
> 			node = node->rb_left;
> 		}
> 	}
> 
> 	return match;
> }
> 
> but now struggling with silent oopses which I guess are not 
> related to multiplexing at all.

Added logging into the code and now see this in dmesg output:

[  175.743879] BUG: unable to handle kernel paging request at 00007fe2a90d1a54
[  175.743899] IP: __task_pid_nr_ns+0x3b/0x90
[  175.743903] PGD 2f317ca067 
[  175.743906] P4D 2f317ca067 
[  175.743910] PUD 0 

[  175.743926] Oops: 0000 [#1] SMP
[  175.743931] Modules linked in: fuse xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack libcrc32c tun bridge stp llc ebtable_filter ebtables ip6table_filter ip6_tables nfsv3 rpcsec_gss_krb5 nfsv4 cmac arc4 md4 nls_utf8 cifs nfs ccm dns_resolver fscache rpcrdma ib_isert iscsi_target_mod ib_iser libiscsi scsi_transport_iscsi ib_srpt target_core_mod ib_srp scsi_transport_srp ib_ipoib rdma_ucm ib_ucm ib_uverbs ib_umad rdma_cm ib_cm iw_cm intel_rapl sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp hfi1 crct10dif_pclmul crc32_pclmul ghash_clmulni_intel intel_cstate rdmavt joydev ipmi_ssif intel_uncore ib_core ipmi_si ipmi_devintf intel_rapl_perf iTCO_wdt iTCO_vendor_support pcspkr tpm_tis tpm_tis_core
[  175.744088]  mei_me tpm i2c_i801 ipmi_msghandler lpc_ich mei shpchp wmi acpi_power_meter acpi_pad nfsd auth_rpcgss nfs_acl lockd grace sunrpc mgag200 drm_kms_helper ttm drm igb crc32c_intel ptp pps_core dca i2c_algo_bit
[  175.744156] CPU: 12 PID: 8272 Comm: perf Not tainted 4.13.0-rc4-v7.3.3+ #13
[  175.744160] Hardware name: Intel Corporation S7200AP/S7200AP, BIOS S72C610.86B.01.01.0190.080520162104 08/05/2016
[  175.744165] task: ffff90c47d4d0000 task.stack: ffffae42d8fb0000
[  175.744177] RIP: 0010:__task_pid_nr_ns+0x3b/0x90
[  175.744181] RSP: 0018:ffff90c4bbd05ae0 EFLAGS: 00010046
[  175.744190] RAX: 0000000000000000 RBX: ffff90c47d4d0000 RCX: 00007fe2a90d1a50
[  175.744204] RDX: ffffffffbee4ed20 RSI: 0000000000000000 RDI: ffff90c47d4d0000
[  175.744209] RBP: ffff90c4bbd05ae0 R08: 0000000000281a93 R09: 0000000000000000
[  175.744213] R10: 0000000000000005 R11: 0000000000000000 R12: ffff90c46d25d800
[  175.744218] R13: ffff90c4bbd05c40 R14: ffff90c47d4d0000 R15: ffff90c46d25d800
[  175.744224] FS:  0000000000000000(0000) GS:ffff90c4bbd00000(0000) knlGS:0000000000000000
[  175.744228] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  175.744232] CR2: 00007fe2a90d1a54 CR3: 0000002f33bd2000 CR4: 00000000001406e0
[  175.744236] Call Trace:
[  175.744243]  <NMI>
[  175.744259]  perf_event_pid_type+0x27/0x40
[  175.744272]  __perf_event_header__init_id+0xb5/0xd0
[  175.744284]  perf_prepare_sample+0x54/0x360
[  175.744293]  perf_event_output_forward+0x2f/0x80
[  175.744304]  ? sched_clock+0xb/0x10
[  175.744315]  ? sched_clock_cpu+0x11/0xb0
[  175.744334]  __perf_event_overflow+0x54/0xe0
[  175.744346]  perf_event_overflow+0x14/0x20
[  175.744356]  intel_pmu_handle_irq+0x203/0x4b0
[  175.744379]  perf_event_nmi_handler+0x2d/0x50
[  175.744391]  nmi_handle+0x61/0x110
[  175.744402]  default_do_nmi+0x44/0x110
[  175.744411]  do_nmi+0x113/0x190
[  175.744421]  end_repeat_nmi+0x1a/0x1e
[  175.744432] RIP: 0010:native_write_msr+0x6/0x30
[  175.744437] RSP: 0018:ffffae42d8fb3c18 EFLAGS: 00000002
[  175.744452] RAX: 0000000000000003 RBX: ffff90c4bbd0a360 RCX: 000000000000038f
[  175.744457] RDX: 0000000000000007 RSI: 0000000000000003 RDI: 000000000000038f
[  175.744461] RBP: ffffae42d8fb3c30 R08: 0000000000281a93 R09: 0000000000000000
[  175.744464] R10: 0000000000000005 R11: 0000000000000000 R12: 0000000000000000
[  175.744468] R13: ffff90c4bbd0a360 R14: ffff90c4bbd0a584 R15: 0000000000000001
[  175.744485]  ? native_write_msr+0x6/0x30
[  175.744495]  ? native_write_msr+0x6/0x30
[  175.744504]  </NMI>
[  175.744514]  ? __intel_pmu_enable_all.isra.13+0x4f/0x80
[  175.744524]  intel_pmu_enable_all+0x10/0x20
[  175.744534]  x86_pmu_enable+0x263/0x2f0
[  175.744545]  perf_pmu_enable+0x22/0x30
[  175.744554]  ctx_resched+0x74/0xb0
[  175.744568]  perf_event_exec+0x17e/0x1e0
[  175.744584]  setup_new_exec+0x72/0x180
[  175.744595]  load_elf_binary+0x39f/0x15ea
[  175.744610]  ? get_user_pages_remote+0x83/0x1f0
[  175.744620]  ? __check_object_size+0x164/0x1a0
[  175.744632]  ? __check_object_size+0x164/0x1a0
[  175.744642]  ? _copy_from_user+0x33/0x70
[  175.744654]  search_binary_handler+0x9e/0x1e0
[  175.744664]  do_execveat_common.isra.31+0x53d/0x700
[  175.744677]  SyS_execve+0x3a/0x50
[  175.744691]  do_syscall_64+0x67/0x150
[  175.744702]  entry_SYSCALL64_slow_path+0x25/0x25
[  175.744709] RIP: 0033:0x7fe2a6cb77a7
[  175.744713] RSP: 002b:00007ffc554690b8 EFLAGS: 00000202 ORIG_RAX: 000000000000003b
[  175.744721] RAX: ffffffffffffffda RBX: 00007ffc5546b900 RCX: 00007fe2a6cb77a7
[  175.744725] RDX: 000000000151dd70 RSI: 00007ffc5546b900 RDI: 00007ffc5546d59e
[  175.744735] RBP: 00007ffc55469140 R08: 00007ffc554690a0 R09: 00007ffc55468f50
[  175.744740] R10: 00007ffc55468ed0 R11: 0000000000000202 R12: 000000000151dd70
[  175.744748] R13: 000000000082e740 R14: 0000000000000000 R15: 00007ffc5546d59e
[  175.744757] Code: bf 78 09 00 00 00 74 46 85 f6 75 35 89 f6 48 8d 04 76 48 8d 84 c7 70 09 00 00 48 8b 48 08 48 85 c9 74 2b 8b b2 30 08 00 00 31 c0 <3b> 71 04 77 0f 48 c1 e6 05 48 8d 4c 31 30 48 3b 51 08 74 0b 5d 
[  175.744910] RIP: __task_pid_nr_ns+0x3b/0x90 RSP: ffff90c4bbd05ae0
[  175.744914] CR2: 00007fe2a90d1a54

> 
> Please look at v8 for a while. It addresses your comments for v7.
> 
>>
>> Regards,
>> --
>> Alex
>>
> 
> Thanks,
> Alexey
>