Re: [PATCH v4 4/9] doc: Correct the description of ->release_folio

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Jul 10, 2023 at 02:02:48PM +0100, Matthew Wilcox (Oracle) wrote:
> The filesystem ->release_folio method is called under more circumstances
> now than when the documentation was written.  The second sentence
> describing the interpretation of the return value is the wrong polarity
> (false indicates failure, not success).  And the third sentence is also
> wrong (the kernel calls try_to_free_buffers() instead).
> 
> So replace the entire paragraph with a detailed description of what the
> state of the folio may be, the meaning of the gfp parameter, why the
> method is being called and what the filesystem is expected to do.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thank you!
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  Documentation/filesystems/locking.rst | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst
> index ed148919e11a..ca3d24becbf5 100644
> --- a/Documentation/filesystems/locking.rst
> +++ b/Documentation/filesystems/locking.rst
> @@ -374,10 +374,17 @@ invalidate_lock before invalidating page cache in truncate / hole punch
>  path (and thus calling into ->invalidate_folio) to block races between page
>  cache invalidation and page cache filling functions (fault, read, ...).
>  
> -->release_folio() is called when the kernel is about to try to drop the
> -buffers from the folio in preparation for freeing it.  It returns false to
> -indicate that the buffers are (or may be) freeable.  If ->release_folio is
> -NULL, the kernel assumes that the fs has no private interest in the buffers.
> +->release_folio() is called when the MM wants to make a change to the
> +folio that would invalidate the filesystem's private data.  For example,
> +it may be about to be removed from the address_space or split.  The folio
> +is locked and not under writeback.  It may be dirty.  The gfp parameter
> +is not usually used for allocation, but rather to indicate what the
> +filesystem may do to attempt to free the private data.  The filesystem may
> +return false to indicate that the folio's private data cannot be freed.
> +If it returns true, it should have already removed the private data from
> +the folio.  If a filesystem does not provide a ->release_folio method,
> +the pagecache will assume that private data is buffer_heads and call
> +try_to_free_buffers().
>  
>  ->free_folio() is called when the kernel has dropped the folio
>  from the page cache.
> -- 
> 2.39.2
>