linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] EVMS core 2/4: evms.h
@ 2002-10-03 12:36 Kevin Corry
  2002-10-03 14:50 ` Christoph Hellwig
  0 siblings, 1 reply; 17+ messages in thread
From: Kevin Corry @ 2002-10-03 12:36 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, evms-devel

Greetings,

Here is part 2 of the EVMS core. This is the common header file included by
the core and all the plugins, and defines commonly used structures and macros.

Kevin Corry
corryk@us.ibm.com
http://evms.sourceforge.net/


diff -Naur linux-2.5.40/include/linux/evms/evms.h linux-2.5.40-evms/include/linux/evms/evms.h
--- linux-2.5.40/include/linux/evms/evms.h	Sun Jul 17 18:46:18 1994
+++ linux-2.5.40-evms/include/linux/evms/evms.h	Tue Oct  1 15:30:14 2002
@@ -0,0 +1,613 @@
+/* -*- linux-c -*- */
+/*
+ *   Copyright (c) International Business Machines  Corp., 2000
+ *
+ *   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.
+ * 
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ *   the GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program;  if not, write to the Free Software 
+ *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+/*
+ * linux/include/linux/evms/evms.h
+ *
+ * EVMS kernel header file
+ *
+ */
+
+#ifndef __EVMS_INCLUDED__
+#define __EVMS_INCLUDED__
+
+#include <linux/blk.h>
+#include <linux/genhd.h>
+#include <linux/fs.h>
+#include <linux/iobuf.h>
+#include <linux/kdev_t.h>
+#include <linux/hdreg.h>
+#include <linux/slab.h>
+#include <linux/proc_fs.h>
+#include <linux/major.h>
+#include <linux/mempool.h>
+
+/**
+ * version info 
+ **/
+#define EVMS_MAJOR_VERSION              1
+#define EVMS_MINOR_VERSION              2
+#define EVMS_PATCHLEVEL_VERSION         0
+
+/**
+ * general defines section
+ **/
+#define FALSE                           0
+#define TRUE                            1
+
+#define MAX_EVMS_VOLUMES                256
+#define EVMS_VOLUME_NAME_SIZE           127
+#define IBM_OEM_ID                      8112
+#define EVMS_INITIAL_CRC                0xFFFFFFFF
+#define EVMS_MAGIC_CRC			0x31415926
+#define EVMS_VSECTOR_SIZE               512
+#define EVMS_VSECTOR_SIZE_SHIFT         9
+
+#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
+
+/**
+ * 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);
+}
+
+/**
+ * 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
+
+/**
+ * kernel logging level	variable
+ **/
+extern int evms_info_level;
+
+/**
+ * kernel logging macros
+ **/
+#define evmsLOG(info_level,prspec) { if (evms_info_level >= info_level) printk prspec; }
+#define evmsLOG2(info_level,statement) { if (evms_info_level >= info_level) statement; }
+
+/**
+ * LOG MACROS to make evms log messages 
+ * look much cleaner in the source.
+ **/
+#define EVMS_LOG_PREFIX "evms: "
+#define LOG_CRITICAL(msg, args...)	evmsLOG(EVMS_INFO_CRITICAL,   (KERN_CRIT    EVMS_LOG_PREFIX LOG_PREFIX msg, ## args))
+#define LOG_SERIOUS(msg, args...)	evmsLOG(EVMS_INFO_SERIOUS,    (KERN_ERR     EVMS_LOG_PREFIX LOG_PREFIX msg, ## args))
+#define LOG_ERROR(msg, args...)		evmsLOG(EVMS_INFO_ERROR,      (KERN_ERR     EVMS_LOG_PREFIX LOG_PREFIX msg, ## args))
+#define LOG_WARNING(msg, args...)	evmsLOG(EVMS_INFO_WARNING,    (KERN_WARNING EVMS_LOG_PREFIX LOG_PREFIX msg, ## args))
+#define LOG_DEFAULT(msg, args...)	evmsLOG(EVMS_INFO_DEFAULT,    (KERN_INFO    EVMS_LOG_PREFIX LOG_PREFIX msg, ## args))
+#define LOG_DETAILS(msg, args...)	evmsLOG(EVMS_INFO_DETAILS,    (KERN_INFO    EVMS_LOG_PREFIX LOG_PREFIX msg, ## args))
+#define LOG_DEBUG(msg, args...)		evmsLOG(EVMS_INFO_DEBUG,      (KERN_INFO    EVMS_LOG_PREFIX LOG_PREFIX msg, ## args))
+#define LOG_EXTRA(msg, args...)		evmsLOG(EVMS_INFO_EXTRA,      (KERN_INFO    EVMS_LOG_PREFIX LOG_PREFIX msg, ## args))
+#define LOG_ENTRY_EXIT(msg, args...)	evmsLOG(EVMS_INFO_ENTRY_EXIT, (KERN_INFO    EVMS_LOG_PREFIX LOG_PREFIX msg, ## args))
+#define LOG_EVERYTHING(msg, args...)	evmsLOG(EVMS_INFO_EVERYTHING, (KERN_INFO    EVMS_LOG_PREFIX LOG_PREFIX msg, ## args))
+
+/**
+ * Macros to cleanly print 64-bit numbers on both 32-bit and 64-bit machines.
+ * Use these in place of %Ld, %Lu, and %Lx.
+ **/
+#if BITS_PER_LONG > 32
+#define PFD64 "%ld"
+#define PFU64 "%lu"
+#define PFX64 "%lx"
+#else
+#define PFD64 "%Ld"
+#define PFU64 "%Lu"
+#define PFX64 "%Lx"
+#endif
+
+/**
+ * helpful PROCFS macro
+ **/
+#ifdef CONFIG_PROC_FS
+#define PROCPRINT(msg, args...) (sz += sprintf(page + sz, msg, ## args));\
+			if (sz < off)\
+				off -= sz, sz = 0;\
+			else if (sz >= off + count)\
+				goto out
+#endif
+
+/**
+ * PluginID convenience macros
+ *
+ * An EVMS PluginID is a 32-bit number with the following bit positions:
+ * Top 16 bits: OEM identifier. See IBM_OEM_ID.
+ * Next 4 bits: Plugin type identifier. See evms_plugin_code.
+ * Lowest 12 bits: Individual plugin identifier within a given plugin type.
+ **/
+#define SetPluginID(oem, type, id) ((oem << 16) | (type << 12) | id)
+#define GetPluginOEM(pluginid) (pluginid >> 16)
+#define GetPluginType(pluginid) ((pluginid >> 12) & 0xf)
+#define GetPluginID(pluginid) (pluginid & 0xfff)
+
+/**
+ * enum evms_plugin_type - evms plugin types
+ **/
+enum evms_plugin_code {
+	EVMS_NO_PLUGIN = 0,
+	EVMS_DEVICE_MANAGER,
+	EVMS_SEGMENT_MANAGER,
+	EVMS_REGION_MANAGER,
+	EVMS_FEATURE,
+	EVMS_ASSOCIATIVE_FEATURE,
+	EVMS_FILESYSTEM_INTERFACE_MODULE,
+	EVMS_CLUSTER_MANAGER_INTERFACE_MODULE,
+	EVMS_DISTRIBUTED_LOCK_MANAGER_INTERFACE_MODULE
+};
+
+/**
+ * struct evms_version - 
+ * @major:	changes when incompatible difference are introduced
+ * @minor:	changes when additions are made
+ * @patchlevel:	reflects bug level fixes within a particular major/minor pair
+ *
+ * generic versioning info used by EVMS
+ **/
+struct evms_version {
+	u32 major;
+	u32 minor;
+	u32 patchlevel;
+};
+
+/**
+ * struct evms_plugin_header - kernel plugin header record
+ * @id: 			plugin id
+ * @version: 			plugin version
+ * @required_services_version: 	required common services version
+ * @fops: 			table of function operations
+ * @headers:			list member field
+ *
+ * kernel plugin header record
+ **/
+struct evms_plugin_header {
+	u32 id;
+	struct evms_version version;
+	struct evms_version required_services_version;
+	struct evms_plugin_fops *fops;
+	struct list_head headers;
+};
+
+/**
+ * struct evms_feature_header - EVMS generic on-disk header for features
+ * @signature: 			unique magic number
+ * @crc: 			structure's crc
+ * @version: 			feature header version
+ * @engine_version: 		created by this evms engine version
+ * @flags: 			feature characteristics, bit definitions below.
+ * @feature_id: 		indicates which feature this header is describing
+ * @sequence_number: 		describes most recent copy of redundant metadata
+ * @alignment_padding: 		used when objects are moved between different sized devices
+ * @feature_data1_start_lsn: 	object relative start of 1st copy feature data
+ * @feature_data1_size: 	size of 1st copy of feature data
+ * @feature_data2_start_lsn: 	object relative start of 2nd copy feature data
+ * @feature_data2_size: 	size of 2nd copy of feature data
+ * @volume_serial_number: 	unique/persistent volume identifier
+ * @volume_system_id: 		unique/persistent minor number
+ * @object_depth: 		depth of object in volume tree
+ * @object_name: 		object's name
+ * @volume_name: 		volume name object is a part of
+ * @pad: 			padding to make structure be 512 byte aligned
+ *
+ * generic on-disk header used to describe any EVMS feature
+ * NOTE: 2nd copy of feature data is optional, if used set start_lsn to 0.
+ **/
+struct evms_feature_header {
+	u32 signature;
+	u32 crc;
+	struct evms_version version;
+	struct evms_version engine_version;
+	u32 flags;
+	u32 feature_id;
+	u64 sequence_number;
+	u64 alignment_padding;
+	u64 feature_data1_start_lsn;
+	u64 feature_data1_size;
+	u64 feature_data2_start_lsn;
+	u64 feature_data2_size;
+	u64 volume_serial_number;
+	u32 volume_system_id;
+	u32 object_depth;
+	u8 object_name[EVMS_VOLUME_NAME_SIZE + 1];
+	u8 volume_name[EVMS_VOLUME_NAME_SIZE + 1];
+	u8 pad[152];
+};
+
+/**
+ * field evms_feature_header.signature majic number
+ **/
+#define EVMS_FEATURE_HEADER_SIGNATURE           0x54414546	/* FEAT */
+/**
+ * field evms_feature_header.flags defines
+ **/
+#define EVMS_FEATURE_ACTIVE                     (1<<0)
+#define EVMS_FEATURE_VOLUME_COMPLETE            (1<<1)
+#define EVMS_VOLUME_DATA_OBJECT			(1<<16)
+#define EVMS_VOLUME_DATA_STOP			(1<<17)
+/**
+ * struct evms_feature_header version info
+ **/
+#define EVMS_FEATURE_HEADER_MAJOR	3
+#define EVMS_FEATURE_HEADER_MINOR	0
+#define EVMS_FEATURE_HEADER_PATCHLEVEL	0
+
+/**
+ * EVMS specific error codes 
+ **/
+#define EVMS_FEATURE_FATAL_ERROR                257
+#define EVMS_VOLUME_FATAL_ERROR                 258
+#define EVMS_FEATURE_INCOMPLETE_ERROR		259
+
+/**
+ * struct evms_volume_info - exported volume info
+ * @volume_sn: 		unique volume identifier
+ * @volume_minor: 	persistent device minor assigned to this volume
+ * @volume_name: 	persistent name assigned to this volume
+ *
+ * a collection of volume specific info
+ **/
+struct evms_volume_info {
+	u64 volume_sn;
+	u32 volume_minor;
+	u8 volume_name[EVMS_VOLUME_NAME_SIZE + 1];
+};
+
+/**
+ * struct evms_logical_node - generic kernel storage object
+ * @total_vsectors: 	  0 size of this object in 512 byte units
+ * @plugin: 		  8 plugin that created/owns/manages this storage object
+ * @private: 		 12 location for owner to store private info
+ * @flags: 		 16 storage object characteristics (set/used by plugins)
+ *	   		    bit definitions located in evms_common.h
+ * @iflags: 		 20 internal flags (used exclusively by the framework, not for plugins to use/set)
+ *	    		    bit definitions below.
+ * @hardsector_size: 	 24 assumed physical sector size of underlying device
+ * @block_size: 	 28 default block size for this object
+ * @system_id: 		 32 system indicator (set by the segment manager)
+ * @volume_info: 	 36 persistent volume info, used only by EVMS volumes
+ * @feature_header: 	 40 generic on-disk metadata describing any EVMS feature
+ * @next: 		 44 linked list field
+ * @discover:		 48 discover list field
+ * @device:		 56 device list field
+ * @fbottom:		 64 bottom-most feature object list field
+ * @removable:		 72 changed removable media list field
+ * @consumer:		 80 list field for use by the object's consumer
+ * @name: 		 88 storage object name
+ *			216
+ *
+ * generic kernel storage object
+ */
+struct evms_logical_node {
+	u64 total_vsectors;
+	struct evms_plugin_header *plugin;
+	void *private;
+	u32 flags;
+	u32 iflags;
+	int hardsector_size;
+	int block_size;
+	u32 system_id;
+	struct evms_volume_info *volume_info;
+	struct evms_feature_header *feature_header;
+	void *consumer_private;
+	struct list_head consumed;
+	struct list_head produced;
+	struct list_head discover;
+	struct list_head device;
+	struct list_head fbottom;
+	struct list_head removable;
+	u8 name[EVMS_VOLUME_NAME_SIZE + 1];
+};
+
+/**
+ * fields evms_logical_node.flags & evms_logical_volume.flags defines
+ **/
+#define EVMS_FLAGS_WIDTH                   	32
+#define EVMS_VOLUME_FLAG                        (1<<0)
+#define EVMS_VOLUME_PARTIAL_FLAG                (1<<1)
+#define EVMS_VOLUME_PARTIAL			(1<<1)
+#define EVMS_VOLUME_SET_READ_ONLY               (1<<2)
+#define EVMS_VOLUME_READ_ONLY               	(1<<2)
+/**
+ * these bits define volume status 
+ **/
+#define EVMS_MEDIA_CHANGED			(1<<20)
+#define EVMS_DEVICE_UNPLUGGED			(1<<21)
+/**
+ * these bits used for removable status 
+ **/
+#define EVMS_DEVICE_MEDIA_PRESENT		(1<<24)
+#define EVMS_DEVICE_PRESENT			(1<<25)
+#define EVMS_DEVICE_LOCKABLE			(1<<26)
+#define EVMS_DEVICE_REMOVABLE			(1<<27)
+
+/**
+ * fields evms_logical_node.iflags defines
+ **/
+#define EVMS_FEATURE_BOTTOM			(1<<0)
+#define EVMS_TOP_SEGMENT			(1<<1)
+
+/**
+ * macro to obtain a node's name from either EVMS or compatibility volumes
+ **/
+#define EVMS_GET_NODE_NAME(node) 				\
+	((node->flags & EVMS_VOLUME_FLAG) ?			\
+		node->volume_info->volume_name :		\
+		node->name)
+
+/**
+ * macro used to transform to/from userland device handles and device storage object nodes
+ **/
+#define EVMS_HANDLE_KEY         0x0123456789ABCDEF
+#define DEV_HANDLE_TO_NODE(handle) ((struct evms_logical_node *)(unsigned long)((handle) ^ EVMS_HANDLE_KEY))
+#define NODE_TO_DEV_HANDLE(node) (((u64)(unsigned long)(node)) ^ EVMS_HANDLE_KEY)
+
+/**
+ * struct evms_logical_volume - logical volume info
+ * @name: 			logical volume name
+ * @node: 			logical volume storage object
+ * @minor:			device minor assigned to volume
+ * @flags: 			characteristics of logical volume
+ * @quiesced: 			quiesce state info
+ * @vfs_quiesced: 		vfs quiesce state info
+ * @requests_in_progress: 	count of in-flight I/Os
+ * @wait_queue: 		used when volume is quiesced
+ * @gd:				gendisk entry for the volume
+ * @request_queue: 		unique request queue
+ * @request_lock: 		unique request queue lock
+ * @volumes:			list member field
+ *
+ * contains all the fields needed to manage to a logical volume
+ **/
+struct evms_logical_volume {
+	u8 *name;
+	struct evms_logical_node *node;
+	int minor;
+	int flags;
+	int quiesced;
+	int vfs_quiesced;
+	atomic_t requests_in_progress;
+	wait_queue_head_t wait_queue;
+	struct gendisk *gd;
+	request_queue_t request_queue;
+	spinlock_t request_lock;
+	struct list_head volumes;
+};
+
+/**
+ * field evms_logical_volume.flags defines 
+ **/
+/**
+ * queued flags bits 
+ **/
+#define EVMS_REQUESTED_DELETE			(1<<5)
+#define EVMS_REQUESTED_QUIESCE			(1<<6)
+#define EVMS_REQUESTED_VFS_QUIESCE		(1<<7)
+/**
+ * this bit indicates corruption 
+ **/
+#define EVMS_VOLUME_CORRUPT			(1<<8)
+/**
+ * these bits define the source of the corruption 
+ **/
+#define EVMS_VOLUME_SOFT_DELETED               	(1<<9)
+#define EVMS_DEVICE_UNAVAILABLE			(1<<10)
+
+/*
+ * The following function table is used for all plugins.
+ */
+/**
+ * struct evms_plugin_fops - evms plugin's table of function operations
+ * @discover: 		volume discovery entry point
+ * @end_discover: 	final discovery entry point
+ * @delete: 		delete volume entry point
+ * @submit_io:		asynchronous read/write entry point
+ * @init_io: 		synchronous io entry point
+ * @ioctl: 		generic ioctl entry point
+ * @direct_ioctl: 	non-generic ioctl entry point
+ *
+ * evms plugin's table of function operations
+ **/
+struct evms_plugin_fops {
+	int (*discover) (struct list_head *);
+	int (*end_discover) (struct list_head *);
+	int (*delete) (struct evms_logical_node *);
+	void (*submit_io) (struct evms_logical_node *, struct bio *);
+	int (*init_io) (struct evms_logical_node *, int, u64,
+			u64, void *);
+	int (*ioctl) (struct evms_logical_node *, struct inode *,
+		      struct file *, u32, unsigned long);
+	int (*direct_ioctl) (struct inode *, struct file *,
+			     u32, unsigned long);
+};
+
+/**
+ * convenience macros to use plugin's fops entry points  
+ **/
+#define DISCOVER(node, list) ((plugin)->fops->discover(list))
+#define END_DISCOVER(node, list) ((plugin)->fops->end_discover(list))
+#define DELETE(node) ((node)->plugin->fops->delete(node))
+#define SUBMIT_IO(node, bio) ((node)->plugin->fops->submit_io(node, bio))
+#define INIT_IO(node, rw_flag, start_sec, num_secs, buf_addr) ((node)->plugin->fops->init_io(node, rw_flag, start_sec, num_secs, buf_addr))
+#define IOCTL(node, inode, file, cmd, arg)    ((node)->plugin->fops->ioctl(node, inode, file, cmd, arg))
+#define DIRECT_IOCTL(plugin, inode, file, cmd, arg)   ((plugin)->fops->direct_ioctl(inode, file, cmd, arg))
+
+/**
+ * struct evms_list_node - generic non-imbedded list node object
+ * @item:	ptr to object in list
+ * @next:	ptr to next item in list
+ *
+ * light weight generic non-imbedded list object definition
+ **/
+struct evms_list_node {
+	void *item;
+	struct evms_list_node *next;
+};
+
+/**
+ * struct evms_pool_mgmt - anchor block for private pool management
+ * @cachep: 		kmem_cache_t variable
+ * @member_size: 	size of each element in the pool
+ * @head:
+ * @waiters: 		count of waiters
+ * @wait_queue: 	list of waiters
+ * @name: 		name of the pool (must be less than 20 chars)
+ *
+ * anchor block for private pool management
+ **/
+struct evms_pool_mgmt {
+	kmem_cache_t *cachep;
+	int member_size;
+	void *head;
+	atomic_t waiters;
+	wait_queue_head_t wait_queue;
+	u8 *name;
+};
+
+/*
+ * Notes:  
+ *	All of the following kernel thread functions belong to EVMS base.
+ *	These functions were copied from md_core.c
+ */
+#define EVMS_THREAD_WAKEUP 0
+/**
+ * struct evms_thread
+ * @run:	
+ * @data:	
+ * @wqueue:	thread wait queue
+ * @flags:	thread attributes
+ * @event:	event completion
+ * @tsk:	task info
+ * @name:	thread name
+ *
+ * data structure for creating/managing a kernel thread
+ **/
+struct evms_thread {
+	void (*run) (void *data);
+	void *data;
+	wait_queue_head_t wqueue;
+	unsigned long flags;
+	struct completion *event;
+	struct task_struct *tsk;
+	const u8 *name;
+};
+
+/**
+ * struct bio_split_cb - bio split control block structure definition
+ * @rc:
+ * @sector:
+ * @original_bio:
+ * @outstanding_bios:
+ * @pool:
+ *
+ * control block for managing bio splitting
+ **/
+struct bio_split_cb {
+	int rc;
+	u64 sector;
+	struct bio *original_bio;
+	atomic_t outstanding_bios;
+	mempool_t *bio_pool;
+	mempool_t *split_pool;
+};
+
+/**
+ * EVMS (common services) exported functions prototypes 
+ *
+ * since these function names are global, evms_cs_ has been prepended
+ * to each function name, to ensure they do not collide with any
+ * other global functions in the kernel.
+ **/
+#define EVMS_COMMON_SERVICES_MAJOR              0
+#define EVMS_COMMON_SERVICES_MINOR              6
+#define EVMS_COMMON_SERVICES_PATCHLEVEL         0
+
+void evms_cs_get_version(int *, int *);
+int evms_cs_check_version(struct evms_version *, struct evms_version *);
+int evms_cs_register_plugin(struct evms_plugin_header *);
+int evms_cs_unregister_plugin(struct evms_plugin_header *);
+#ifdef EVMS_MEM_DEBUG
+int evms_cs_verify_memory_integrity(int);
+#endif
+int evms_cs_allocate_logical_node(struct evms_logical_node **);
+void evms_cs_deallocate_volume_info(struct evms_logical_node *);
+void evms_cs_deallocate_logical_node(struct evms_logical_node *);
+int evms_cs_kernel_ioctl(struct evms_logical_node *, u32,
+			 unsigned long);
+inline unsigned long evms_cs_size_in_vsectors(long long);
+inline int evms_cs_log2(long long);
+u32 evms_cs_calculate_crc(u32, void *, u32);
+int evms_cs_register_for_end_io_notification(void *,
+					     struct bio *,
+					     void *callback_function);
+struct evms_list_node **evms_cs_lookup_item_in_list(struct evms_list_node **,
+						    void *);
+void evms_cs_signal_event(int);
+struct evms_thread *evms_cs_register_thread(void (*run) (void *),
+					    void *data, const u8 *name);
+void evms_cs_unregister_thread(struct evms_thread *thread);
+void evms_cs_wakeup_thread(struct evms_thread *thread);
+void evms_cs_interrupt_thread(struct evms_thread *thread);
+struct proc_dir_entry *evms_cs_get_evms_proc_dir(void);
+int evms_cs_volume_request_in_progress(kdev_t, int, int *);
+void evms_cs_invalidate_volume(struct evms_logical_node *topmost_node);
+int evms_cs_split_bio(struct bio *, u64, struct bio **, struct bio **,
+		      mempool_t *, mempool_t *);
+request_queue_t *evms_find_queue(kdev_t dev);
+
+/* EVMS exported global variables */
+extern struct evms_pool_mgmt *evms_bio_pool;
+extern u8 *evms_primary_string;
+extern u8 *evms_secondary_string;
+extern struct list_head evms_device_list;
+
+/* Have to include this at the end, since it depends
+ * on structures and definitions in this file.
+ */
+#include <linux/evms/evms_ioctl.h>
+
+#endif

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] EVMS core 2/4: evms.h
  2002-10-03 12:36 [PATCH] EVMS core 2/4: evms.h Kevin Corry
@ 2002-10-03 14:50 ` Christoph Hellwig
  2002-10-03 15:10   ` [Evms-devel] " Michael Clark
  0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2002-10-03 14:50 UTC (permalink / raw)
  To: Kevin Corry; +Cc: torvalds, linux-kernel, evms-devel

> diff -Naur linux-2.5.40/include/linux/evms/evms.h linux-2.5.40-evms/include/linux/evms/evms.h
> --- linux-2.5.40/include/linux/evms/evms.h	Sun Jul 17 18:46:18 1994
> +++ linux-2.5.40-evms/include/linux/evms/evms.h	Tue Oct  1 15:30:14 2002

What about drivers/md?

> @@ -0,0 +1,613 @@
> +/* -*- linux-c -*- */
> +/*
> + *   Copyright (c) International Business Machines  Corp., 2000

Has this file _really_ not been touched for two years??

> +/*
> + * linux/include/linux/evms/evms.h
> + *
> + * EVMS kernel header file
> + *
> + */

This comment sais exactly nothing.  You aswell just remove it..

> +
> +#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.
> +
> +/**
> + * general defines section
> + **/
> +#define FALSE                           0
> +#define TRUE                            1

Please just use 0/1 directly just like everyone else..

> +#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.

> +
> +/**
> + * 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

> +
> +/**
> + * 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?

> +/**
> + * helpful PROCFS macro
> + **/
> +#ifdef CONFIG_PROC_FS
> +#define PROCPRINT(msg, args...) (sz += sprintf(page + sz, msg, ## args));\
> +			if (sz < off)\
> +				off -= sz, sz = 0;\
> +			else if (sz >= off + count)\
> +				goto out
> +#endif

I think this really wants to be converted to the seq_file interface..

> +
> +/**
> + * PluginID convenience macros
> + *
> + * An EVMS PluginID is a 32-bit number with the following bit positions:
> + * Top 16 bits: OEM identifier. See IBM_OEM_ID.
> + * Next 4 bits: Plugin type identifier. See evms_plugin_code.
> + * Lowest 12 bits: Individual plugin identifier within a given plugin type.
> + **/
> +#define SetPluginID(oem, type, id) ((oem << 16) | (type << 12) | id)
> +#define GetPluginOEM(pluginid) (pluginid >> 16)
> +#define GetPluginType(pluginid) ((pluginid >> 12) & 0xf)
> +#define GetPluginID(pluginid) (pluginid & 0xfff)

What is the prupose of OEM IDs?

> +struct evms_plugin_header {
> +	u32 id;
> +	struct evms_version version;
> +	struct evms_version required_services_version;
> +	struct evms_plugin_fops *fops;
> +	struct list_head headers;
> +};

What is the required services version?

> +struct evms_plugin_fops {

What about evms_plugin_ops?

> +	int (*ioctl) (struct evms_logical_node *, struct inode *,
> +		      struct file *, u32, unsigned long);
> +	int (*direct_ioctl) (struct inode *, struct file *,
> +			     u32, unsigned long);

Umm, why do you need two ioctl handlers?

> +/**
> + * convenience macros to use plugin's fops entry points  
> + **/
> +#define DISCOVER(node, list) ((plugin)->fops->discover(list))
> +#define END_DISCOVER(node, list) ((plugin)->fops->end_discover(list))
> +#define DELETE(node) ((node)->plugin->fops->delete(node))
> +#define SUBMIT_IO(node, bio) ((node)->plugin->fops->submit_io(node, bio))
> +#define INIT_IO(node, rw_flag, start_sec, num_secs, buf_addr) ((node)->plugin->fops->init_io(node, rw_flag, start_sec, num_secs, buf_addr))
> +#define IOCTL(node, inode, file, cmd, arg)    ((node)->plugin->fops->ioctl(node, inode, file, cmd, arg))
> +#define DIRECT_IOCTL(plugin, inode, file, cmd, arg)   ((plugin)->fops->direct_ioctl(inode, file, cmd, arg))

Do you really need those wrapper?  They just obsfucate the code

> +/**
> + * struct evms_pool_mgmt - anchor block for private pool management
> + * @cachep: 		kmem_cache_t variable
> + * @member_size: 	size of each element in the pool
> + * @head:
> + * @waiters: 		count of waiters
> + * @wait_queue: 	list of waiters
> + * @name: 		name of the pool (must be less than 20 chars)
> + *
> + * anchor block for private pool management
> + **/
> +struct evms_pool_mgmt {
> +	kmem_cache_t *cachep;
> +	int member_size;
> +	void *head;
> +	atomic_t waiters;
> +	wait_queue_head_t wait_queue;
> +	u8 *name;
> +};

What's the pruipose of this?  Is it really evms-specific?

> +
> +/*
> + * Notes:  
> + *	All of the following kernel thread functions belong to EVMS base.
> + *	These functions were copied from md_core.c
> + */

What about moving them to the core kernel code so everyone
can use them?

> +/* Have to include this at the end, since it depends
> + * on structures and definitions in this file.
> + */
> +#include <linux/evms/evms_ioctl.h>

Just remove this and make the individual sources include it


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
  2002-10-03 14:50 ` Christoph Hellwig
