1. Patchew
  2. linux
  3. [PATCH v2 0/6] ceph: CephFS writeback correctness and performance fixes
  4. View patch

[PATCH v2 4/6] ceph: Split out page-array discarding to a function

Sam Edwards posted 6 patches 1 month ago
There is a newer version of this series
[PATCH v2 4/6] ceph: Split out page-array discarding to a function
Posted by Sam Edwards 1 month ago 
Discarding a page array (i.e. after failure to submit it) is a little
complex:
- Every folio in the batch needs to be redirtied and unlocked.
- Some folios are bounce pages created for fscrypt; the underlying
  plaintext folios also need to be redirtied and unlocked.
- The array itself can come either from the mempool or general kalloc,
  so different free functions need to be used depending on which.

Although currently only ceph_submit_write() does this, this logic is
complex enough to warrant its own function. Move it to a new
ceph_discard_page_array() function that is called by ceph_submit_write()
instead.

Suggested-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
 fs/ceph/addr.c | 67 ++++++++++++++++++++++++++++----------------------
 1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 467aa7242b49..3becb13a09fe 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -1222,6 +1222,43 @@ void ceph_allocate_page_array(struct address_space *mapping,
 	ceph_wbc->len = 0;
 }
 
+static inline
+void ceph_discard_page_array(struct writeback_control *wbc,
+			     struct ceph_writeback_ctl *ceph_wbc)
+{
+	int i;
+	struct page *page;
+
+	for (i = 0; i < folio_batch_count(&ceph_wbc->fbatch); i++) {
+		struct folio *folio = ceph_wbc->fbatch.folios[i];
+
+		if (!folio)
+			continue;
+
+		page = &folio->page;
+		redirty_page_for_writepage(wbc, page);
+		unlock_page(page);
+	}
+
+	for (i = 0; i < ceph_wbc->locked_pages; i++) {
+		page = ceph_fscrypt_pagecache_page(ceph_wbc->pages[i]);
+
+		if (!page)
+			continue;
+
+		redirty_page_for_writepage(wbc, page);
+		unlock_page(page);
+	}
+
+	if (ceph_wbc->from_pool) {
+		mempool_free(ceph_wbc->pages, ceph_wb_pagevec_pool);
+		ceph_wbc->from_pool = false;
+	} else
+		kfree(ceph_wbc->pages);
+	ceph_wbc->pages = NULL;
+	ceph_wbc->locked_pages = 0;
+}
+
 static inline
 bool is_folio_index_contiguous(const struct ceph_writeback_ctl *ceph_wbc,
 			      const struct folio *folio)
