Messages in this thread | | | Date | Thu, 2 Mar 2023 14:44:28 +0800 | From | Leo Yan <> | Subject | Re: [PATCH v3 09/14] perf kvm: Use histograms list to replace cached list |
| |
On Tue, Feb 28, 2023 at 04:03:27PM -0800, Namhyung Kim wrote:
[...]
> > @@ -507,22 +530,26 @@ static struct kvm_event *find_create_kvm_event(struct perf_kvm_stat *kvm, > > struct perf_sample *sample) > > { > > struct kvm_event *event; > > - struct list_head *head; > > + struct rb_root_cached *root; > > + struct rb_node *nd; > > > > BUG_ON(key->key == INVALID_KEY); > > > > - head = &kvm->kvm_events_cache[kvm_events_hash_fn(key->key)]; > > - list_for_each_entry(event, head, hash_entry) { > > + if (hists__has(&kvm_hists.hists, need_collapse)) > > + root = &kvm_hists.hists.entries_collapsed; > > + else > > + root = kvm_hists.hists.entries_in; > > + > > + for (nd = rb_first_cached(root); nd; nd = rb_next(nd)) { > > + struct hist_entry *he = rb_entry(nd, struct hist_entry, > > + rb_node_in); > > + > > + event = container_of(he, struct kvm_event, he); > > if (event->key.key == key->key && event->key.info == key->info) > > return event; > > This seems inefficient and even unnecessary. You should find > the event based on the return value of hist_entry__cmp() from > the root and go down. > > But I think that's what hists__add_entry_ops() does. Maybe > you may need to move the init logic (like init_stats) to the > kvm_he_zalloc().
Indeed. I will drop above code and rely on hists__add_entry_ops() to check if have existed event entry.
Thanks a lot for suggestion!
Leo
| |