Re: [PATCH 5/6] mtdblock: Replace array of block cache info with pointers in struct mtd_info

[Artem Bityutskiy <dedekind1@gmail.com>]

On Tue, 2010-01-05 at 15:22 +0000, Ben Hutchings wrote:
>  	mutex_lock(&mtdblk->cache_mutex);
>  	write_cached_data(mtdblk);
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 0f32a9b..5c1b1ac 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -177,6 +177,10 @@ struct mtd_info {
>  	 */
>  	struct backing_dev_info *backing_dev_info;
>  
> +#if defined(CONFIG_MTD_BLOCK) || defined(CONFIG_MTD_BLOCK_MODULE)
> +	/* Block device state */
> +	struct mtdblk_dev *blk_dev;
> +#endif

I think injecting this to mtd_info is conceptually very wrong. It breaks
layering. Block devices sit on top of MTD devices, not vice-versa. Lower
layer (MTD) should not know anything about the higher layer (block).

I think you should a global list or rb-tree instead of the
'mtdblks[MAX_MTD_DEVICES]' array. But you should not inject block device
fields to 'struct mtd_info'.

Moreover, all this mtdblock stuff is legacy, and coupling it tighter to
'struct mtd_info' is not a good idea.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)