@@ -1445,35 +1482,7 @@ int ceph_submit_write(struct address_space *mapping,
 	BUG_ON(len < ceph_fscrypt_page_offset(page) + thp_size(page) - offset);
 
 	if (!ceph_inc_osd_stopping_blocker(fsc->mdsc)) {
-		for (i = 0; i < folio_batch_count(&ceph_wbc->fbatch); i++) {
-			struct folio *folio = ceph_wbc->fbatch.folios[i];
-
-			if (!folio)
-				continue;
-
-			page = &folio->page;
-			redirty_page_for_writepage(wbc, page);
-			unlock_page(page);
-		}
-
-		for (i = 0; i < ceph_wbc->locked_pages; i++) {
-			page = ceph_fscrypt_pagecache_page(ceph_wbc->pages[i]);
-
-			if (!page)
-				continue;
-
-			redirty_page_for_writepage(wbc, page);
-			unlock_page(page);
-		}
-
-		if (ceph_wbc->from_pool) {
-			mempool_free(ceph_wbc->pages, ceph_wb_pagevec_pool);
-			ceph_wbc->from_pool = false;
-		} else
-			kfree(ceph_wbc->pages);
-		ceph_wbc->pages = NULL;
-		ceph_wbc->locked_pages = 0;
-
+		ceph_discard_page_array(wbc, ceph_wbc);
 		ceph_osdc_put_request(req);
 		return -EIO;
 	}
-- 
2.51.2
Re: [PATCH v2 4/6] ceph: Split out page-array discarding to a function
Posted by Viacheslav Dubeyko 4 weeks, 1 day ago 
On Wed, 2026-01-07 at 13:01 -0800, Sam Edwards wrote:
> Discarding a page array (i.e. after failure to submit it) is a little
> complex:
> - Every folio in the batch needs to be redirtied and unlocked.
> - Some folios are bounce pages created for fscrypt; the underlying
>   plaintext folios also need to be redirtied and unlocked.
> - The array itself can come either from the mempool or general kalloc,
>   so different free functions need to be used depending on which.
> 
> Although currently only ceph_submit_write() does this, this logic is
> complex enough to warrant its own function. Move it to a new
> ceph_discard_page_array() function that is called by ceph_submit_write()
> instead.
> 
> Suggested-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
> Signed-off-by: Sam Edwards <CFSworks@gmail.com>
> ---
>  fs/ceph/addr.c | 67 ++++++++++++++++++++++++++++----------------------
>  1 file changed, 38 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 467aa7242b49..3becb13a09fe 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -1222,6 +1222,43 @@ void ceph_allocate_page_array(struct address_space *mapping,
>  	ceph_wbc->len = 0;
>  }
>  
> +static inline
> +void ceph_discard_page_array(struct writeback_control *wbc,
> +			     struct ceph_writeback_ctl *ceph_wbc)
> +{
> +	int i;
> +	struct page *page;
> +
> +	for (i = 0; i < folio_batch_count(&ceph_wbc->fbatch); i++) {
> +		struct folio *folio = ceph_wbc->fbatch.folios[i];
> +
> +		if (!folio)
> +			continue;
> +
> +		page = &folio->page;
> +		redirty_page_for_writepage(wbc, page);
> +		unlock_page(page);
> +	}
> +
> +	for (i = 0; i < ceph_wbc->locked_pages; i++) {
> +		page = ceph_fscrypt_pagecache_page(ceph_wbc->pages[i]);
> +
> +		if (!page)
> +			continue;
> +
> +		redirty_page_for_writepage(wbc, page);
> +		unlock_page(page);
> +	}
> +
> +	if (ceph_wbc->from_pool) {
> +		mempool_free(ceph_wbc->pages, ceph_wb_pagevec_pool);
> +		ceph_wbc->from_pool = false;
> +	} else
> +		kfree(ceph_wbc->pages);
> +	ceph_wbc->pages = NULL;
> +	ceph_wbc->locked_pages = 0;
> +}
> +
>  static inline
>  bool is_folio_index_contiguous(const struct ceph_writeback_ctl *ceph_wbc,
>  			      const struct folio *folio)
> @@ -1445,35 +1482,7 @@ int ceph_submit_write(struct address_space *mapping,
>  	BUG_ON(len < ceph_fscrypt_page_offset(page) + thp_size(page) - offset);
>  
>  	if (!ceph_inc_osd_stopping_blocker(fsc->mdsc)) {
> -		for (i = 0; i < folio_batch_count(&ceph_wbc->fbatch); i++) {
> -			struct folio *folio = ceph_wbc->fbatch.folios[i];
> -
> -			if (!folio)
> -				continue;
> -
> -			page = &folio->page;
> -			redirty_page_for_writepage(wbc, page);
> -			unlock_page(page);
> -		}
> -
> -		for (i = 0; i < ceph_wbc->locked_pages; i++) {
> -			page = ceph_fscrypt_pagecache_page(ceph_wbc->pages[i]);
> -
> -			if (!page)
> -				continue;
> -
> -			redirty_page_for_writepage(wbc, page);
> -			unlock_page(page);
> -		}
> -
> -		if (ceph_wbc->from_pool) {
> -			mempool_free(ceph_wbc->pages, ceph_wb_pagevec_pool);
> -			ceph_wbc->from_pool = false;
> -		} else
> -			kfree(ceph_wbc->pages);
> -		ceph_wbc->pages = NULL;
> -		ceph_wbc->locked_pages = 0;
> -
> +		ceph_discard_page_array(wbc, ceph_wbc);
>  		ceph_osdc_put_request(req);
>  		return -EIO;
>  	}

This patch makes sense to me. Looks good.

Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>

Thanks,
Slava.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.