RE: [PATCH v5 03/10] fs/ntfs3: Add bitmap

[Konstantin Komarov]

From: Matthew Wilcox <willy@xxxxxxxxxxxxx>
Sent: Friday, September 18, 2020 7:55 PM
> Subject: Re: [PATCH v5 03/10] fs/ntfs3: Add bitmap
> 
> On Fri, Sep 18, 2020 at 04:35:11PM +0000, Konstantin Komarov wrote:
> > > That was only just renamed.  More concerningly, the documentation is
> > > quite unambiguous:
> > >
> > >  * This function is for filesystems to call when they want to start
> > >  * readahead beyond a file's stated i_size.  This is almost certainly
> > >  * not the function you want to call.  Use page_cache_async_readahead()
> > >  * or page_cache_sync_readahead() instead.
> >
> > Hi Matthew! it's not so clear for us by several reasons (please correct
> > if this is wrong):
> > page_cache_sync_readahead() seems applicable as a replacement, but
> > it doesn't seem to be reasonable as readahead in this case gives perf
> > improvement because of it's async nature. The 'async' function is incompatible
> > replacement based on the arguments list.
> 
> I think the naming has confused you (so I need to clarify the docs).
> The sync function is to be called when you need the page which is being
> read, and you might want to take the opportunity to read more pages.
> The async version is to be called when the page you need is in cache,
> but you've noticed that you're getting towards the end of the readahead
> window.  Neither version waits for I/O to complete; you have to wait for
> the page to become unlocked and then you can check PageUptodate.
> 
> Looking at what you're doing, you don't have a file_ra_state because
> you're just trying to readahead fs metadata, right?

Hi Matthew! Yes, correct.

> I think you want
> to call force_page_cache_readahead(mapping, NULL, start, nr_pages);
> The prototype for it is in mm/internal.h, but I think moving it to
> include/linux/pagemap.h is justifiable.

Seems like this. We decided to temporarily remove the ra usage iv V7, while it's
not clear which of the public options is "our" case. Is there anything we can
assist regarding the force_page_cache_readahead() move to
include/linux/pagemap.h?

Thanks.