Re: [PATCH RESEND v1 02/16] vfs: add init and cleanup functions

[David Sterba]

On Thu, Dec 20, 2012 at 10:43:21PM +0800, zwu.kernel@xxxxxxxxx wrote:
> From: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
> --- a/fs/hot_tracking.c
> +++ b/fs/hot_tracking.c
> @@ -107,3 +189,38 @@ err:
>  	kmem_cache_destroy(hot_inode_item_cachep);
>  }
>  EXPORT_SYMBOL_GPL(hot_cache_init);
> +
> +/*
> + * Initialize the data structures for hot data tracking.
> + */
> +int hot_track_init(struct super_block *sb)
> +{
> +	struct hot_info *root;
> +	int ret = -ENOMEM;
> +
> +	root = kzalloc(sizeof(struct hot_info), GFP_NOFS);
> +	if (!root) {
> +		printk(KERN_ERR "%s: Failed to malloc memory for "
> +				"hot_info\n", __func__);
> +		return ret;
> +	}
> +
> +	hot_inode_tree_init(root);

This function is supposed to be called from the filesystem init, please
add a sanity check that would catch multiple initialization attempts.

> +
> +	sb->s_hot_root = root;
> +
> +	printk(KERN_INFO "VFS: Turning on hot data tracking\n");
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(hot_track_init);
> +
> +void hot_track_exit(struct super_block *sb)
> +{
> +	struct hot_info *root = sb->s_hot_root;

another sanity check to catch the opposite.

Why? The option is parsed and enabled from the filesystems, due to
unexpected bugs eg with remounting or incorrectly handled error paths,
vfs layer should IMHO rather warn than crash.

> +
> +	hot_inode_tree_exit(root);
> +	sb->s_hot_root = NULL;
> +	kfree(root);
> +}
> +EXPORT_SYMBOL_GPL(hot_track_exit);


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