@ 2002-10-03 15:10   ` Michael Clark
  2002-10-03 15:14     ` Christoph Hellwig
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Clark @ 2002-10-03 15:10 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Kevin Corry, torvalds, linux-kernel, evms-devel


>>+#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.

It does for specifying root devices and for devfs.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
  2002-10-03 15:10   ` [Evms-devel] " Michael Clark
@ 2002-10-03 15:14     ` Christoph Hellwig
  2002-10-03 16:22       ` Linus Torvalds
  0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2002-10-03 15:14 UTC (permalink / raw)
  To: Michael Clark
  Cc: Christoph Hellwig, Kevin Corry, torvalds, linux-kernel, evms-devel

On Thu, Oct 03, 2002 at 11:10:14PM +0800, Michael Clark wrote:
> 
> >>+#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.
> 
> It does for specifying root devices and for devfs.A

root device should be in do_mount.c and not in obscure headers.
devfs doesn'T need hardcoded "/dev" prefixes, and it's better to not
add defines for the strings in devfs so that crap doesn't get spread
over the code but is localized to the existing devfs damage

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
  2002-10-03 15:14     ` Christoph Hellwig
@ 2002-10-03 16:22       ` Linus Torvalds
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Torvalds @ 2002-10-03 16:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Michael Clark, Kevin Corry, linux-kernel, evms-devel


On Thu, 3 Oct 2002, Christoph Hellwig wrote:
> 
> root device should be in do_mount.c and not in obscure headers.

No, they should _not_ be in do_mount.c either. They should be in the 
driver registration, and do_mount.c should not have a random list of 
devices. 

I'm not accepting do_mount.c expansion here, simply because I don't want 
to help a horribly broken interface. You can always use a hex number 
(which is what things like lilo will install anyway, I believe, rather 
than using the "root=/dev/xxx" command line), and if people get too tired 
about remembering numbers, maybe somebody who cares will step up to the 
plate and write a reverse of "__bdevname()" and do it right.

Hint: see __bdevname in fs/block_dev.c, and realize that it does the 
"kdev->name" translation without _any_ tables at all. Think about doing 
the same the other way, by just walking the registered block devices.

		Linus


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
  2002-10-04 18:45 Steve Pratt
@ 2002-10-07 17:25 ` Christoph Hellwig
  0 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2002-10-07 17:25 UTC (permalink / raw)
  To: Steve Pratt; +Cc: Mark Peloquin, torvalds, linux-kernel, evms-devel

