[PATCH] gfs2: Remove use of error flag in journal reads

[PATCH] gfs2: Remove use of error flag in journal reads

[Matthew Wilcox (Oracle)]

Conventionally we use the uptodate bit to signal whether a read
encountered an error or not.  Use folio_end_read() to set the uptodate
bit on success.  Also use filemap_set_wb_err() to communicate the errno
instead of the more heavy-weight mapping_set_error().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/gfs2/lops.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 483f69807062..314ec2a70167 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -391,22 +391,15 @@ static void gfs2_log_write_page(struct gfs2_sbd *sdp, struct page *page)
  * Simply unlock the pages in the bio. The main thread will wait on them and
  * process them in order as necessary.
  */
-
 static void gfs2_end_log_read(struct bio *bio)
 {
-	struct page *page;
-	struct bio_vec *bvec;
-	struct bvec_iter_all iter_all;
+	int error = blk_status_to_errno(bio->bi_status);
+	struct folio_iter fi;
 
-	bio_for_each_segment_all(bvec, bio, iter_all) {
-		page = bvec->bv_page;
-		if (bio->bi_status) {
-			int err = blk_status_to_errno(bio->bi_status);
-
-			SetPageError(page);
-			mapping_set_error(page->mapping, err);
-		}
-		unlock_page(page);
+	bio_for_each_folio_all(fi, bio) {
+		/* We're abusing wb_err to get the error to gfs2_find_jhead */
+		filemap_set_wb_err(fi.folio->mapping, error);
+		folio_end_read(fi.folio, !error);
 	}
 
 	bio_put(bio);
@@ -475,7 +468,7 @@ static void gfs2_jhead_process_page(struct gfs2_jdesc *jd, unsigned long index,
 	folio = filemap_get_folio(jd->jd_inode->i_mapping, index);
 
 	folio_wait_locked(folio);
-	if (folio_test_error(folio))
+	if (!folio_test_uptodate(folio))
 		*done = true;
 
 	if (!*done)
-- 
2.42.0

Re: [PATCH] gfs2: Remove use of error flag in journal reads

[Matthew Wilcox]

On Wed, Dec 06, 2023 at 07:58:06PM +0000, Matthew Wilcox (Oracle) wrote:
> Conventionally we use the uptodate bit to signal whether a read
> encountered an error or not.  Use folio_end_read() to set the uptodate
> bit on success.  Also use filemap_set_wb_err() to communicate the errno
> instead of the more heavy-weight mapping_set_error().

Ping?

> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/gfs2/lops.c | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
> index 483f69807062..314ec2a70167 100644
> --- a/fs/gfs2/lops.c
> +++ b/fs/gfs2/lops.c
> @@ -391,22 +391,15 @@ static void gfs2_log_write_page(struct gfs2_sbd *sdp, struct page *page)
>   * Simply unlock the pages in the bio. The main thread will wait on them and
>   * process them in order as necessary.
>   */
> -
>  static void gfs2_end_log_read(struct bio *bio)
>  {
> -	struct page *page;
> -	struct bio_vec *bvec;
> -	struct bvec_iter_all iter_all;
> +	int error = blk_status_to_errno(bio->bi_status);
> +	struct folio_iter fi;
>  
> -	bio_for_each_segment_all(bvec, bio, iter_all) {
> -		page = bvec->bv_page;
> -		if (bio->bi_status) {
> -			int err = blk_status_to_errno(bio->bi_status);
> -
> -			SetPageError(page);
> -			mapping_set_error(page->mapping, err);
> -		}
> -		unlock_page(page);
> +	bio_for_each_folio_all(fi, bio) {
> +		/* We're abusing wb_err to get the error to gfs2_find_jhead */
> +		filemap_set_wb_err(fi.folio->mapping, error);
> +		folio_end_read(fi.folio, !error);
>  	}
>  
>  	bio_put(bio);
> @@ -475,7 +468,7 @@ static void gfs2_jhead_process_page(struct gfs2_jdesc *jd, unsigned long index,
>  	folio = filemap_get_folio(jd->jd_inode->i_mapping, index);
>  
>  	folio_wait_locked(folio);
> -	if (folio_test_error(folio))
> +	if (!folio_test_uptodate(folio))
>  		*done = true;
>  
>  	if (!*done)
> -- 
> 2.42.0
> 

Re: [PATCH] gfs2: Remove use of error flag in journal reads

[Andreas Gruenbacher]

On Wed, Dec 20, 2023 at 5:05 PM Matthew Wilcox <willy@infradead.org> wrote:
> On Wed, Dec 06, 2023 at 07:58:06PM +0000, Matthew Wilcox (Oracle) wrote:
> > Conventionally we use the uptodate bit to signal whether a read
> > encountered an error or not.  Use folio_end_read() to set the uptodate
> > bit on success.  Also use filemap_set_wb_err() to communicate the errno
> > instead of the more heavy-weight mapping_set_error().
>
> Ping?

Pushed to for-next now. Thanks for the patch and reminder.

Andreas