Re: [PATCH v2 2/7] doc: Correct the description of ->release_folio

Darrick J. Wong
2023-06-04T17:55:53Z

On Fri, Jun 02, 2023 at 11:24:39PM +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>
> ---
> Documentation/filesystems/locking.rst | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst
> index aa1a233b0fa8..91dc9d5bc602 100644
> --- a/Documentation/filesystems/locking.rst
> +++ b/Documentation/filesystems/locking.rst
> @@ -374,10 +374,16 @@ 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 kernel will call try_to_free_buffers().

the MM? Since you changed that above... :)

With that nit fixed,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>
> ->free_folio() is called when the kernel has dropped the folio
> from the page cache.
> --
> 2.39.2
>