> >> and raid and LVM them all you want, and you don't consume 1000*layers
> >> device nodes.
> 
> >I don't think it's a benefit but really ugly.  There is no reason to now

sorry, that "now" should have been a 'not'.

> >allow access to the lower layers.  How do I e.g. write a new volume label
> to
> >the lower level devices?
> 
> I am not sure I understand your question.  Did you mean that there does not
> appear to be a **way** to access lower level devices or did you really mean
> no reason to do so?

Sorry again for above typo - it makes my whole statement worthless..

To clarify: I think not having device nodes for anything but the uppermost
layer of evms volumes is a bad idea.  This does not only make it impossible
to access those normally from userspace but also makes evms duplicate
code in the block layers as we already have stacking support there.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
@ 2002-10-04 18:45 Steve Pratt
  2002-10-07 17:25 ` Christoph Hellwig
  0 siblings, 1 reply; 17+ messages in thread
From: Steve Pratt @ 2002-10-04 18:45 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Mark Peloquin, torvalds, linux-kernel, evms-devel


Christoph Hellwig wrote:
>On Fri, Oct 04, 2002 at 08:34:02AM -0600, Andreas Dilger wrote:
>> On Oct 04, 2002  15:14 +0100, Christoph Hellwig wrote:
>> > > the IOCTL entry point is used to send to volumes.
>> > > the DIRECT_IOCTL entry point is used for point-
>> > > to-point ioctls between corresponding user space
>> > > and kernel space plugins.
>> >
>> > Do the ioctl directly to the device node of the lower layer plugin
instead.
>>
>> Not possible - EVMS doesn't export the lower-level device nodes at all.
>> That is one of the benefits - you can take 1000 drives and stack them
>> and raid and LVM them all you want, and you don't consume 1000*layers
>> device nodes.

>I don't think it's a benefit but really ugly.  There is no reason to now
>allow access to the lower layers.  How do I e.g. write a new volume label
to
>the lower level devices?

I am not sure I understand your question.  Did you mean that there does not
appear to be a **way** to access lower level devices or did you really mean
no reason to do so?  The reason to do so is for plug-ins such as MD which
may want to kick an array member out of the array which is a command
specific to MD.  The way in which this is accomplished in EVMS is as Mark
described above through the use of the DIRECT_IOCTL which allows for direct
communication to intermediate levels of an EVMS volume stack.

Note that this method is only used for certain cases like the MD one
described.  For normal IO type operations such as writing metadata or
changing volume labels, since EVMS in userspace knows about ALL of the
layers of the volume stack we can resolve all metadata writes to disk
relative offsets in our userspace config tools and thus don't have to write
to intermediate nodes to build more complex volumes.

Steve




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
  2002-10-04 14:34   ` Andreas Dilger
