Re: [RFC v2 01/10] vfs: introduce private rb structures

[Ram Pai <linuxram@us.ibm.com>]

On Sun, Sep 23, 2012 at 08:56:26PM +0800, zwu.kernel@gmail.com wrote:
> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> 
>   One root structure hot_info is defined, is hooked
> up in super_block, and will be used to hold rb trees
> root, hash list root and some other information, etc.
>   Adds hot_inode_tree struct to keep track of
> frequently accessed files, and be keyed by {inode, offset}.
> Trees contain hot_inode_items representing those files
> and ranges.
>   Having these trees means that vfs can quickly determine the
> temperature of some data by doing some calculations on the
> hot_freq_data struct that hangs off of the tree item.
>   Define two items hot_inode_item and hot_range_item,
> one of them represents one tracked file
> to keep track of its access frequency and the tree of
> ranges in this file, while the latter represents
> a file range of one inode.
>   Each of the two structures contains a hot_freq_data
> struct with its frequency of access metrics (number of
> {reads, writes}, last {read,write} time, frequency of
> {reads,writes}).
>   Also, each hot_inode_item contains one hot_range_tree
> struct which is keyed by {inode, offset, length}
> and used to keep track of all the ranges in this file.
> 
> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> ---
> +
..snip..

> +/* A tree that sits on the hot_info */
> +struct hot_inode_tree {
> +	struct rb_root map;
> +	rwlock_t lock;
> +};
> +
> +/* A tree of ranges for each inode in the hot_inode_tree */
> +struct hot_range_tree {
> +	struct rb_root map;
> +	rwlock_t lock;
> +};

Can as well have a generic datastructure called hot_tree instead
of having two different datastructure which basically are the same.

> +
> +/* A frequency data struct holds values that are used to
> + * determine temperature of files and file ranges. These structs
> + * are members of hot_inode_item and hot_range_item
> + */
> +struct hot_freq_data {
> +	struct timespec last_read_time;
> +	struct timespec last_write_time;
> +	u32 nr_reads;
> +	u32 nr_writes;
> +	u64 avg_delta_reads;
> +	u64 avg_delta_writes;
> +	u8 flags;
> +	u32 last_temperature;
> +};
> +
> +/* An item representing an inode and its access frequency */
> +struct hot_inode_item {
> +	/* node for hot_inode_tree rb_tree */
> +	struct rb_node rb_node;
> +	/* tree of ranges in this inode */
> +	struct hot_range_tree hot_range_tree;
> +	/* frequency data for this inode */
> +	struct hot_freq_data hot_freq_data;
> +	/* inode number, copied from inode */
> +	unsigned long i_ino;
> +	/* used to check for errors in ref counting */
> +	u8 in_tree;
> +	/* protects hot_freq_data, i_no, in_tree */
> +	spinlock_t lock;
> +	/* prevents kfree */
> +	struct kref refs;
> +};
> +
> +/*
> + * An item representing a range inside of an inode whose frequency
> + * is being tracked
> + */
> +struct hot_range_item {
> +	/* node for hot_range_tree rb_tree */
> +	struct rb_node rb_node;
> +	/* frequency data for this range */
> +	struct hot_freq_data hot_freq_data;
> +	/* the hot_inode_item associated with this hot_range_item */
> +	struct hot_inode_item *hot_inode;
> +	/* starting offset of this range */
> +	u64 start;
> +	/* length of this range */
> +	u64 len;
> +	/* used to check for errors in ref counting */
> +	u8 in_tree;
> +	/* protects hot_freq_data, start, len, and in_tree */
> +	spinlock_t lock;
> +	/* prevents kfree */
> +	struct kref refs;
> +};

might as well have just one generic datastructure called hot_item with 
all the common fields and then have 

	struct hot_inode_item  {
		struct hot_item hot_inode;
		struct hot_tree hot_range_tree;
		unsigned long i_ino;
	}

	and 

	struct hot_range_item {
		struct hot_item hot_range;
		u64 start;
		u64 len;	/* length of this range */
	}

This should help you eliminate some duplicate code as well.


RP