From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758206AbcGKKVG (ORCPT ); Mon, 11 Jul 2016 06:21:06 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:59410 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751847AbcGKKVD (ORCPT ); Mon, 11 Jul 2016 06:21:03 -0400 Subject: Re: [PATCH v13 2/8] perf evlist: Introduce aux evlist To: Jiri Olsa 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> <20160708144654.GA31763@krava> CC: , , , , He Kuang , Jiri Olsa , Masami Hiramatsu , Namhyung Kim , Nilay Vaish From: "Wangnan (F)" Message-ID: <578372EF.6060902@huawei.com> Date: Mon, 11 Jul 2016 18:20:31 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <20160708144654.GA31763@krava> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.111.66.109] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090201.5783730C.00A1,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: bcf20300dc222ceab81faa90e22ec7a4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016/7/8 22:46, Jiri Olsa wrote: > 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 I don't like it either, but aux_evlist is the easiest way to do this work. Other potential solutions require heavy API changes. Thank you.