Re: [PATCH v14 026/138] mm/writeback: Add folio_wait_stable()

[Mike Rapoport]

On Thu, Jul 15, 2021 at 04:35:12AM +0100, Matthew Wilcox (Oracle) wrote:
> Move wait_for_stable_page() into the folio compatibility file.
> folio_wait_stable() avoids a call to compound_head() and is 14 bytes
> smaller than wait_for_stable_page() was.  The net text size grows by 16
> bytes as a result of this patch.  We can also remove thp_head() as this
> was the last user.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Acked-by: Jeff Layton <jlayton@xxxxxxxxxx>
> Acked-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> Acked-by: Vlastimil Babka <vbabka@xxxxxxx>
> Reviewed-by: William Kucharski <william.kucharski@xxxxxxxxxx>
> Reviewed-by: David Howells <dhowells@xxxxxxxxxx>
> ---
>  include/linux/huge_mm.h | 15 ---------------
>  include/linux/pagemap.h |  1 +
>  mm/folio-compat.c       |  6 ++++++
>  mm/page-writeback.c     | 24 ++++++++++++++----------
>  4 files changed, 21 insertions(+), 25 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index f123e15d966e..f280f33ff223 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -250,15 +250,6 @@ static inline spinlock_t *pud_trans_huge_lock(pud_t *pud,
>  		return NULL;
>  }
>  
> -/**
> - * thp_head - Head page of a transparent huge page.
> - * @page: Any page (tail, head or regular) found in the page cache.

kerneldoc will warn about missing return description

> - */
> -static inline struct page *thp_head(struct page *page)
> -{
> -	return compound_head(page);
> -}
> -
>  /**
>   * thp_order - Order of a transparent huge page.
>   * @page: Head page of a transparent huge page.
> @@ -336,12 +327,6 @@ static inline struct list_head *page_deferred_list(struct page *page)
>  #define HPAGE_PUD_MASK ({ BUILD_BUG(); 0; })
>  #define HPAGE_PUD_SIZE ({ BUILD_BUG(); 0; })
>  
> -static inline struct page *thp_head(struct page *page)
> -{
> -	VM_BUG_ON_PGFLAGS(PageTail(page), page);
> -	return page;
> -}
> -
>  static inline unsigned int thp_order(struct page *page)
>  {
>  	VM_BUG_ON_PGFLAGS(PageTail(page), page);
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 0c5f53368fe9..96b62a2331fb 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -772,6 +772,7 @@ int folio_wait_writeback_killable(struct folio *folio);
>  void end_page_writeback(struct page *page);
>  void folio_end_writeback(struct folio *folio);
>  void wait_for_stable_page(struct page *page);
> +void folio_wait_stable(struct folio *folio);
>  
>  void __set_page_dirty(struct page *, struct address_space *, int warn);
>  int __set_page_dirty_nobuffers(struct page *page);
> diff --git a/mm/folio-compat.c b/mm/folio-compat.c
> index 41275dac7a92..3c83f03b80d7 100644
> --- a/mm/folio-compat.c
> +++ b/mm/folio-compat.c
> @@ -29,3 +29,9 @@ void wait_on_page_writeback(struct page *page)
>  	return folio_wait_writeback(page_folio(page));
>  }
>  EXPORT_SYMBOL_GPL(wait_on_page_writeback);
> +
> +void wait_for_stable_page(struct page *page)
> +{
> +	return folio_wait_stable(page_folio(page));
> +}
> +EXPORT_SYMBOL_GPL(wait_for_stable_page);
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index c2c00e1533ad..a078e9786cc4 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -2877,17 +2877,21 @@ int folio_wait_writeback_killable(struct folio *folio)
>  EXPORT_SYMBOL_GPL(folio_wait_writeback_killable);
>  
>  /**
> - * wait_for_stable_page() - wait for writeback to finish, if necessary.
> - * @page:	The page to wait on.
> + * folio_wait_stable() - wait for writeback to finish, if necessary.
> + * @folio: The folio to wait on.
>   *
> - * This function determines if the given page is related to a backing device
> - * that requires page contents to be held stable during writeback.  If so, then
> - * it will wait for any pending writeback to complete.
> + * This function determines if the given folio is related to a backing
> + * device that requires folio contents to be held stable during writeback.
> + * If so, then it will wait for any pending writeback to complete.
> + *
> + * Context: Sleeps.  Must be called in process context and with
> + * no spinlocks held.  Caller should hold a reference on the folio.
> + * If the folio is not locked, writeback may start again after writeback
> + * has finished.
>   */
> -void wait_for_stable_page(struct page *page)
> +void folio_wait_stable(struct folio *folio)
>  {
> -	page = thp_head(page);
> -	if (page->mapping->host->i_sb->s_iflags & SB_I_STABLE_WRITES)
> -		wait_on_page_writeback(page);
> +	if (folio->mapping->host->i_sb->s_iflags & SB_I_STABLE_WRITES)
> +		folio_wait_writeback(folio);
>  }
> -EXPORT_SYMBOL_GPL(wait_for_stable_page);
> +EXPORT_SYMBOL_GPL(folio_wait_stable);
> -- 
> 2.30.2
> 
> 

-- 
Sincerely yours,
Mike.