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@xxxxxxxxxxxxx> 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.
