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

[Matthew Wilcox]

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?  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.