On Mon, 29 Nov 2004 14:23:23 PST, Greg KH wrote:
> On Mon, Nov 29, 2004 at 10:49:09AM -0800, Gerrit Huizenga wrote:
> > +#define TC_DEBUG(fmt, args...) do { \
> > +/* printk("%s: " fmt, __FUNCTION__ , ## args); */ } while (0)
> 
> Again with the new debug macro :(
> 
> > +static struct ckrm_task_class taskclass_dflt_class = {
> > +};
> 
> Empty structure?  Why?
 
Initialized definition, not declaration.  Although with no initializer
which was a bit odd.  struct ckrm_task_class is defined in ckrm_tc.h.

> > +// Hubertus .. following functions should move to ckrm_rc.h
> 
> Why haven't they moved :)

Because we aren't done yet.  ;-)

> > +static inline void ckrm_task_lock(struct task_struct *tsk)
> > +{
> > +   spin_lock(&tsk->ckrm_tsklock);
> > +}
> 
> Just lock (or unlock) the lock, don't wrap a lock in a function.
 
Yep.  Done.

> > +DECLARE_MUTEX(async_serializer);   // serialize all async functions
> 
> Should this really be global?  The code says otherwise :)
 
Not any more.

> > +   printk("...... Initializing ClassType<%s> ........\n",
> > +          CT_taskclass.name);
> 
> What a pretty log message.  Unfortunately it's wrong (me hears the
> growing mumblings of the kernel janitor mob...)
 
Yep - fixed.

> > +#if 0
> > +
> > +/******************************************************************************
> > + * Debugging Task Classes:  Utility functions
> > + 
> > ******************************************************************************/
> 
> Then remove the code, if it's not needed.
 
Okay.  I can easily carry a debug patch later.  Should have done that
sooner...

> > +EXPORT_SYMBOL(tcp_v4_lookup_listener);
> 
> Not EXPORT_SYMBOL_GPL()?
 
Currently makes it just like all the others.  I'll let the networking
folks chime in on how they want that exported when this patch gets
cross posted to netdev.

thanks,

gerrit


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
ckrm-tech mailing list
https://lists.sourceforge.net/lists/listinfo/ckrm-tech

Reply via email to