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

[Ben Hutchings <bhutchings@solarflare.com>]

On Mon, 2010-01-11 at 17:41 +0000, Ben Hutchings wrote:
> On Sun, 2010-01-10 at 12:34 +0200, Artem Bityutskiy wrote:
> > 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 agree in principle, but it already does know about block and char
> devices.
[...]

It looks like this is actually quite easy to do cleanly.  Based on the
original mtdblock code I thought that the cache structures (struct
mtdblk_dev) could be shared between multiple block translation devices
(struct mtd_blktrans_dev).  However, instances of each type have to be
uniquely identified by MTD index, so this cannot happen.  It should be
possible to embed struct mtd_blktrans_dev in struct mtdblk_dev and then
look up the latter with container_of().  Right?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.