@ 2002-10-04 17:10     ` Christoph Hellwig
  0 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2002-10-04 17:10 UTC (permalink / raw)
  To: Mark Peloquin, torvalds, linux-kernel, evms-devel

On Fri, Oct 04, 2002 at 08:34:02AM -0600, Andreas Dilger wrote:
> On Oct 04, 2002  15:14 +0100, Christoph Hellwig wrote:
> > > the IOCTL entry point is used to send to volumes.
> > > the DIRECT_IOCTL entry point is used for point-
> > > to-point ioctls between corresponding user space
> > > and kernel space plugins.
> > 
> > Do the ioctl directly to the device node of the lower layer plugin instead.
> 
> Not possible - EVMS doesn't export the lower-level device nodes at all.
> That is one of the benefits - you can take 1000 drives and stack them
> and raid and LVM them all you want, and you don't consume 1000*layers
> device nodes.

I don't think it's a benefit but really ugly.  There is no reason to now
allow access to the lower layers.  How do I e.g. write a new volume label to
the lower level devices?

> Um, how about EXT3_I() and EXT3_SB(), or almost any filesystem in
> 2.5 which hides inode->u.generic_ip->foo_inode_info->blah?

That one actually provides a benfit as we have two different 24 and one 2.5 method
to access it.  I'm speaking about the wrappers for function pointer
invocations.  And yes, XFS has some massive macro abuse, but it's legacy
code that's not to easy to change while EVMS is new, from-the-scratch code that
should rather do it right.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
@ 2002-10-04 14:59 Mark Peloquin
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Peloquin @ 2002-10-04 14:59 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: torvalds, linux-kernel, evms-devel


On 10/04/2002 at 9:08 AM, Christoph Hellwig wrote:
> You can"t know where devfs is mounted.  So of the above only
EVMS_DIR_NAME
> and EVMS_DEV_NAME make sense.

Agreed.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
  2002-10-04 14:08 ` Christoph Hellwig
