From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755483AbcGHOrH (ORCPT ); Fri, 8 Jul 2016 10:47:07 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49378 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755245AbcGHOq5 (ORCPT ); Fri, 8 Jul 2016 10:46:57 -0400 Date: Fri, 8 Jul 2016 16:46:54 +0200 From: Jiri Olsa To: "Wangnan (F)" Cc: acme@kernel.org, linux-kernel@vger.kernel.org, pi3orama@163.com, lizefan@huawei.com, He Kuang , Jiri Olsa , Masami Hiramatsu , Namhyung Kim , Nilay Vaish Subject: Re: [PATCH v13 2/8] perf evlist: Introduce aux evlist Message-ID: <20160708144654.GA31763@krava> References: <1467613209-191781-1-git-send-email-wangnan0@huawei.com> <1467613209-191781-3-git-send-email-wangnan0@huawei.com> <20160706113615.GB26517@krava> <577CF6B4.6060303@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <577CF6B4.6060303@huawei.com> User-Agent: Mutt/1.6.1 (2016-04-27) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Fri, 08 Jul 2016 14:46:57 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 06, 2016 at 08:16:52PM +0800, Wangnan (F) wrote: > > > On 2016/7/6 19:36, Jiri Olsa wrote: > > On Mon, Jul 04, 2016 at 06:20:03AM +0000, Wang Nan wrote: > > > > SNIP > > > > > +struct perf_evlist *perf_evlist__new_aux(struct perf_evlist *parent) > > > +{ > > > + struct perf_evlist *evlist; > > > + > > > + if (perf_evlist__is_aux(parent)) { > > > + pr_err("Internal error: create aux evlist from another aux evlist\n"); > > > + return NULL; > > > + } > > > + > > > + evlist = zalloc(sizeof(*evlist)); > > > + if (!evlist) > > > + return NULL; > > > + > > > + perf_evlist__init(evlist, parent->cpus, parent->threads); > > > + evlist->parent = parent; > > > + INIT_LIST_HEAD(&evlist->list); > > > + list_add(&evlist->list, &parent->children); > > I understand there's some reason for separating maps with and > > without overwrite set, but I'm missing it.. why is that? > > You are asking overwrite, not write_backward? > > Overwrite mapping needs to be mapped without PROT_WRITE, so its > control page is also read only, so perf_evlist__mmap_consume() is > not able to use, and there's no way to tell kernel to where we have > read. Kernel overwrite old records when its full. Compare with normal > mapping: perf uses perf_evlist__mmap_consume() to tell kernel the > last byte it has read, so kernel stop writing data to it when it full, > and issues LOST event. This is the reason we need to separate maps > with and without overwrite set. > > For write backward: kernel write data in different direction, so > requires map separation. I dont like the idea of duplicating whole perf_evlist in order just to map some events with overwrite/backward perf_evlist carries all the other info about events, not just memory maping.. I think it'd be better to do it some other way, like: - we have mmaps for events/evsels, so you're able to map it differently with or without PROT_WRITE even in current design.. there's struct perf_mmap that can carry that info then it's the matter of reading/processing those maps that needs to change.. new perf_evlist interface - we could keep separate struct perf_mmap arrays for forward and backward/overwrite maps - ... I understand both mapping need different treatment, but I think that should be encapsulated within the struct perf_evlist interface thanks, jirka