From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754961Ab0EQR3T (ORCPT ); Mon, 17 May 2010 13:29:19 -0400 Received: from toro.web-alm.net ([62.245.132.31]:55951 "EHLO toro.web-alm.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753265Ab0EQR3R (ORCPT ); Mon, 17 May 2010 13:29:17 -0400 Message-ID: <4BF17C95.3060304@osadl.org> Date: Mon, 17 May 2010 19:27:49 +0200 From: Carsten Emde Organization: Open Source Automation Development Lab (OSADL) eG User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100330 Fedora/3.0.4-1.fc12 Thunderbird/3.0.4 MIME-Version: 1.0 To: Peter Zijlstra CC: LKML , Ingo Molnar , Steven Rostedt , Andrew Morton , Frederic Weisbecker Subject: Re: [PATCH 1/1] fix-task-states-in-sched_switch-event.patch References: <20100516221844.515632338@osadl.org> <20100516222207.541520625@osadl.org> <1274098973.5605.4695.camel@twins> <4BF15F08.5050108@osadl.org> <1274115243.5605.5255.camel@twins> In-Reply-To: <1274115243.5605.5255.camel@twins> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/17/2010 06:54 PM, Peter Zijlstra wrote: > On Mon, 2010-05-17 at 17:21 +0200, Carsten Emde wrote: > >>> Since we all love vile macro magic, is the below any better? >>> >>> include/linux/task_states.h >>> >>> TASK_STATE(RUNNING, "R", "running") >>> TASK_STATE(INTERRUPTIBLE, "S", "sleeping") >>> ... >> Well, yes, this looks very nice and is perfectly readable and >> maintainable. >> >>> enum { >>> #define TASK_STATE(tstate, tstate_c, tstate_s) __TASK_##tstate, >>> #include >>> #undef TASK_STATE >>> }; >>> >>> enum { >>> #define TASK_STATE(tstate, tstate_c, tstate_s) \ >>> TASK_##tstate = 1<< __TASK_##tstate, >>> #include >>> #undef TASK_STATE >>> }; >>> >>> const char *task_state_to_char = >>> #define TASK_STATE(tstate, tstate_c, tstate_s) tstate_c >>> #include >>> #undef TASK_STATE >>> ; >>> >>> const char *task_state_to_string[] = { >>> #define TASK_STATE(tstate, tstate_c, tstate_s) tstate_s, >>> #include >>> #undef TASK_STATE >>> }; >> I find this section less convincing (although certainly >> indistinguishable from magic). >> >> In addition, we need to take care of the various state name prefixes >> TASK, __TASK and EXIT and name clashes: >> TASK_RUNNING >> TASK_INTERRUPTIBLE >> TASK_UNINTERRUPTIBLE >> __TASK_STOPPED >> __TASK_TRACED >> EXIT_ZOMBIE >> EXIT_DEAD >> TASK_DEAD >> TASK_WAKEKILL >> TASK_WAKING > > We could manually add: > > #define EXIT_ZOMBIE TASK_ZOMBIE > #define EXIT_DEAD TASK_DEAD > > But those two __TASK ones are unfortunate indeed. > >> And we still need to maintain the defines in include/trace/events/ >> sched.h: >> { 1, TASK_STATE_1 } , { 2, TASK_STATE_2 }, >> { 4, TASK_STATE_4 }, { 8, TASK_STATE_8 }, >> { 16, TASK_STATE_16 }, { 32, TASK_STATE_32 }, >> { 64, TASK_STATE_64 }, { 128, TASK_STATE_128 }, >> { 256, TASK_STATE_256 } >> ) : TASK_STATE_0, > > #define TASK_STATE(tstate, tstate_c, tstate_s) \ > { __TASK_##tstate, tstate_c }, > #include > #undef TASK_STATE > > Should get you mostly there I guess, trick would be making the user deal > with { 0, "R" } > >> If we could use a general approach for all states, I would immediately >> go for your proposal. But since we anyway need to define the states >> individually, I would vote for the current version of the patch. >> >> Or would you prefer to simply apply a minimal fix to correct the >> erroneous output of the sched_switch event and to leave the rest as an >> exercise for the future? > > Dunno, I guess we can do with your version, just wanted to mention this > method. Yes, thanks, your method is great - much better than using intermediate scripts and similar. However, this approach works best, if you start with a new design and you are free to arrange everything to build on it. At least, we should have this approach in mind when making upgrade changes in the related code - maybe, your method will fit better one day. Carsten.