[Ocfs2-tools-devel] Re: [PATCH 1/2] The io_cache implementation

[tao.ma]

Joel Becker wrote:
> This is a simple caching implementation for iochannel in libocfs2.  When
> initialized, it uses an rbtree to look up existing blocks and an lru to
> cache unseen blocks.
>
> The cache must be explicitly initialized by a caller.  This allows some
> libocfs2 programs to ensure direct read and write.  Initialization
> merely takes the size of the cache in blocks or bytes.  io_set_blksize()
> must have already been called.
>
> The io_read_block() and io_write_block() calls now use the cache if it
> is available.  Writes are write-through - the cache will be updated
> *and* the new data will be synchronized to disk.
I found a little inconvenient in this design when I try to use your 
io_cache in extent map.
You define the io_cache in io_channel which is the member of 
ocfs2_filesys and created in ocfs2_open.
So it means that when I want to use cache, then all the IO to the file 
system will use cache including both metadata and normal data(Since 
io_channel is initialized by io_open, I bet there should only be one 
io_channel). This isn't suitable. A large file data read/write will soon 
use up all the cache blocks and no metadata will be reserved. And We 
both know that we initialize the implementation of caching because we 
want to cache extent block for extent map.
So my suggestion is: Can we change the design a little? Make io_channel 
the member of io_cache.
struct _io_channel {
        char *io_name;
        int io_blksize;
        int io_flags;
        int io_error;
        int io_fd;
};
struct io_cache {
        struct _io_channel io_channel;
        size_t ic_nr_blocks;
        struct list_head ic_lru;
        struct rb_root ic_lookup;

        /* Housekeeping */
        struct io_cache_block *ic_metadata_buffer;
        char *ic_data_buffer;
};

So now io_cache is a separate mechanism. If a user wants direct IO, ok, 
just use the normal io_channel. If cache IO is needed, just create a 
io_cache for its own use and use the cache* function accordingly. This 
also make different modules have their own cache blocks and they can 
alloc nums of blocks according to their own needs.
For extent map, I can create the io_cache and only cache the extent 
block I want to reuse in the future.