Re: [RFC v2 05/10] vfs: introduce one hash table

From: Ram Pai
Date: Tue Sep 25 2012 - 05:54:05 EST


On Sun, Sep 23, 2012 at 08:56:30PM +0800, zwu.kernel@xxxxxxxxx wrote:
> From: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
>
> Adds a hash table structure which contains
> a lot of hash list and is used to efficiently
> look up the data temperature of a file or its
> ranges.
> In each hash list of hash table, the hash node
> will keep track of temperature info.
>
> Signed-off-by: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
> ---
> fs/hot_tracking.c | 77 ++++++++++++++++++++++++++++++++++++++++-
> include/linux/hot_tracking.h | 35 +++++++++++++++++++
> 2 files changed, 110 insertions(+), 2 deletions(-)
>
> diff --git a/fs/hot_tracking.c b/fs/hot_tracking.c
> index fa89f70..5f96442 100644
> --- a/fs/hot_tracking.c
> +++ b/fs/hot_tracking.c
> @@ -16,6 +16,7 @@
> #include <linux/module.h>
> #include <linux/spinlock.h>
> #include <linux/hardirq.h>
> +#include <linux/hash.h>
> #include <linux/fs.h>
> #include <linux/blkdev.h>
> #include <linux/types.h>
> @@ -24,6 +25,9 @@

...snip...

> +/* Hash list heads for hot hash table */
> +struct hot_hash_head {
> + struct hlist_head hashhead;
> + rwlock_t rwlock;
> + u32 temperature;
> +};
> +
> +/* Nodes stored in each hash list of hash table */
> +struct hot_hash_node {
> + struct hlist_node hashnode;
> + struct list_head node;
> + struct hot_freq_data *hot_freq_data;
> + struct hot_hash_head *hlist;
> + spinlock_t lock; /* protects hlist */
> +
> + /*
> + * number of references to this node
> + * equals 1 (hashlist entry)
> + */
> + struct kref refs;
> +};

Dont see why you need yet another datastructure to hash the inode_item
and the range_item into a hash list. You can just add another
hlist_node in the inode_item and range_item. This field can be then used
to link into the corresponding hash list.

You can use the container_of() get to the inode_item or the range_item
using the hlist_node field.

You can thus eliminate a lot of code.

> +
> /* An item representing an inode and its access frequency */
> struct hot_inode_item {
> /* node for hot_inode_tree rb_tree */
> @@ -68,6 +93,8 @@ struct hot_inode_item {
> spinlock_t lock;
> /* prevents kfree */
> struct kref refs;
> + /* hashlist node for this inode */
> + struct hot_hash_node *heat_node;

this can be just
struct hlist_node head_node; /* lookup hot_inode hash list */

Use this field to link it into the corresponding hashlist.

> };
>
this can be just
> /*
> @@ -91,6 +118,8 @@ struct hot_range_item {
> spinlock_t lock;
> /* prevents kfree */
> struct kref refs;
> + /* hashlist node for this range */
> + struct hot_hash_node *heat_node;

this can be just
struct hlist_node head_node; /* lookup hot_range hash list */


> };
>
> struct hot_info {
> @@ -98,6 +127,12 @@ struct hot_info {
>
> /* red-black tree that keeps track of fs-wide hot data */
> struct hot_inode_tree hot_inode_tree;
> +
> + /* hash map of inode temperature */
> + struct hot_hash_head heat_inode_hl[HEAT_HASH_SIZE];
> +
> + /* hash map of range temperature */
> + struct hot_hash_head heat_range_hl[HEAT_HASH_SIZE];
> };
>
> #endif /* _LINUX_HOTTRACK_H */

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/