On Tue, Sep 25, 2012 at 5:54 PM, Ram Pai [off-list ref] wrote:

On Sun, Sep 23, 2012 at 08:56:30PM +0800, zwu.kernel@gmail.com wrote:

quoted

From: Zhi Yong Wu <redacted>

  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 <redacted>
---
 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...

quoted

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

If there's such one structure, we can rapidly know which hash bucket
one inode_item is currently linking to via its hlist field.
This is useful if one inode_item corresponding to one file need to
move from one bucket to another bucket when this file get cold or
hotter.
Does it make sense to you?

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.

As what i said above, i don't think it can eliminate a lo of code.

quoted

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

quoted

 };

this can be just

quoted

 /*

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

quoted

 };

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

-- 
Regards,

Zhi Yong Wu