Hi Jiri,

On 5/27/2020 6:28 PM, Jiri Olsa wrote:
On Wed, May 27, 2020 at 02:31:03PM +0800, Jin, Yao wrote:

SNIP

Thanks
Jin Yao

Issue is found!

It looks we can't set "pos->leader = pos" in either for_each_group_member()
or in for_each_group_evsel() because it may exit the iteration immediately.

        evlist__for_each_entry(evlist, evsel) {
                if (evsel->leader == evsel)
                        continue;

                if (cpu_maps_matched(evsel->leader, evsel))
                        continue;

                pr_warning("WARNING: event cpu maps are not fully matched, "
                           "disable group\n");

                for_each_group_member(pos, evsel->leader) {
                        pos->leader = pos;
                        pos->core.nr_members = 0;
                }

Let me use the example of '{cycles,unc_cbo_cache_lookup.any_i}' again.

In evlist:
cycles,
unc_cbo_cache_lookup.any_i,
unc_cbo_cache_lookup.any_i,
unc_cbo_cache_lookup.any_i,
unc_cbo_cache_lookup.any_i,

When we reach the for_each_group_member at first time, evsel is the first
unc_cbo_cache_lookup.any_i and evsel->leader is cycles. pos is same as the
evsel (the first unc_cbo_cache_lookup.any_i).

Once we execute "pos->leader = pos;", it's actually "evsel->leader = evsel".
So now evsel->leader is changed to the first unc_cbo_cache_lookup.any_i.

In next iteration, pos is the second unc_cbo_cache_lookup.any_i. pos->leader
is cycles but unfortunately evsel->leader has been changed to the first
unc_cbo_cache_lookup.any_i. So iteration stops immediately.

hum, AFAICS the iteration will not break but continue to next evsel and
pass the 'continue' for another group member.. what do I miss?

jirka


Let me use this example again.

cycles,
unc_cbo_cache_lookup.any_i,
unc_cbo_cache_lookup.any_i,
unc_cbo_cache_lookup.any_i,
unc_cbo_cache_lookup.any_i,

Yes, once for_each_group_member breaks (due to the issue in 'pos->leader = pos'), evlist__for_each_entry will continue to the second unc_cbo_cache_lookup.any_i. But now evsel->leader != evsel (evsel->leader is "cycles"), so it will go to cpu_maps_matched.

But actually we don't need to go to cpu_maps_matched again.

for_each_group_member(pos, evsel->leader) {
        pos->leader = pos;
        pos->core.nr_members = 0;
}

If we solve the issue in above code, for_each_group_member doesn't break, the leaders of all members in this group will be set to themselves.

if (evsel->leader == evsel)
        continue;

So the iteration will continue to the next evsel.

Thanks
Jin Yao

Reply via email to