Date:   Thu, 31 Aug 2017 13:12:26 +0300
From:   Alexey Budankov <alexey.budankov@...ux.intel.com>
To:     Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Andi Kleen <ak@...ux.intel.com>,
        Kan Liang <kan.liang@...el.com>,
        Dmitri Prokhorov <Dmitry.Prohorov@...el.com>,
        Valery Cherepennikov <valery.cherepennikov@...el.com>,
        Mark Rutland <mark.rutland@....com>,
        Stephane Eranian <eranian@...gle.com>,
        David Carrillo-Cisneros <davidcc@...gle.com>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 1/3] perf/core: use rb trees for pinned/flexible groups

On 30.08.2017 14:16, Alexey Budankov wrote:
> On 30.08.2017 11:30, Alexey Budankov wrote:
>> On 29.08.2017 16:51, Alexander Shishkin wrote:
>>> Alexey Budankov <alexey.budankov@...ux.intel.com> 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

I eventually managed to overcome difficulties with implementation
of rb_tree indexed by {cpu,index} for event groups so please 
see patches v9.

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

Powered by blists - more mailing lists