@ 2002-10-04 14:36   ` Richard B. Johnson
  0 siblings, 0 replies; 17+ messages in thread
From: Richard B. Johnson @ 2002-10-04 14:36 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Mark Peloquin, torvalds, linux-kernel, evms-devel

On Fri, 4 Oct 2002, Christoph Hellwig wrote:

> On Thu, Oct 03, 2002 at 12:42:23PM -0500, Mark Peloquin wrote:
> > >> +#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.
> 
> Sure there are other offenders in the drivers, from known offenders like
> Richard or IBM, but no core code.  There is a reason why theses defines are
> not in kernel.h..

Is it because TRUE is not ~FALSE? or because FALSE is not ~TRUE or
is it because TRUE is not !FALSE? or because FALSE is not !TRUE?

		^;)

Cheers,
Dick Johnson
Penguin : Linux version 2.4.18 on an i686 machine (797.90 BogoMips).
The US military has given us many words, FUBAR, SNAFU, now ENRON.
Yes, top management were graduates of West Point and Annapolis.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
  2002-10-04 14:14 ` Christoph Hellwig
@ 2002-10-04 14:34   ` Andreas Dilger
  2002-10-04 17:10     ` Christoph Hellwig
  0 siblings, 1 reply; 17+ messages in thread
From: Andreas Dilger @ 2002-10-04 14:34 UTC (permalink / raw)
  To: Christoph Hellwig, Mark Peloquin, torvalds, linux-kernel, evms-devel

