Re: [PATCH v6 0/3] ubi: Add block interface

[Ezequiel Garcia <ezequiel.garcia@free-electrons.com>]

On Mon, Feb 17, 2014 at 12:53:40PM +0200, Artem Bityutskiy wrote:
> On Mon, 2014-02-17 at 07:48 -0300, Ezequiel Garcia wrote:
> > On Mon, Feb 17, 2014 at 12:19:52PM +0200, Artem Bityutskiy wrote:
> > > Thanks!
> > > 
> > > On Sun, 2014-02-16 at 17:03 -0300, Ezequiel Garcia wrote:
> > > > The main target of this patch is supporting lighter, read-only
> > > > filesystem,
> > > > by putting a squashfs on top of ubiblock. And since squashfs already
> > > > provides
> > > > a file cache (which is even configurable), a first uncached
> > > > implementation
> > > > it's good enough for now.
> > > 
> > > Doesn't Linux provide a fake inode for _every_ block device, and uses
> > > page-cache to cache pages?
> > > 
> > > There is even entire special-purpose file-system for managing these fake
> > > block device inodes, see fs/block_dev.c
> > > 
> > > static struct file_system_type bd_type = {
> > >         .name           = "bdev",
> > >         .mount          = bd_mount,
> > >         .kill_sb        = kill_anon_super,
> > > };
> > > 
> > > I am really not sure which other cache do we need. The only thing coming
> > > to my mind is about emulating a HW disk cache in software. But I would
> > > not go that far without a really good reason.
> > > 
> > 
> > If we ever re-add the write support, it would be natural to implement it
> > using a 1-LEB cache. Block sectors are much much smaller than LEB's
> > (around three orders of magnitude), so we would want to read an entire
> > LEB into a 1-LEB buffer, update the buffer, and then write it back.
> > 
> > In this scheme, it's almost straightforward to keep this buffer around
> > as a cache, and only write-back to disk when the new read/write requests
> > asks for a different LEB.
> > 
> > IIRC, this is the main reason why we had the 1-LEB cache.
> 
> May be. But you are submitting R/O driver. And yet talk about caches and
> write support in several places. Caches are only relevant to write
> support. Write support is absent. Let's focus on what is present.
> 
> Feel free to put all the thoughts about a possible write support
> implementation to a single big comment in the code. That's fine. I do
> not object. I just want to focus on what is present.
> 

Totally agreed. Those comments were just a product of my own inertia.

If at all possible, take a look at the website doc and I'll push a new
round soon.

Thanks for the review,
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com