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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEH94LiWJQDmm=Sqn3yQ81KPrA_7qfGLZh+JHqOo_5T8unUywQ@mail.gmail.com>
Date:	Wed, 26 Sep 2012 12:08:51 +0800
From:	Zhi Yong Wu <zwu.kernel@...il.com>
To:	Ram Pai <linuxram@...ibm.com>
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-btrfs@...r.kernel.org, linux-ext4@...r.kernel.org,
	linuxram@...ux.vnet.ibm.com, viro@...iv.linux.org.uk,
	cmm@...ibm.com, tytso@....edu, marco.stornelli@...il.com,
	david@...morbit.com, stroetmann@...olinux.com, diegocg@...il.com,
	chris@...muel.org, Zhi Yong Wu <wuzhy@...ux.vnet.ibm.com>
Subject: Re: [RFC v2 05/10] vfs: introduce one hash table

On Tue, Sep 25, 2012 at 5:54 PM, Ram Pai <linuxram@...ibm.com> wrote:
> On Sun, Sep 23, 2012 at 08:56:30PM +0800, zwu.kernel@...il.com wrote:
>> From: Zhi Yong Wu <wuzhy@...ux.vnet.ibm.com>
>>
>>   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@...ux.vnet.ibm.com>
>> ---
>>  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
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.
>
>> +
>>  /* 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 */
>



-- 
Regards,

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