On 10/03/2002 at 9:50 AM, Chrisopth Hellwig wrote:
>> +/* -*- linux-c -*- */
>> +/*
>> + * Copyright (c) International Business Machines Corp., 2000
> Has this file _really_ not been touched for two years??
Yup, it has, I've fixed them.
>> +/*
>> + * linux/include/linux/evms/evms.h
>> + *
>> + * EVMS kernel header file
>> + *
>> + */
> This comment sais exactly nothing. You aswell just remove it..
Agreed. The comment is obvious in this case. Its gone.
>> +
>> +#ifndef __EVMS_INCLUDED__
>> +#define __EVMS_INCLUDED__
>> +
>> +#include <linux/blk.h>
>> +#include <linux/genhd.h>
>> +#include <linux/fs.h>
>> +#include <linux/iobuf.h>
> I can't see the need for this include anywhere in the file. Please check
> whether you need all these includes.
Ok, done.
>> +
>> +/**
>> + * general defines section
>> + **/
>> +#define FALSE 0
>> +#define TRUE 1
> Please just use 0/1 directly just like everyone else..
More than happy to comply, however grep'ing the tree its
plain to see that not "everyone else" is following this
suggestion.
>> +#define DEV_PATH "/dev"
>> +#define EVMS_DIR_NAME "evms"
>> +#define EVMS_DEV_NAME "block_device"
>> +#define EVMS_DEV_NODE_PATH DEV_PATH "/" EVMS_DIR_NAME "/"
>> +#define EVMS_DEVICE_NAME DEV_PATH "/" EVMS_DIR_NAME "/"
EVMS_DEV_NAME
> The kernel doesn't know about device names at all.
I realize this is a goal, and I'm not opposed to it. However,
I know devfs is not popular, but people are using it, and it
*is* still available in 2.5. For the cases where ppl are
using it, the EVMS kernel component needs this info to tell
devfs the name of the devnode to create. I don't want to get
into a devfs flamewar, EVMS is simply offering interoperability
with what ppl can do today. Should that change, EVMS is more
than happy to adapt to the latest technology.
>> +
>> +/**
>> + * list_for_each_entry_safe - iterate over list safe against removal
of list entry
>> + * @pos: the type * to use as a loop counter.
>> + * @n: another type * to use as temporary storage
>> + * @head: the head for your list.
>> + * @member: the name of the list_struct within the struct.
>> + */
>> +#define list_for_each_entry_safe(pos, n, head, member)
\
>> + for (pos = list_entry((head)->next, typeof(*pos), member),
\
>> + n = list_entry(pos->member.next, typeof(*pos), member); \
>> + &pos->member != (head); \
>> + pos = n, \
>> + n = list_entry(pos->member.next, typeof(*pos), member))
>> +/**
>> + * list_member - tests whether a list member is currently on a list
>> + * @member: member to evaulate
>> + */
>> +static inline int
>> +list_member(struct list_head *member)
>> +{
>> + return ((!member->next || !member->prev) ? FALSE : TRUE);
>> +}
> This should go into list.h
Yes it should. I will pull this out of this header
and submit a patch for list.h.
>> +
>> +/**
>> + * kernel logging levels defines
>> + **/
>> +#define EVMS_INFO_CRITICAL 0
>> +#define EVMS_INFO_SERIOUS 1
>> +#define EVMS_INFO_ERROR 2
>> +#define EVMS_INFO_WARNING 3
>> +#define EVMS_INFO_DEFAULT 5
>> +#define EVMS_INFO_DETAILS 6
>> +#define EVMS_INFO_DEBUG 7
>> +#define EVMS_INFO_EXTRA 8
>> +#define EVMS_INFO_ENTRY_EXIT 9
>> +#define EVMS_INFO_EVERYTHING 10
> What about just using normal logging levels?
The quick answer is granularity. EVMS can generate a *lot*
of logging messages and we wanted a way to control the
amount of debugging messages. A simply ON/OFF didn't
quite seem adequate for many cases.
More to follow ....
Mark
-
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/
This archive was generated by hypermail 2b29 : Mon Oct 07 2002 - 22:00:41 EST