Messages in this thread | | | Date | Mon, 29 Nov 2004 13:33:10 -0800 | From | Greg KH <> | Subject | Re: [PATCH] CKRM 1/10: Base CKRM Events |
| |
On Mon, Nov 29, 2004 at 10:46:00AM -0800, Gerrit Huizenga wrote: > +++ linux-2.6.10-rc2/include/linux/ckrm_events.h 2004-11-19 20:40:52.517303823 -0800 > @@ -0,0 +1,190 @@ > +/* > + * ckrm_events.h - Class-based Kernel Resource Management (CKRM) > + * event handling > + * > + * Copyright (C) Hubertus Franke, IBM Corp. 2003,2004 > + * (C) Shailabh Nagar, IBM Corp. 2003 > + * (C) Chandra Seetharaman, IBM Corp. 2003 > + * > + * > + * Provides a base header file including macros and basic data structures. > + * > + * Latest version, more details at http://ckrm.sf.net > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of version 2.1 of the GNU Lesser General Public License > + * as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it would be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > + * > + */ > + > +/* > + * Changes > + * > + * 28 Aug 2003 > + * Created. > + * 06 Nov 2003 > + * Made modifications to suit the new RBCE module. > + * 10 Nov 2003 > + * Added callbacks_active and surrounding logic. Added task paramter > + * for all CE callbacks. > + * 19 Nov 2004 > + * New Event callback structure > + */ > + > +#ifndef _LINUX_CKRM_EVENTS_H > +#define _LINUX_CKRM_EVENTS_H > + > +#ifdef CONFIG_CKRM
This ifdef is not needed.
> +/* > + * Data structure and function to get the list of registered > + * resource controllers. > + */ > + > +/* > + * CKRM defines a set of events at particular points in the kernel > + * at which callbacks registered by various class types are called > + */ > + > +enum ckrm_event {
Please use a __bitwise marker for your enum so that sparse will check for improper usage of it.
> +#ifdef __KERNEL__
Not needed (see the recent thread on lkml for why).
> +#ifdef CONFIG_CKRM > + > +/* > + * CKRM event callback specification for the classtypes or resource controllers > + * typically an array is specified using CKRM_EVENT_SPEC terminated with > + * CKRM_EVENT_SPEC_LAST and then that array is registered using > + * ckrm_register_event_set. > + * Individual registration of event_cb is also possible > + */ > + > +typedef void (*ckrm_event_cb) (void *arg); > + > +struct ckrm_hook_cb { > + ckrm_event_cb fct; > + struct ckrm_hook_cb *next;
Why not use the kernel list structures here instead?
> +#else /* !CONFIG_CKRM */
Why have 2 different sections in the same header file for this check? Please put them all together.
> +#ifdef CONFIG_CKRM > +void ckrm_cb_newtask(struct task_struct *); > +void ckrm_cb_exit(struct task_struct *); > +#else > +#define ckrm_cb_newtask(x) do { } while (0) > +#define ckrm_cb_exit(x) do { } while (0)
Make these static inlines to get the compiler to check the arguments properly.
> +extern int get_exe_path_name(struct task_struct *, char *, int);
Should this really be here?
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000 > +++ linux-2.6.10-rc2/kernel/ckrm/ckrm_events.c 2004-11-19 20:40:52.526302397 -0800 > @@ -0,0 +1,97 @@ > +/* ckrm_events.c - Class-based Kernel Resource Management (CKRM) > + * - event handling routines > + * > + * Copyright (C) Hubertus Franke, IBM Corp. 2003, 2004 > + * (C) Chandra Seetharaman, IBM Corp. 2003 > + * > + * > + * Provides API for event registration and handling for different > + * classtypes. > + * > + * Latest version, more details at http://ckrm.sf.net > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + */ > + > +/* Changes > + * > + * 29 Sep 2004 > + * Separated from ckrm.c > + * > + */
No changelogs in files please.
> +#define ECC_PRINTK(fmt, args...) \ > +// printk("%s: " fmt, __FUNCTION__ , ## args)
Looks like it's unused to me :)
Use pr_debug() if you want debugging stuff, don't make up your own...
thanks,
greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |