Re: [RFC PATCH v2] implement orangefs_readahead

[Mike Marshall <hubcap@omnibond.com>]

This seems OK... ?

static void orangefs_readahead(struct readahead_control *rac)
{
loff_t offset;
struct iov_iter iter;
struct file *file = rac->file;
struct inode *inode = file->f_mapping->host;
struct xarray *i_pages;
struct page *page;
loff_t new_start = readahead_pos(rac);
int ret;
size_t new_len = 0;

loff_t bytes_remaining = inode->i_size - readahead_pos(rac);
loff_t pages_remaining = bytes_remaining / PAGE_SIZE;

if (pages_remaining >= 1024)
new_len = 4194304;
else if (pages_remaining > readahead_count(rac))
new_len = bytes_remaining;

if (new_len)
readahead_expand(rac, new_start, new_len);

offset = readahead_pos(rac);
i_pages = &file->f_mapping->i_pages;

iov_iter_xarray(&iter, READ, i_pages, offset, readahead_length(rac));

/* read in the pages. */
if ((ret = wait_for_direct_io(ORANGEFS_IO_READ, inode,
&offset, &iter, readahead_length(rac),
inode->i_size, NULL, NULL, file)) < 0)
gossip_debug(GOSSIP_FILE_DEBUG,
"%s: wait_for_direct_io failed. \n", __func__);
else
ret = 0;

/* clean up. */
while ((page = readahead_page(rac))) {
page_endio(page, false, ret);
put_page(page);
}
}

I need to go remember how to git send-email through the
kernel.org email server, I apologize for the way gmail
unformats my code, even in plain text mode...

-Mike

On Sat, Mar 27, 2021 at 11:56 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Sat, Mar 27, 2021 at 11:40:08AM -0400, Mike Marshall wrote:
> > int ret;
> >
> > loff_t new_start = readahead_index(rac) * PAGE_SIZE;
>
> That looks like readahead_pos() to me.
>
> > size_t new_len = 524288;
> > readahead_expand(rac, new_start, new_len);
> >
> > npages = readahead_count(rac);
> > offset = readahead_pos(rac);
> > i_pages = &file->f_mapping->i_pages;
> >
> > iov_iter_xarray(&iter, READ, i_pages, offset, npages * PAGE_SIZE);
>
> readahead_length()?
>
> > /* read in the pages. */
> > ret = wait_for_direct_io(ORANGEFS_IO_READ, inode, &offset, &iter,
> > npages * PAGE_SIZE, inode->i_size, NULL, NULL, file);
> >
> > /* clean up. */
> > while ((page = readahead_page(rac))) {
> > page_endio(page, false, 0);
> > put_page(page);
> > }
> > }
>
> What if wait_for_direct_io() returns an error?  Shouldn't you be calling
>
> page_endio(page, false, ret)
>
> ?
>
> > On Sat, Mar 27, 2021 at 9:57 AM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > On Sat, Mar 27, 2021 at 08:31:38AM +0000, David Howells wrote:
> > > > However, in Mike's orangefs_readahead_cleanup(), he could replace:
> > > >
> > > >       rcu_read_lock();
> > > >       xas_for_each(&xas, page, last) {
> > > >               page_endio(page, false, 0);
> > > >               put_page(page);
> > > >       }
> > > >       rcu_read_unlock();
> > > >
> > > > with:
> > > >
> > > >       while ((page = readahead_page(ractl))) {
> > > >               page_endio(page, false, 0);
> > > >               put_page(page);
> > > >       }
> > > >
> > > > maybe?
> > >
> > > I'd rather see that than open-coded use of the XArray.  It's mildly
> > > slower, but given that we're talking about doing I/O, probably not enough
> > > to care about.