On Oct 04, 2002  15:14 +0100, Christoph Hellwig wrote:
> > the IOCTL entry point is used to send to volumes.
> > the DIRECT_IOCTL entry point is used for point-
> > to-point ioctls between corresponding user space
> > and kernel space plugins.
> 
> Do the ioctl directly to the device node of the lower layer plugin instead.

Not possible - EVMS doesn't export the lower-level device nodes at all.
That is one of the benefits - you can take 1000 drives and stack them
and raid and LVM them all you want, and you don't consume 1000*layers
device nodes.

> > >> +/**
> > >> + * convenience macros to use plugin's fops entry points
> > >> + **/
> > >> +#define DISCOVER(node, list) ((plugin)->fops->discover(list))
> > >> +#define END_DISCOVER(node, list) ((plugin)->fops->end_discover(list))
> > >> +#define DELETE(node) ((node)->plugin->fops->delete(node))
> > >> +#define SUBMIT_IO(node, bio) ((node)->plugin->fops->submit_io(node,
> > bio))
> > >> +#define INIT_IO(node, rw_flag, start_sec, num_secs, buf_addr) >((node)
> > ->plugin->fops->init_io(node, rw_flag, start_sec, num_secs, buf_addr))
> > >> +#define IOCTL(node, inode, file, cmd, arg)    ((node)
> > ->plugin->fops->ioctl(node, >inode, file, cmd, arg))
> > >> +#define DIRECT_IOCTL(plugin, inode, file, cmd, arg)   >((plugin)
> > ->fops->direct_ioctl(inode, file, cmd, arg))
> > 
> > > Do you really need those wrapper?
> > 
> > No the wrappers aren't really needed. However they do make the
> > code a great deal more readable.
> > 
> > > They just obsfucate the code
> > 
> > The same argument could be made about *all* macros then.
> > Its simply a tradeoff between readability and potential
> > hiding.
> 
> CodingStyle is one big flamewar.  As no other operations vector
> in the linux kernel uses such wrappers the syle police seems to
> be on my side in this case :)

