Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751849AbcDROhZ (ORCPT ); Mon, 18 Apr 2016 10:37:25 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:26881 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750860AbcDROhY (ORCPT ); Mon, 18 Apr 2016 10:37:24 -0400 Subject: Re: [PATCH v4 1/6] perf tools: Derive trigger class from auxtrace_snapshot To: Jiri Olsa References: <1460961133-182746-1-git-send-email-wangnan0@huawei.com> <1460961133-182746-2-git-send-email-wangnan0@huawei.com> <20160418134544.GA13675@krava.brq.redhat.com> <5714ED27.5090701@huawei.com> <20160418142915.GB13675@krava.brq.redhat.com> CC: , , , Adrian Hunter , He Kuang , Jiri Olsa , Masami Hiramatsu , Namhyung Kim , Zefan Li From: "Wangnan (F)" Message-ID: <5714F110.2080101@huawei.com> Date: Mon, 18 Apr 2016 22:37:04 +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: <20160418142915.GB13675@krava.brq.redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.111.66.109] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2341 Lines: 63 On 2016/4/18 22:29, Jiri Olsa wrote: > On Mon, Apr 18, 2016 at 10:20:23PM +0800, Wangnan (F) wrote: >> >> On 2016/4/18 21:45, Jiri Olsa wrote: >>> On Mon, Apr 18, 2016 at 06:32:08AM +0000, Wang Nan wrote: >>>> Use 'trigger' to model operations which need to be executed when >>>> an event (a signal, for example) is observed. >>>> >>>> States and transits: >>>> >>>> OFF--(on)--> READY --(toggle)--> TOGGLED --(process)--> PROCESSING >>>> ^ | | >>>> | | | >>>> | (ready) (ready) >>>> | | | >>>> \__________________/______________________/ >>>> >>>> is_toggled and is_ready are two key functions to query the state of >>>> a trigger. is_toggled means the event already happen; is_ready means the >>>> trigger is waiting for the event. >>>> >>>> 'PROCESSING' represents a state the event happens and be observed, and >>>> the processing is on the way so can't accept a new event immediately. >>> hum, I must be missing something.. but I dont see how you're >>> using this state except for smal window within: >>> >>> if (auxtrace_snapshot_is_toggled()) { >>> -> auxtrace_snapshot_process(); >>> if (!auxtrace_snapshot_is_error()) >>> -> record__read_auxtrace_snapshot(rec); >>> >>> but no other place queries or depends on this state >> Right. >> >> Since we are creating a new class, I think we can make code simpler by >> merging >> all state variables into trigger. >> >> Without this state we must keep 'auxtrace_record__snapshot_started'. > do we? we dont need this for switch_output code apparently I think it is a good chance to clean existing code. However, number of lines introduced to .h in patch 1/6 is more than lines removed from .c. So I think removing the 'PROCESSING' is okay. If in future we find switch_output also need this state we can add it back at that time. Please wait for v5. Thank you. > jirka > >> I think merging it into trigger class makes the whole program a little >> bit simpler. Or do you think keeping trigger class simpler whould be better? >> >> Thank you. >> >>