Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754342AbcDTNU4 (ORCPT ); Wed, 20 Apr 2016 09:20:56 -0400 Received: from mail-pa0-f65.google.com ([209.85.220.65]:34063 "EHLO mail-pa0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751740AbcDTNUz (ORCPT ); Wed, 20 Apr 2016 09:20:55 -0400 Date: Wed, 20 Apr 2016 22:20:23 +0900 From: Namhyung Kim To: Wang Nan Cc: acme@kernel.org, jolsa@redhat.com, linux-kernel@vger.kernel.org, pi3orama@163.com, Adrian Hunter , He Kuang , Jiri Olsa , Masami Hiramatsu , Zefan Li Subject: Re: [PATCH v5 1/6] perf tools: Derive trigger class from auxtrace_snapshot Message-ID: <20160420132023.GB29186@danjae.aot.lge.com> References: <1460991332-185772-1-git-send-email-wangnan0@huawei.com> <1460991332-185772-2-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1460991332-185772-2-git-send-email-wangnan0@huawei.com> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2104 Lines: 60 Hello, On Mon, Apr 18, 2016 at 02:55:27PM +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 > ^ | > | (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. Not sure 'toggle' is the right word in this case. Maybe 'set/reset' or 'ready/hit' can be used, if each operation is one-way. But this is not a big deal anyway. Btw why not split this patch into two parts - introducing trigger logic and replacing snapshot code with the trigger? > > Signed-off-by: Wang Nan > Cc: Adrian Hunter > Cc: He Kuang > Cc: Jiri Olsa > Cc: Masami Hiramatsu > Cc: Namhyung Kim > Cc: Zefan Li > Cc: pi3orama@163.com > --- [SNIP] > +#define __TRIGGER_VAR(n) n##_state > +#define __DEF_TRIGGER_VOID_FUNC(n, op) \ > +static inline void n##_##op(void) {trigger_##op(&__TRIGGER_VAR(n)); } > +#define __DEF_TRIGGER_BOOL_FUNC(n, op) \ > +static inline bool n##_##op(void) {return trigger_##op(&__TRIGGER_VAR(n)); } > + > +#define DEFINE_TRIGGER(n) \ > +struct trigger n##_state = {.state = TRIGGER_OFF, .name = #n}; \ > +__DEF_TRIGGER_VOID_FUNC(n, on) \ > +__DEF_TRIGGER_VOID_FUNC(n, ready) \ > +__DEF_TRIGGER_VOID_FUNC(n, toggle) \ > +__DEF_TRIGGER_VOID_FUNC(n, off) \ > +__DEF_TRIGGER_VOID_FUNC(n, error) \ > +__DEF_TRIGGER_BOOL_FUNC(n, is_ready) \ > +__DEF_TRIGGER_BOOL_FUNC(n, is_toggled) \ > +__DEF_TRIGGER_BOOL_FUNC(n, is_error) Why did you define all functions for each trigger variable? Wouldn't it be better using generic trigger code and passing the trigger instead? Thanks, Namhyung