Um, how about EXT3_I() and EXT3_SB(), or almost any filesystem in
2.5 which hides inode->u.generic_ip->foo_inode_info->blah?  Given
that you want to keep lines < 80 chars where possible having short
names to access common methods is a big win, IMHO.

Are you telling me XFS doesn't have some awful abstractions in it too?
VOP_GETATTR(), ugh.  So, yes it's nice to have useful criticism, but
no need to pick every space and tab to death.

Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
  2002-10-03 18:59 Mark Peloquin
@ 2002-10-04 14:14 ` Christoph Hellwig
  2002-10-04 14:34   ` Andreas Dilger
  0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2002-10-04 14:14 UTC (permalink / raw)
  To: Mark Peloquin; +Cc: torvalds, linux-kernel, evms-devel

> >> +#define SetPluginID(oem, type, id) ((oem << 16) | (type << 12) | id)
> >> +#define GetPluginOEM(pluginid) (pluginid >> 16)
> >> +#define GetPluginType(pluginid) ((pluginid >> 12) & 0xf)
> >> +#define GetPluginID(pluginid) (pluginid & 0xfff)
> 
> > What is the prupose of OEM IDs?
> 
> To allow unique identification of plugins. If you wrote
> plugin, you might give it an ID of 1. Someone else
> may do the same thing. However, by letting you add
> something specific which identifies you (i.e. like
> your initials), then possibility of collisions is
> reduced.

Please stop using magic numbers in the kernel _at all_.  The only
sensible thing against collision is proper naming, i.e. strings.

> 
> >> +struct evms_plugin_header {
> >> +  u32 id;
> >> +  struct evms_version version;
> >> +  struct evms_version required_services_version;
> >> +  struct evms_plugin_fops *fops;
> >> +  struct list_head headers;
> >> +};
> 
> > What is the required services version?
> 
> The common services are a set of functions exported
> by the core code. We have major,minor,patchlevel
> versions for them. Plugin writers specify the
> version of the interface they are coded to comply
> with. Mismatching core services and plugin
> version expectations are caught at plugin registration
> (load) time, and prevented from being usable.

Doesn't work in a linux enviroment.  People just have to stick to the
kernel version they write for.  If you think you really need it make
such checks at the cpp level as there is no such thing as binary
compatiblity anyway.

> the IOCTL entry point is used to send to volumes.
> the DIRECT_IOCTL entry point is used for point-
> to-point ioctls between corresponding user space
> and kernel space plugins.

Do the ioctl directly to the device node of the lower layer plugin instead.

> 
> >> +/**
> >> + * convenience macros to use plugin's fops entry points
> >> + **/
> >> +#define DISCOVER(node, list) ((plugin)->fops->discover(list))
> >> +#define END_DISCOVER(node, list) ((plugin)->fops->end_discover(list))
> >> +#define DELETE(node) ((node)->plugin->fops->delete(node))
> >> +#define SUBMIT_IO(node, bio) ((node)->plugin->fops->submit_io(node,
> bio))
> >> +#define INIT_IO(node, rw_flag, start_sec, num_secs, buf_addr) >((node)
> ->plugin->fops->init_io(node, rw_flag, start_sec, num_secs, buf_addr))
> >> +#define IOCTL(node, inode, file, cmd, arg)    ((node)
> ->plugin->fops->ioctl(node, >inode, file, cmd, arg))
> >> +#define DIRECT_IOCTL(plugin, inode, file, cmd, arg)   >((plugin)
> ->fops->direct_ioctl(inode, file, cmd, arg))
> 
> > Do you really need those wrapper?
> 
> No the wrappers aren't really needed. However they do make the
> code a great deal more readable.
> 
> > They just obsfucate the code
> 
> The same argument could be made about *all* macros then.
> Its simply a tradeoff between readability and potential
> hiding.

CodingStyle is one big flamewar.  As no other operations vector
in the linux kernel uses such wrappers the syle police seems to
be on my side in this case :)


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
  2002-10-03 17:42 Mark Peloquin
  2002-10-04 12:28 ` Robert Varga
@ 2002-10-04 14:08 ` Christoph Hellwig
  2002-10-04 14:36   ` Richard B. Johnson
  1 sibling, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2002-10-04 14:08 UTC (permalink / raw)
  To: Mark Peloquin; +Cc: torvalds, linux-kernel, evms-devel

On Thu, Oct 03, 2002 at 12:42:23PM -0500, Mark Peloquin wrote:
> >> +#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.

Sure there are other offenders in the drivers, from known offenders like
Richard or IBM, but no core code.  There is a reason why theses defines are
not in kernel.h..

> 
> >> +#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 n do today. Should that change, EVMS is more
> than happy to adapt to the latest technology.

You can"t know where devfs is mounted.  So of the above only EVMS_DIR_NAME
and EVMS_DEV_NAME make sense.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
@ 2002-10-04 13:59 Mark Peloquin
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Peloquin @ 2002-10-04 13:59 UTC (permalink / raw)
  To: Robert Varga; +Cc: linux-kernel


On 10/04/2002 at 7:28 AM, Robert Varga wrote:
<snip...>
> Possibly shortened to:

> static inline int list_member(struct list_head *member)
> {
>     return member->next && member->prev;
> }

> Faster, and (at least to me) it looks more obvious.

Yes, this may be shorter. However with this change
the return type would also need to be changed to
portable across archs.

Mark



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
  2002-10-03 17:42 Mark Peloquin
@ 2002-10-04 12:28 ` Robert Varga
  2002-10-04 14:08 ` Christoph Hellwig
  1 sibling, 0 replies; 17+ messages in thread
From: Robert Varga @ 2002-10-04 12:28 UTC (permalink / raw)
  To: Mark Peloquin; +Cc: linux-kernel

On Thu, Oct 03, 2002 at 12:42:23PM -0500, Mark Peloquin wrote:
> >> +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.

Possibly shortened to:

static inline int list_member(struct list_head *member)
{
	return member->next && member->prev;
}

Faster, and (at least to me) it looks more obvious.

-- 
Kind regards,
Robert Varga
------------------------------------------------------------------------------
n@hq.sk                                          http://hq.sk/~nite/gpgkey.txt

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
@ 2002-10-03 18:59 Mark Peloquin
  2002-10-04 14:14 ` Christoph Hellwig
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Peloquin @ 2002-10-03 18:59 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: torvalds, linux-kernel, evms-devel


