On Mon, Oct 29, 2012 at 12:30:52PM +0800, zwu.kernel@xxxxxxxxx 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
> + }
> +
> + 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
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
