From: "Darrick J. Wong" Subject: Re: [RFC v4+ hot_track 10/19] vfs: introduce hot func register framework Date: Wed, 7 Nov 2012 10:58:02 -0800 Message-ID: <20121107185802.GA4038@blackbox.djwong.org> References: <1351485061-12297-1-git-send-email-zwu.kernel@gmail.com> <1351485061-12297-11-git-send-email-zwu.kernel@gmail.com> <20121106233011.GC4255@blackbox.djwong.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org, linuxram@linux.vnet.ibm.com, viro@zeniv.linux.org.uk, david@fromorbit.com, tytso@mit.edu, cmm@us.ibm.com, wuzhy@linux.vnet.ibm.com, wenqing.lz@taobao.com To: Zhi Yong Wu Return-path: Received: from aserp1040.oracle.com ([141.146.126.69]:44825 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751284Ab2KGS6z (ORCPT ); Wed, 7 Nov 2012 13:58:55 -0500 Content-Disposition: inline In-Reply-To: Sender: linux-ext4-owner@vger.kernel.org List-ID: On Wed, Nov 07, 2012 at 04:34:35PM +0800, Zhi Yong Wu wrote: > On Wed, Nov 7, 2012 at 7:30 AM, Darrick J. Wong wrote: > > On Mon, Oct 29, 2012 at 12:30:52PM +0800, zwu.kernel@gmail.com wrote: > >> From: Zhi Yong Wu > >> > >> Introduce one framwork to enable that specific FS > >> can register its own hot tracking functions. > >> > >> Signed-off-by: Zhi Yong Wu > >> --- > >> fs/hot_tracking.c | 78 ++++++++++++++++++++++++++++++++++++++---- > >> include/linux/hot_tracking.h | 25 +++++++++++++ > >> 2 files changed, 96 insertions(+), 7 deletions(-) > >> > >> diff --git a/fs/hot_tracking.c b/fs/hot_tracking.c > >> index 0ef9cad..c6c6138 100644 > >> --- a/fs/hot_tracking.c > >> +++ b/fs/hot_tracking.c > >> @@ -24,6 +24,9 @@ > >> #include > >> #include "hot_tracking.h" > >> > >> +static DEFINE_SPINLOCK(hot_func_list_lock); > >> +static LIST_HEAD(hot_func_list); > >> + > >> /* kmem_cache pointers for slab caches */ > >> static struct kmem_cache *hot_inode_item_cachep __read_mostly; > >> static struct kmem_cache *hot_range_item_cachep __read_mostly; > >> @@ -305,20 +308,23 @@ static u64 hot_average_update(struct timespec old_atime, > >> return new_avg; > >> } > >> > >> -static void hot_freq_data_update(struct hot_freq_data *freq_data, bool write) > >> +static void hot_freq_data_update(struct hot_info *root, > >> + struct hot_freq_data *freq_data, bool write) > >> { > >> struct timespec cur_time = current_kernel_time(); > >> > >> if (write) { > >> freq_data->nr_writes += 1; > >> - freq_data->avg_delta_writes = hot_average_update( > >> + freq_data->avg_delta_writes = > >> + root->hot_func_type->ops.hot_rw_freq_calc_fn( > >> freq_data->last_write_time, > >> cur_time, > >> freq_data->avg_delta_writes); > >> freq_data->last_write_time = cur_time; > >> } else { > >> freq_data->nr_reads += 1; > >> - freq_data->avg_delta_reads = hot_average_update( > >> + freq_data->avg_delta_reads = > >> + root->hot_func_type->ops.hot_rw_freq_calc_fn( > >> freq_data->last_read_time, > >> cur_time, > >> freq_data->avg_delta_reads); > >> @@ -430,7 +436,7 @@ static void hot_map_array_update(struct hot_freq_data *freq_data, > >> struct hot_comm_item *comm_item; > >> struct hot_inode_item *he; > >> struct hot_range_item *hr; > >> - u32 temp = hot_temp_calc(freq_data); > >> + u32 temp = root->hot_func_type->ops.hot_temp_calc_fn(freq_data); > >> u8 a_temp = temp >> (32 - HEAT_MAP_BITS); > >> u8 b_temp = freq_data->last_temp >> (32 - HEAT_MAP_BITS); > >> > >> @@ -511,7 +517,7 @@ static void hot_range_update(struct hot_inode_item *he, > >> &hr_nodes[i]->hot_range.hot_freq_data, root); > >> > >> spin_lock(&hr_nodes[i]->hot_range.lock); > >> - obsolete = hot_is_obsolete( > >> + obsolete = root->hot_func_type->ops.hot_is_obsolete_fn( > >> &hr_nodes[i]->hot_range.hot_freq_data); > >> spin_unlock(&hr_nodes[i]->hot_range.lock); > >> > >> @@ -668,7 +674,7 @@ void hot_update_freqs(struct inode *inode, u64 start, > >> } > >> > >> spin_lock(&he->hot_inode.lock); > >> - hot_freq_data_update(&he->hot_inode.hot_freq_data, rw); > >> + hot_freq_data_update(root, &he->hot_inode.hot_freq_data, rw); > >> spin_unlock(&he->hot_inode.lock); > >> > >> /* > >> @@ -685,7 +691,7 @@ void hot_update_freqs(struct inode *inode, u64 start, > >> } > >> > >> spin_lock(&hr->hot_range.lock); > >> - hot_freq_data_update(&hr->hot_range.hot_freq_data, rw); > >> + hot_freq_data_update(root, &hr->hot_range.hot_freq_data, rw); > >> spin_unlock(&hr->hot_range.lock); > >> > >> hot_range_item_put(hr); > >> @@ -695,6 +701,61 @@ void hot_update_freqs(struct inode *inode, u64 start, > >> } > >> EXPORT_SYMBOL_GPL(hot_update_freqs); > >> > >> +static struct hot_func_type hot_func_def = { > >> + .hot_func_name = "hot_type_def", > >> + .ops = { > >> + .hot_rw_freq_calc_fn = hot_average_update, > >> + .hot_temp_calc_fn = hot_temp_calc, > >> + .hot_is_obsolete_fn = hot_is_obsolete, > >> + }, > >> +}; > > > > If these hot_ops are per-filesystem, why not just embed a struct hot_func_ops > > inside of struct file_system_type? That eliminates this _get function, > this _get function is very small, only some loc, if hot_func_ops is > embedded in struct file_system_type, i am afraid to introduce some > regressions.... What kind of regressions are you afraid of, specifically? I don't think fstype is performance-critical enough to worry about wreaking havoc in the caches due to adding three function pointers. > > collision avoidance, etc. You can fill in NULL function pointers in > fill in NULL func pointer? why? > > hot_track_init (or just code around them). I guess you could just require that everyone fill out .hot_temp_calc_fn, even if they just point it to generic_hot_temp_calc. --D > > > > --D > > > >> + > >> +static struct hot_func_type *hot_func_get(const char *name) > >> +{ > >> + struct hot_func_type *f, *h = &hot_func_def; > >> + > >> + spin_lock(&hot_func_list_lock); > >> + list_for_each_entry(f, &hot_func_list, list) { > >> + if (!strcmp(f->hot_func_name, name)) > >> + h = f; > >> + } > >> + spin_unlock(&hot_func_list_lock); > >> + > >> + return h; > >> +} > >> + > >> +int hot_func_register(struct hot_func_type *h) > >> +{ > >> + struct hot_func_type *f, *t = NULL; > >> + > >> + /* register, don't allow duplicate names */ > >> + spin_lock(&hot_func_list_lock); > >> + list_for_each_entry(f, &hot_func_list, list) { > >> + if (!strcmp(f->hot_func_name, h->hot_func_name)) > >> + t = f; > >> + } > >> + > >> + if (t) { > >> + spin_unlock(&hot_func_list_lock); > >> + return -EBUSY; > >> + } > >> + > >> + list_add_tail(&h->list, &hot_func_list); > >> + spin_unlock(&hot_func_list_lock); > >> + > >> + return 0; > >> +} > >> +EXPORT_SYMBOL_GPL(hot_func_register); > >> + > >> +void hot_func_unregister(struct hot_func_type *h) > >> +{ > >> + /* unregister */ > >> + spin_lock(&hot_func_list_lock); > >> + list_del_init(&h->list); > >> + spin_unlock(&hot_func_list_lock); > >> +} > >> +EXPORT_SYMBOL_GPL(hot_func_unregister); > >> + > >> /* > >> * Initialize the data structures for hot data tracking. > >> */ > >> @@ -714,6 +775,9 @@ int hot_track_init(struct super_block *sb) > >> hot_inode_tree_init(root); > >> hot_map_array_init(root); > >> > >> + /* Get hot func type */ > >> + root->hot_func_type = hot_func_get(sb->s_type->name); > >> + > >> root->update_wq = alloc_workqueue( > >> "hot_update_wq", WQ_NON_REENTRANT, 0); > >> if (!root->update_wq) { > >> diff --git a/include/linux/hot_tracking.h b/include/linux/hot_tracking.h > >> index 2ee0d02..3941052 100644 > >> --- a/include/linux/hot_tracking.h > >> +++ b/include/linux/hot_tracking.h > >> @@ -23,6 +23,8 @@ > >> #define HEAT_MAP_BITS 8 > >> #define HEAT_MAP_SIZE (1 << HEAT_MAP_BITS) > >> > >> +#define HOT_NAME_MAX 16 > >> + > >> /* > >> * A frequency data struct holds values that are used to > >> * determine temperature of files and file ranges. These structs > >> @@ -73,6 +75,25 @@ struct hot_range_item { > >> u32 len; /* length in bytes */ > >> }; > >> > >> +typedef u64 (hot_rw_freq_calc_fn) (struct timespec old_atime, > >> + struct timespec cur_time, u64 old_avg); > >> +typedef u32 (hot_temp_calc_fn) (struct hot_freq_data *freq_data); > >> +typedef bool (hot_is_obsolete_fn) (struct hot_freq_data *freq_data); > >> + > >> +struct hot_func_ops { > >> + hot_rw_freq_calc_fn *hot_rw_freq_calc_fn; > >> + hot_temp_calc_fn *hot_temp_calc_fn; > >> + hot_is_obsolete_fn *hot_is_obsolete_fn; > >> +}; > >> + > >> +/* identifies an hot func type */ > >> +struct hot_func_type { > >> + char hot_func_name[HOT_NAME_MAX]; > >> + /* fields provided by specific FS */ > >> + struct hot_func_ops ops; > >> + struct list_head list; > >> +}; > >> + > >> struct hot_info { > >> struct radix_tree_root hot_inode_tree; > >> spinlock_t lock; /*protect inode tree */ > >> @@ -85,6 +106,7 @@ struct hot_info { > >> > >> struct workqueue_struct *update_wq; > >> struct delayed_work update_work; > >> + struct hot_func_type *hot_func_type; > >> }; > >> > >> extern void __init hot_cache_init(void); > >> @@ -93,4 +115,7 @@ extern void hot_track_exit(struct super_block *sb); > >> extern void hot_update_freqs(struct inode *inode, u64 start, > >> u64 len, int rw); > >> > >> +extern int hot_func_register(struct hot_func_type *h); > >> +extern void hot_func_unregister(struct hot_func_type *h); > >> + > >> #endif /* _LINUX_HOTTRACK_H */ > >> -- > >> 1.7.6.5 > >> > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > Regards, > > Zhi Yong Wu > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html