Bob and Steve, On Tue, Nov 3, 2015 at 8:02 PM, Steven Whitehouse <swhit...@redhat.com> wrote: >> Most of this looks good. However, two comments: >> >> 1. I don't like adding a new u16 to the gfs2_inode. I've been working to >> reduce the size of gfs2's inodes lately, so I'd rather see this >> implemented as a new GIF_RAHEAD (or similar) flag in gfs2_inode's >> i_flags.
that's how I had this implemented in the previous version, before Steve requested to have it changed. It seems pretty unlikely to me that we'll readahead more than one block here, so I one bit should actually be enough. >> 2. It seems to me like we should take advantage of function gfs2_meta_ra() >> which already submits one block and a variable number of additional >> blocks for read-ahead, then waits for the first block IO to complete. > > The meta_ra thing is a bit of a hack, and best avoided for this use. We want > to only send out a single I/O here rather than let the I/O stack do the > merging after the fact, Well, the first patch might as well use gfs2_meta_ra I guess. I'm unsure whether submitting a single I/O request is worth it given that the code for doing that is quite ugly. If it is, we could make the argument that it's also worth it for gfs2_meta_ra. The block size might make a big difference here as well. Thanks, Andreas
