lkml.org 
[lkml]   [2004]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2005-03-22 14:08    [from the cache]
©2003-2011 Jasper Spaans. Advertise on this site