Re: [RFC v4+ hot_track 10/19] vfs: introduce hot func register framework
From: David Sterba <hidden>
Date: 2012-11-06 23:14:47
Also in:
linux-btrfs, linux-fsdevel, lkml
On Mon, Oct 29, 2012 at 12:30:52PM +0800, zwu.kernel@gmail.com wrote:
+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;You probably want to break here
+ }
+ 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 duplicate names are not allowed, then a warning may make sense to let us know that something is wrong
quoted hunk ↗ jump to hunk
+ } + + 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);--- a/include/linux/hot_tracking.h +++ b/include/linux/hot_tracking.h@@ -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);
I'm thinking, whether these typedefs are useful, similar ops structures do not introduce them, also when you pick a struct member names exactly same as the typedefs:
+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;
+};My suggestion is to make the types explicit in the structure.
+/* identifies an hot func type */
+struct hot_func_type {
+ char hot_func_name[HOT_NAME_MAX];'name' would be sufficient IMHO
+ /* fields provided by specific FS */ + struct hot_func_ops ops; + struct list_head list; +};
david