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    [W:0.045 / U:90.104 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site