On 10/03/2002 at 9:50 AM, Christoph Hellwig wrote:

>> +/**
>> + * helpful PROCFS macro
>> + **/
>> +#ifdef CONFIG_PROC_FS
>> +#define PROCPRINT(msg, args...) (sz += sprintf(page + sz, msg, ##
args));\
>> +              if (sz < off)\
>> +                    off -= sz, sz = 0;\
>> +              else if (sz >= off + count)\
>> +                    goto out
>> +#endif

> I think this really wants to be converted to the seq_file interface..

We plan to.

>> +
>> +/**
>> + * PluginID convenience macros
>> + *
>> + * An EVMS PluginID is a 32-bit number with the following bit
positions:
>> + * Top 16 bits: OEM identifier. See IBM_OEM_ID.
>> + * Next 4 bits: Plugin type identifier. See evms_plugin_code.
>> + * Lowest 12 bits: Individual plugin identifier within a given plugin
type.
>> + **/
>> +#define SetPluginID(oem, type, id) ((oem << 16) | (type << 12) | id)
>> +#define GetPluginOEM(pluginid) (pluginid >> 16)
>> +#define GetPluginType(pluginid) ((pluginid >> 12) & 0xf)
>> +#define GetPluginID(pluginid) (pluginid & 0xfff)

> What is the prupose of OEM IDs?

To allow unique identification of plugins. If you wrote
plugin, you might give it an ID of 1. Someone else
may do the same thing. However, by letting you add
something specific which identifies you (i.e. like
your initials), then possibility of collisions is
reduced.

>> +struct evms_plugin_header {
>> +  u32 id;
>> +  struct evms_version version;
>> +  struct evms_version required_services_version;
>> +  struct evms_plugin_fops *fops;
>> +  struct list_head headers;
>> +};

> What is the required services version?

The common services are a set of functions exported
by the core code. We have major,minor,patchlevel
versions for them. Plugin writers specify the
version of the interface they are coded to comply
with. Mismatching core services and plugin
version expectations are caught at plugin registration
(load) time, and prevented from being usable.

>> +struct evms_plugin_fops {

> What about evms_plugin_ops?

>> +  int (*ioctl) (struct evms_logical_node *, struct inode *,
>> +              struct file *, u32, unsigned long);
>> +  int (*direct_ioctl) (struct inode *, struct file *,
>> +                   u32, unsigned long);

> Umm, why do you need two ioctl handlers?

the IOCTL entry point is used to send to volumes.
the DIRECT_IOCTL entry point is used for point-
to-point ioctls between corresponding user space
and kernel space plugins.

>> +/**
>> + * convenience macros to use plugin's fops entry points
>> + **/
>> +#define DISCOVER(node, list) ((plugin)->fops->discover(list))
>> +#define END_DISCOVER(node, list) ((plugin)->fops->end_discover(list))
>> +#define DELETE(node) ((node)->plugin->fops->delete(node))
>> +#define SUBMIT_IO(node, bio) ((node)->plugin->fops->submit_io(node,
bio))
>> +#define INIT_IO(node, rw_flag, start_sec, num_secs, buf_addr) >((node)
->plugin->fops->init_io(node, rw_flag, start_sec, num_secs, buf_addr))
>> +#define IOCTL(node, inode, file, cmd, arg)    ((node)
->plugin->fops->ioctl(node, >inode, file, cmd, arg))
>> +#define DIRECT_IOCTL(plugin, inode, file, cmd, arg)   >((plugin)
->fops->direct_ioctl(inode, file, cmd, arg))

> Do you really need those wrapper?

No the wrappers aren't really needed. However they do make the
code a great deal more readable.

> They just obsfucate the code

The same argument could be made about *all* macros then.
Its simply a tradeoff between readability and potential
hiding.

>> +/**
>> + * struct evms_pool_mgmt - anchor block for private pool management
>> + * @cachep:         kmem_cache_t variable
>> + * @member_size:    size of each element in the pool
>> + * @head:
>> + * @waiters:        count of waiters
>> + * @wait_queue:     list of waiters
>> + * @name:           name of the pool (must be less than 20 chars)
>> + *
>> + * anchor block for private pool management
>> + **/
>> +struct evms_pool_mgmt {
>> +  kmem_cache_t *cachep;
>> +  int member_size;
>> +  void *head;
>> +  atomic_t waiters;
>> +  wait_queue_head_t wait_queue;
>> +  u8 *name;
>> +};

> What's the pruipose of this?  Is it really evms-specific?

Its a reminent of 2.4 stuff before mempool was available.
Its gone.

>> +
>> +/*
>> + * Notes:
>> + *      All of the following kernel thread functions belong to EVMS
base.
>> + *      These functions were copied from md_core.c
>> + */

> What about moving them to the core kernel code so everyone
> can use them?

I've got no problem with doing that.

>> +/* Have to include this at the end, since it depends
>> + * on structures and definitions in this file.
>> + */
>> +#include <linux/evms/evms_ioctl.h>

> Just remove this and make the individual sources include it

Sure, its easy to do. Having nested includes allowed us
to enforce include ordering.

Mark



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
@ 2002-10-03 17:42 Mark Peloquin
  2002-10-04 12:28 ` Robert Varga
  2002-10-04 14:08 ` Christoph Hellwig
  0 siblings, 2 replies; 17+ messages in thread
From: Mark Peloquin @ 2002-10-03 17:42 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: torvalds, linux-kernel, evms-devel


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



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2002-10-07 17:20 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-10-03 12:36 [PATCH] EVMS core 2/4: evms.h Kevin Corry
2002-10-03 14:50 ` Christoph Hellwig
2002-10-03 15:10   ` [Evms-devel] " Michael Clark
2002-10-03 15:14     ` Christoph Hellwig
2002-10-03 16:22       ` Linus Torvalds
2002-10-03 17:42 Mark Peloquin
2002-10-04 12:28 ` Robert Varga
2002-10-04 14:08 ` Christoph Hellwig
2002-10-04 14:36   ` Richard B. Johnson
2002-10-03 18:59 Mark Peloquin
2002-10-04 14:14 ` Christoph Hellwig
2002-10-04 14:34   ` Andreas Dilger
2002-10-04 17:10     ` Christoph Hellwig
2002-10-04 13:59 Mark Peloquin
2002-10-04 14:59 Mark Peloquin
2002-10-04 18:45 Steve Pratt
2002-10-07 17:25 ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).