Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751731AbcDROUq (ORCPT ); Mon, 18 Apr 2016 10:20:46 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:12172 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751015AbcDROUp (ORCPT ); Mon, 18 Apr 2016 10:20:45 -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> CC: , , , Adrian Hunter , He Kuang , Jiri Olsa , Masami Hiramatsu , Namhyung Kim , Zefan Li From: "Wangnan (F)" Message-ID: <5714ED27.5090701@huawei.com> Date: Mon, 18 Apr 2016 22:20:23 +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: <20160418134544.GA13675@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: 1761 Lines: 45 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'. 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.