[PATCH 00/30] Remove PG_error flag

[PATCH 00/30] Remove PG_error flag

[Matthew Wilcox (Oracle)]

We've been steadily reducing the number of places which rely on
PG_error.  There are only two left, so the first five patches remove
those dependencies.

Every patch after the jfs patch is independent, and can be taken by the
respective maintainer immediately.  They might depend on patches I sent
in the last week or two (eg jfs, ntfs3).

Obviously I've done no testing beyond compilation.  All patches can
be found on linux-fsdevel.  I've bcc'd this cover letter to all the
maintainers, and cc'd each patch to whoever's listed in the MAINTAINERS
file.

Matthew Wilcox (Oracle) (30):
  btrfs: Use a folio in wait_dev_supers()
  btrfs: Use a folio in write_dev_supers()
  btrfs: Use the folio iterator in btrfs_end_super_write()
  btrfs: Remove use of the folio error flag
  jfs: Remove use of folio error flag
  bcachefs: Remove calls to folio_set_error
  befs: Convert befs_symlink_read_folio() to use folio_end_read()
  coda: Convert coda_symlink_filler() to use folio_end_read()
  ext2: Remove call to folio_set_error()
  ext4: Remove calls to to set/clear the folio error flag
  fuse: Convert fuse_readpages_end() to use folio_end_read()
  hostfs: Convert hostfs_read_folio() to use a folio
  isofs: Remove calls to set/clear the error flag
  jffs2: Remove calls to set/clear the folio error flag
  nfs: Remove calls to folio_set_error
  nilfs2: Remove calls to folio_set_error() and folio_clear_error()
  ntfs3: Remove calls to set/clear the error flag
  orangefs: Remove calls to set/clear the error flag
  reiserfs: Remove call to folio_set_error()
  romfs: Convert romfs_read_folio() to use a folio
  smb: Remove calls to set folio error flag
  squashfs: Convert squashfs_symlink_read_folio to use folio APIs
  squashfs: Remove calls to set the folio error flag
  ufs: Remove call to set the folio error flag
  vboxsf: Convert vboxsf_read_folio() to use a folio
  mm/memory-failure: Stop setting the folio error flag
  iomap: Remove calls to set and clear folio error flag
  buffer: Remove calls to set and clear the folio error flag
  fs: Remove calls to set and clear the folio error flag
  mm: Remove PG_error

 Documentation/filesystems/vfs.rst      |  3 +-
 fs/bcachefs/fs-io-buffered.c           | 12 +---
 fs/befs/linuxvfs.c                     | 10 ++-
 fs/btrfs/disk-io.c                     | 84 +++++++++++---------------
 fs/btrfs/extent_io.c                   |  2 +-
 fs/btrfs/volumes.h                     |  5 ++
 fs/buffer.c                            |  7 +--
 fs/coda/symlink.c                      | 10 +--
 fs/ext2/dir.c                          |  1 -
 fs/ext4/move_extent.c                  |  4 +-
 fs/ext4/page-io.c                      |  3 -
 fs/ext4/readpage.c                     |  1 -
 fs/fuse/file.c                         | 10 +--
 fs/hostfs/hostfs_kern.c                | 23 ++-----
 fs/iomap/buffered-io.c                 |  8 ---
 fs/isofs/compress.c                    |  4 --
 fs/jffs2/file.c                        | 14 +----
 fs/jfs/jfs_metapage.c                  | 47 +++++++-------
 fs/mpage.c                             | 13 +---
 fs/nfs/read.c                          |  2 -
 fs/nfs/symlink.c                       | 12 +---
 fs/nfs/write.c                         |  1 -
 fs/nilfs2/dir.c                        |  1 -
 fs/nilfs2/segment.c                    |  8 +--
 fs/ntfs3/frecord.c                     |  4 --
 fs/orangefs/inode.c                    | 13 +---
 fs/orangefs/orangefs-bufmap.c          |  4 +-
 fs/proc/page.c                         |  1 -
 fs/reiserfs/inode.c                    |  1 -
 fs/romfs/super.c                       | 22 ++-----
 fs/smb/client/file.c                   |  2 -
 fs/squashfs/file.c                     |  6 +-
 fs/squashfs/file_direct.c              |  3 +-
 fs/squashfs/symlink.c                  | 35 +++++------
 fs/ufs/dir.c                           |  1 -
 fs/vboxsf/file.c                       | 18 ++----
 include/linux/page-flags.h             |  6 +-
 include/trace/events/mmflags.h         |  1 -
 include/uapi/linux/kernel-page-flags.h |  2 +-
 mm/filemap.c                           |  8 ---
 mm/memory-failure.c                    | 29 ---------
 mm/migrate.c                           |  2 -
 42 files changed, 129 insertions(+), 314 deletions(-)

-- 
2.43.0

[PATCH 01/30] btrfs: Use a folio in wait_dev_supers()

[Matthew Wilcox (Oracle)]

Removes a few calls to compound_head() and calls to obsolete APIs.

Cc: Chris Mason <clm@fb.com>
Cc: Josef Bacik <josef@toxicpanda.com>
Cc: David Sterba <dsterba@suse.com>
Cc: linux-btrfs@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/btrfs/disk-io.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 3d512b041977..32cf64ccd761 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3849,7 +3849,7 @@ static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
 		max_mirrors = BTRFS_SUPER_MIRROR_MAX;
 
 	for (i = 0; i < max_mirrors; i++) {
-		struct page *page;
+		struct folio *folio;
 
 		ret = btrfs_sb_log_location(device, i, READ, &bytenr);
 		if (ret == -ENOENT) {
@@ -3864,27 +3864,27 @@ static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
 		    device->commit_total_bytes)
 			break;
 
-		page = find_get_page(device->bdev->bd_mapping,
+		folio = filemap_get_folio(device->bdev->bd_mapping,
 				     bytenr >> PAGE_SHIFT);
-		if (!page) {
+		if (IS_ERR(folio)) {
 			errors++;
 			if (i == 0)
 				primary_failed = true;
 			continue;
 		}
-		/* Page is submitted locked and unlocked once the IO completes */
-		wait_on_page_locked(page);
-		if (PageError(page)) {
+		/* Folio is unlocked once the write completes */
+		folio_wait_locked(folio);
+		if (folio_test_error(folio)) {
 			errors++;
 			if (i == 0)
 				primary_failed = true;
 		}
 
 		/* Drop our reference */
-		put_page(page);
+		folio_put(folio);
 
 		/* Drop the reference from the writing run */
-		put_page(page);
+		folio_put(folio);
 	}
 
 	/* log error, force error return */
-- 
2.43.0

Re: [PATCH 01/30] btrfs: Use a folio in wait_dev_supers()

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 02/30] btrfs: Use a folio in write_dev_supers()

[Matthew Wilcox (Oracle)]

Remove some calls to obsolete APIs and some hidden calls to
compound_head().

Cc: Chris Mason <clm@fb.com>
Cc: Josef Bacik <josef@toxicpanda.com>
Cc: David Sterba <dsterba@suse.com>
Cc: linux-btrfs@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/btrfs/disk-io.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 32cf64ccd761..8fa7c526093c 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3760,9 +3760,10 @@ static int write_dev_supers(struct btrfs_device *device,
 	shash->tfm = fs_info->csum_shash;
 
 	for (i = 0; i < max_mirrors; i++) {
-		struct page *page;
+		struct folio *folio;
 		struct bio *bio;
 		struct btrfs_super_block *disk_super;
+		size_t offset;
 
 		bytenr_orig = btrfs_sb_offset(i);
 		ret = btrfs_sb_log_location(device, i, WRITE, &bytenr);
@@ -3785,9 +3786,9 @@ static int write_dev_supers(struct btrfs_device *device,
 				    BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE,
 				    sb->csum);
 
-		page = find_or_create_page(mapping, bytenr >> PAGE_SHIFT,
-					   GFP_NOFS);
-		if (!page) {
+		folio = __filemap_get_folio(mapping, bytenr >> PAGE_SHIFT,
+				FGP_LOCK | FGP_ACCESSED | FGP_CREAT, GFP_NOFS);
+		if (IS_ERR(folio)) {
 			btrfs_err(device->fs_info,
 			    "couldn't get super block page for bytenr %llu",
 			    bytenr);
@@ -3796,9 +3797,10 @@ static int write_dev_supers(struct btrfs_device *device,
 		}
 
 		/* Bump the refcount for wait_dev_supers() */
-		get_page(page);
+		folio_get(folio);
 
-		disk_super = page_address(page);
+		offset = offset_in_folio(folio, bytenr);
+		disk_super = folio_address(folio) + offset;
 		memcpy(disk_super, sb, BTRFS_SUPER_INFO_SIZE);
 
 		/*
@@ -3812,8 +3814,7 @@ static int write_dev_supers(struct btrfs_device *device,
 		bio->bi_iter.bi_sector = bytenr >> SECTOR_SHIFT;
 		bio->bi_private = device;
 		bio->bi_end_io = btrfs_end_super_write;
-		__bio_add_page(bio, page, BTRFS_SUPER_INFO_SIZE,
-			       offset_in_page(bytenr));
+		bio_add_folio_nofail(bio, folio, BTRFS_SUPER_INFO_SIZE, offset);
 
 		/*
 		 * We FUA only the first super block.  The others we allow to
-- 
2.43.0

Re: [PATCH 02/30] btrfs: Use a folio in write_dev_supers()

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 02/30] btrfs: Use a folio in write_dev_supers()

[David Sterba]

On Sat, Apr 20, 2024 at 03:49:57AM +0100, Matthew Wilcox (Oracle) wrote:
> @@ -3812,8 +3814,7 @@ static int write_dev_supers(struct btrfs_device *device,
>  		bio->bi_iter.bi_sector = bytenr >> SECTOR_SHIFT;
>  		bio->bi_private = device;
>  		bio->bi_end_io = btrfs_end_super_write;
> -		__bio_add_page(bio, page, BTRFS_SUPER_INFO_SIZE,
> -			       offset_in_page(bytenr));
> +		bio_add_folio_nofail(bio, folio, BTRFS_SUPER_INFO_SIZE, offset);

Compilation fails when btrfs is built as a module, bio_add_folio_nofail()
is not exported. I can keep __bio_add_page() and the conversion can be
done later.

Re: [PATCH 02/30] btrfs: Use a folio in write_dev_supers()

[Matthew Wilcox]

On Thu, Apr 25, 2024 at 04:44:03PM +0200, David Sterba wrote:
> On Sat, Apr 20, 2024 at 03:49:57AM +0100, Matthew Wilcox (Oracle) wrote:
> > @@ -3812,8 +3814,7 @@ static int write_dev_supers(struct btrfs_device *device,
> >  		bio->bi_iter.bi_sector = bytenr >> SECTOR_SHIFT;
> >  		bio->bi_private = device;
> >  		bio->bi_end_io = btrfs_end_super_write;
> > -		__bio_add_page(bio, page, BTRFS_SUPER_INFO_SIZE,
> > -			       offset_in_page(bytenr));
> > +		bio_add_folio_nofail(bio, folio, BTRFS_SUPER_INFO_SIZE, offset);
> 
> Compilation fails when btrfs is built as a module, bio_add_folio_nofail()
> is not exported. I can keep __bio_add_page() and the conversion can be
> done later.

I'd rather you added the obvious patch I just sent ...

(please don't get me stuck in the infinite loop of "you can't export a
symbol without any users" "you can't add a user until this is exported")

[PATCH 03/30] btrfs: Use the folio iterator in btrfs_end_super_write()

[Matthew Wilcox (Oracle)]

Iterate over folios instead of bvecs.  Switch the order of unlock and put
to be the usual order; we know this folio can't be put until it's been
waited for, but that's fragile.  Remove the calls to ClearPageUptodate /
SetPageUptodate -- if PAGE_SIZE is larger than BTRFS_SUPER_INFO_SIZE,
we'd be marking the entire folio uptodate without having actually
initialised all the bytes in the page.

Cc: Chris Mason <clm@fb.com>
Cc: Josef Bacik <josef@toxicpanda.com>
Cc: David Sterba <dsterba@suse.com>
Cc: linux-btrfs@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/btrfs/disk-io.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 8fa7c526093c..18c47bf3f383 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3627,28 +3627,21 @@ ALLOW_ERROR_INJECTION(open_ctree, ERRNO);
 static void btrfs_end_super_write(struct bio *bio)
 {
 	struct btrfs_device *device = bio->bi_private;
-	struct bio_vec *bvec;
-	struct bvec_iter_all iter_all;
-	struct page *page;
-
-	bio_for_each_segment_all(bvec, bio, iter_all) {
-		page = bvec->bv_page;
+	struct folio_iter fi;
 
+	bio_for_each_folio_all(fi, bio) {
 		if (bio->bi_status) {
 			btrfs_warn_rl_in_rcu(device->fs_info,
-				"lost page write due to IO error on %s (%d)",
+				"lost sb write due to IO error on %s (%d)",
 				btrfs_dev_name(device),
 				blk_status_to_errno(bio->bi_status));
-			ClearPageUptodate(page);
-			SetPageError(page);
+			folio_set_error(fi.folio);
 			btrfs_dev_stat_inc_and_print(device,
 						     BTRFS_DEV_STAT_WRITE_ERRS);
-		} else {
-			SetPageUptodate(page);
 		}
 
-		put_page(page);
-		unlock_page(page);
+		folio_unlock(fi.folio);
+		folio_put(fi.folio);
 	}
 
 	bio_put(bio);
-- 
2.43.0

Re: [PATCH 03/30] btrfs: Use the folio iterator in btrfs_end_super_write()

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 04/30] btrfs: Remove use of the folio error flag

[Matthew Wilcox (Oracle)]

Count the number of superblock writeback errors in the btrfs_device.
That means we don't need the folio to stay around until it's waited for,
and can avoid the extra call to folio_get/put.

Also remove a mention of PageError in a comment.

Cc: Chris Mason <clm@fb.com>
Cc: Josef Bacik <josef@toxicpanda.com>
Cc: David Sterba <dsterba@suse.com>
Cc: linux-btrfs@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/btrfs/disk-io.c   | 44 ++++++++++++++++++--------------------------
 fs/btrfs/extent_io.c |  2 +-
 fs/btrfs/volumes.h   |  5 +++++
 3 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 18c47bf3f383..3fa073f969d8 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3635,11 +3635,15 @@ static void btrfs_end_super_write(struct bio *bio)
 				"lost sb write due to IO error on %s (%d)",
 				btrfs_dev_name(device),
 				blk_status_to_errno(bio->bi_status));
-			folio_set_error(fi.folio);
 			btrfs_dev_stat_inc_and_print(device,
 						     BTRFS_DEV_STAT_WRITE_ERRS);
+			/* Ensure failure if a primary sb fails */
+			if (bio->bi_opf & REQ_FUA)
+				atomic_set(&device->sb_wb_errors,
+						BTRFS_DEV_PRIMARY_ERROR);
+			else
+				atomic_inc(&device->sb_wb_errors);
 		}
-
 		folio_unlock(fi.folio);
 		folio_put(fi.folio);
 	}
@@ -3743,10 +3747,11 @@ static int write_dev_supers(struct btrfs_device *device,
 	struct address_space *mapping = device->bdev->bd_mapping;
 	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
 	int i;
-	int errors = 0;
 	int ret;
 	u64 bytenr, bytenr_orig;
 
+	atomic_set(&device->sb_wb_errors, 0);
+
 	if (max_mirrors == 0)
 		max_mirrors = BTRFS_SUPER_MIRROR_MAX;
 
@@ -3766,7 +3771,7 @@ static int write_dev_supers(struct btrfs_device *device,
 			btrfs_err(device->fs_info,
 				"couldn't get super block location for mirror %d",
 				i);
-			errors++;
+			atomic_inc(&device->sb_wb_errors);
 			continue;
 		}
 		if (bytenr + BTRFS_SUPER_INFO_SIZE >=
@@ -3785,13 +3790,10 @@ static int write_dev_supers(struct btrfs_device *device,
 			btrfs_err(device->fs_info,
 			    "couldn't get super block page for bytenr %llu",
 			    bytenr);
-			errors++;
+			atomic_inc(&device->sb_wb_errors);
 			continue;
 		}
 
-		/* Bump the refcount for wait_dev_supers() */
-		folio_get(folio);
-
 		offset = offset_in_folio(folio, bytenr);
 		disk_super = folio_address(folio) + offset;
 		memcpy(disk_super, sb, BTRFS_SUPER_INFO_SIZE);
@@ -3819,9 +3821,9 @@ static int write_dev_supers(struct btrfs_device *device,
 		submit_bio(bio);
 
 		if (btrfs_advance_sb_log(device, i))
-			errors++;
+			atomic_inc(&device->sb_wb_errors);
 	}
-	return errors < i ? 0 : -1;
+	return atomic_read(&device->sb_wb_errors) < i ? 0 : -1;
 }
 
 /*
@@ -3860,27 +3862,17 @@ static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
 
 		folio = filemap_get_folio(device->bdev->bd_mapping,
 				     bytenr >> PAGE_SHIFT);
-		if (IS_ERR(folio)) {
-			errors++;
-			if (i == 0)
-				primary_failed = true;
+		/* If the folio has been removed, then we know it completed */
+		if (IS_ERR(folio))
 			continue;
-		}
-		/* Folio is unlocked once the write completes */
+		/* Folio is unlocked once the IO completes */
 		folio_wait_locked(folio);
-		if (folio_test_error(folio)) {
-			errors++;
-			if (i == 0)
-				primary_failed = true;
-		}
-
-		/* Drop our reference */
-		folio_put(folio);
-
-		/* Drop the reference from the writing run */
 		folio_put(folio);
 	}
 
+	errors += atomic_read(&device->sb_wb_errors);
+	if (errors >= BTRFS_DEV_PRIMARY_ERROR)
+		primary_failed = true;
 	/* log error, force error return */
 	if (primary_failed) {
 		btrfs_err(device->fs_info, "error writing primary super block to device %llu",
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 7b10f47d8f83..7a1bd23833e5 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1602,7 +1602,7 @@ static void set_btree_ioerr(struct extent_buffer *eb)
 	 * can be no longer dirty nor marked anymore for writeback (if a
 	 * subsequent modification to the extent buffer didn't happen before the
 	 * transaction commit), which makes filemap_fdata[write|wait]_range not
-	 * able to find the pages tagged with SetPageError at transaction
+	 * able to find the pages which contain errors at transaction
 	 * commit time. So if this happens we must abort the transaction,
 	 * otherwise we commit a super block with btree roots that point to
 	 * btree nodes/leafs whose content on disk is invalid - either garbage
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index cf555f5b47ce..cdab144410a8 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -92,6 +92,9 @@ enum btrfs_raid_types {
 #define BTRFS_DEV_STATE_FLUSH_SENT	(4)
 #define BTRFS_DEV_STATE_NO_READA	(5)
 
+/* We'll never have this many superblocks */
+#define BTRFS_DEV_PRIMARY_ERROR		(INT_MAX / 2)
+
 struct btrfs_fs_devices;
 
 struct btrfs_device {
@@ -142,6 +145,8 @@ struct btrfs_device {
 	/* type and info about this device */
 	u64 type;
 
+	atomic_t sb_wb_errors;
+
 	/* minimal io size for this device */
 	u32 sector_size;
 
-- 
2.43.0

[PATCH 05/30] jfs: Remove use of folio error flag

[Matthew Wilcox (Oracle)]

Store the blk_status per folio (if we can have multiple metapages per
folio) instead of setting the folio error flag.  This will allow us to
reclaim a precious folio flag shortly.

Cc: Dave Kleikamp <shaggy@kernel.org>
Cc: jfs-discussion@lists.sourceforge.net
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/jfs/jfs_metapage.c | 47 +++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/fs/jfs/jfs_metapage.c b/fs/jfs/jfs_metapage.c
index 19854bd8dfea..df575a873ec6 100644
--- a/fs/jfs/jfs_metapage.c
+++ b/fs/jfs/jfs_metapage.c
@@ -76,6 +76,7 @@ static mempool_t *metapage_mempool;
 struct meta_anchor {
 	int mp_count;
 	atomic_t io_count;
+	blk_status_t status;
 	struct metapage *mp[MPS_PER_PAGE];
 };
 
@@ -138,12 +139,16 @@ static inline void inc_io(struct folio *folio)
 	atomic_inc(&anchor->io_count);
 }
 
-static inline void dec_io(struct folio *folio, void (*handler) (struct folio *))
+static inline void dec_io(struct folio *folio, blk_status_t status,
+		void (*handler)(struct folio *, blk_status_t))
 {
 	struct meta_anchor *anchor = folio->private;
 
+	if (anchor->status == BLK_STS_OK)
+		anchor->status = status;
+
 	if (atomic_dec_and_test(&anchor->io_count))
-		handler(folio);
+		handler(folio, anchor->status);
 }
 
 #else
@@ -168,7 +173,7 @@ static inline void remove_metapage(struct folio *folio, struct metapage *mp)
 }
 
 #define inc_io(folio) do {} while(0)
-#define dec_io(folio, handler) handler(folio)
+#define dec_io(folio, status, handler) handler(folio, status)
 
 #endif
 
@@ -258,23 +263,20 @@ static sector_t metapage_get_blocks(struct inode *inode, sector_t lblock,
 	return lblock;
 }
 
-static void last_read_complete(struct folio *folio)
+static void last_read_complete(struct folio *folio, blk_status_t status)
 {
-	if (!folio_test_error(folio))
-		folio_mark_uptodate(folio);
-	folio_unlock(folio);
+	if (status)
+		printk(KERN_ERR "Read error %d at %#llx\n", status,
+				folio_pos(folio));
+
+	folio_end_read(folio, status == 0);
 }
 
 static void metapage_read_end_io(struct bio *bio)
 {
 	struct folio *folio = bio->bi_private;
 
-	if (bio->bi_status) {
-		printk(KERN_ERR "metapage_read_end_io: I/O error\n");
-		folio_set_error(folio);
-	}
-
-	dec_io(folio, last_read_complete);
+	dec_io(folio, bio->bi_status, last_read_complete);
 	bio_put(bio);
 }
 
@@ -300,11 +302,17 @@ static void remove_from_logsync(struct metapage *mp)
 	LOGSYNC_UNLOCK(log, flags);
 }
 
-static void last_write_complete(struct folio *folio)
+static void last_write_complete(struct folio *folio, blk_status_t status)
 {
 	struct metapage *mp;
 	unsigned int offset;
 
+	if (status) {
+		int err = blk_status_to_errno(status);
+		printk(KERN_ERR "metapage_write_end_io: I/O error\n");
+		mapping_set_error(folio->mapping, err);
+	}
+
 	for (offset = 0; offset < PAGE_SIZE; offset += PSIZE) {
 		mp = folio_to_mp(folio, offset);
 		if (mp && test_bit(META_io, &mp->flag)) {
@@ -326,12 +334,7 @@ static void metapage_write_end_io(struct bio *bio)
 
 	BUG_ON(!folio->private);
 
-	if (bio->bi_status) {
-		int err = blk_status_to_errno(bio->bi_status);
-		printk(KERN_ERR "metapage_write_end_io: I/O error\n");
-		mapping_set_error(folio->mapping, err);
-	}
-	dec_io(folio, last_write_complete);
+	dec_io(folio, bio->bi_status, last_write_complete);
 	bio_put(bio);
 }
 
@@ -454,10 +457,10 @@ static int metapage_write_folio(struct folio *folio,
 		       4, bio, sizeof(*bio), 0);
 	bio_put(bio);
 	folio_unlock(folio);
-	dec_io(folio, last_write_complete);
+	dec_io(folio, BLK_STS_OK, last_write_complete);
 err_out:
 	while (bad_blocks--)
-		dec_io(folio, last_write_complete);
+		dec_io(folio, BLK_STS_OK, last_write_complete);
 	return -EIO;
 }
 
-- 
2.43.0

[PATCH 06/30] bcachefs: Remove calls to folio_set_error

[Matthew Wilcox (Oracle)]

Common code doesn't test the error flag, so we don't need to set it in
bcachefs.  We can use folio_end_read() to combine the setting (or not)
of the uptodate flag and clearing the lock flag.

Cc: Kent Overstreet <kent.overstreet@linux.dev>
Cc: Brian Foster <bfoster@redhat.com>
Cc: linux-bcachefs@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/bcachefs/fs-io-buffered.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/fs/bcachefs/fs-io-buffered.c b/fs/bcachefs/fs-io-buffered.c
index 39292e7ef342..4bff641b8be6 100644
--- a/fs/bcachefs/fs-io-buffered.c
+++ b/fs/bcachefs/fs-io-buffered.c
@@ -30,15 +30,8 @@ static void bch2_readpages_end_io(struct bio *bio)
 {
 	struct folio_iter fi;
 
-	bio_for_each_folio_all(fi, bio) {
-		if (!bio->bi_status) {
-			folio_mark_uptodate(fi.folio);
-		} else {
-			folio_clear_uptodate(fi.folio);
-			folio_set_error(fi.folio);
-		}
-		folio_unlock(fi.folio);
-	}
+	bio_for_each_folio_all(fi, bio)
+		folio_end_read(fi.folio, bio->bi_status == BLK_STS_OK);
 
 	bio_put(bio);
 }
@@ -408,7 +401,6 @@ static void bch2_writepage_io_done(struct bch_write_op *op)
 		bio_for_each_folio_all(fi, bio) {
 			struct bch_folio *s;
 
-			folio_set_error(fi.folio);
 			mapping_set_error(fi.folio->mapping, -EIO);
 
 			s = __bch2_folio(fi.folio);
-- 
2.43.0

Re: [PATCH 06/30] bcachefs: Remove calls to folio_set_error

[Kent Overstreet]

On Sat, Apr 20, 2024 at 03:50:01AM +0100, Matthew Wilcox (Oracle) wrote:
> Common code doesn't test the error flag, so we don't need to set it in
> bcachefs.  We can use folio_end_read() to combine the setting (or not)
> of the uptodate flag and clearing the lock flag.
> 
> Cc: Kent Overstreet <kent.overstreet@linux.dev>
> Cc: Brian Foster <bfoster@redhat.com>
> Cc: linux-bcachefs@vger.kernel.org
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

applied

[PATCH 07/30] befs: Convert befs_symlink_read_folio() to use folio_end_read()

[Matthew Wilcox (Oracle)]

This is slightly more efficient than separate calls to
folio_mark_uptodate() and folio_unlock(), and it's easier to read.
Get rid of the call to folio_set_error() as nobody will check this flag.

Cc: Luis de Bethencourt <luisbg@kernel.org>
Cc: Salah Triki <salah.triki@gmail.com>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/befs/linuxvfs.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
index d76f406d3b2e..f92f108840f5 100644
--- a/fs/befs/linuxvfs.c
+++ b/fs/befs/linuxvfs.c
@@ -475,6 +475,7 @@ static int befs_symlink_read_folio(struct file *unused, struct folio *folio)
 	befs_data_stream *data = &befs_ino->i_data.ds;
 	befs_off_t len = data->size;
 	char *link = folio_address(folio);
+	int err = -EIO;
 
 	if (len == 0 || len > PAGE_SIZE) {
 		befs_error(sb, "Long symlink with illegal length");
@@ -487,13 +488,10 @@ static int befs_symlink_read_folio(struct file *unused, struct folio *folio)
 		goto fail;
 	}
 	link[len - 1] = '\0';
-	folio_mark_uptodate(folio);
-	folio_unlock(folio);
-	return 0;
+	err = 0;
 fail:
-	folio_set_error(folio);
-	folio_unlock(folio);
-	return -EIO;
+	folio_end_read(folio, err == 0);
+	return err;
 }
 
 /*
-- 
2.43.0

[PATCH 08/30] coda: Convert coda_symlink_filler() to use folio_end_read()

[Matthew Wilcox (Oracle)]

This is slightly more efficient than separate calls to
folio_mark_uptodate() and folio_unlock(), and it's easier to read.
Get rid of the call to folio_set_error() as nobody will check this flag.

Cc: Jan Harkes <jaharkes@cs.cmu.edu>
Cc: coda@cs.cmu.edu
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/coda/symlink.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/fs/coda/symlink.c b/fs/coda/symlink.c
index ccdbec388091..40f84d014524 100644
--- a/fs/coda/symlink.c
+++ b/fs/coda/symlink.c
@@ -31,15 +31,7 @@ static int coda_symlink_filler(struct file *file, struct folio *folio)
 	cii = ITOC(inode);
 
 	error = venus_readlink(inode->i_sb, &cii->c_fid, p, &len);
-	if (error)
-		goto fail;
-	folio_mark_uptodate(folio);
-	folio_unlock(folio);
-	return 0;
-
-fail:
-	folio_set_error(folio);
-	folio_unlock(folio);
+	folio_end_read(folio, error == 0);
 	return error;
 }
 
-- 
2.43.0

[PATCH 09/30] ext2: Remove call to folio_set_error()

[Matthew Wilcox (Oracle)]

Nobody checks this flag on ext2 folios, stop setting it.

Cc: Jan Kara <jack@suse.com>
Cc: linux-ext4@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ext2/dir.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index 4fb155b5a958..087457061c6e 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -175,7 +175,6 @@ static bool ext2_check_folio(struct folio *folio, int quiet, char *kaddr)
 			(unsigned long) le32_to_cpu(p->inode));
 	}
 fail:
-	folio_set_error(folio);
 	return false;
 }
 
-- 
2.43.0

[PATCH 10/30] ext4: Remove calls to to set/clear the folio error flag

[Matthew Wilcox (Oracle)]

Nobody checks this flag on ext4 folios, stop setting and clearing it.

Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: linux-ext4@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ext4/move_extent.c | 4 +---
 fs/ext4/page-io.c     | 3 ---
 fs/ext4/readpage.c    | 1 -
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 7cd4afa4de1d..204f53b23622 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -199,10 +199,8 @@ mext_page_mkuptodate(struct folio *folio, unsigned from, unsigned to)
 			continue;
 		if (!buffer_mapped(bh)) {
 			err = ext4_get_block(inode, block, bh, 0);
-			if (err) {
-				folio_set_error(folio);
+			if (err)
 				return err;
-			}
 			if (!buffer_mapped(bh)) {
 				folio_zero_range(folio, block_start, blocksize);
 				set_buffer_uptodate(bh);
diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 312bc6813357..ad5543866d21 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -117,7 +117,6 @@ static void ext4_finish_bio(struct bio *bio)
 
 		if (bio->bi_status) {
 			int err = blk_status_to_errno(bio->bi_status);
-			folio_set_error(folio);
 			mapping_set_error(folio->mapping, err);
 		}
 		bh = head = folio_buffers(folio);
@@ -441,8 +440,6 @@ int ext4_bio_write_folio(struct ext4_io_submit *io, struct folio *folio,
 	BUG_ON(!folio_test_locked(folio));
 	BUG_ON(folio_test_writeback(folio));
 
-	folio_clear_error(folio);
-
 	/*
 	 * Comments copied from block_write_full_folio:
 	 *
diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
index 21e8f0aebb3c..8494492582ab 100644
--- a/fs/ext4/readpage.c
+++ b/fs/ext4/readpage.c
@@ -289,7 +289,6 @@ int ext4_mpage_readpages(struct inode *inode,
 
 				if (ext4_map_blocks(NULL, inode, &map, 0) < 0) {
 				set_error_page:
-					folio_set_error(folio);
 					folio_zero_segment(folio, 0,
 							  folio_size(folio));
 					folio_unlock(folio);
-- 
2.43.0

[PATCH 11/30] fuse: Convert fuse_readpages_end() to use folio_end_read()

[Matthew Wilcox (Oracle)]

Nobody checks the error flag on fuse folios, so stop setting it.
Optimise the (optional) setting of the uptodate flag and clearing
of the lock flag by using folio_end_read().

Cc: Miklos Szeredi <miklos@szeredi.hu>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/fuse/file.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index b57ce4157640..f39456c65ed7 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -935,14 +935,10 @@ static void fuse_readpages_end(struct fuse_mount *fm, struct fuse_args *args,
 	}
 
 	for (i = 0; i < ap->num_pages; i++) {
-		struct page *page = ap->pages[i];
+		struct folio *folio = page_folio(ap->pages[i]);
 
-		if (!err)
-			SetPageUptodate(page);
-		else
-			SetPageError(page);
-		unlock_page(page);
-		put_page(page);
+		folio_end_read(folio, !err);
+		folio_put(folio);
 	}
 	if (ia->ff)
 		fuse_file_put(ia->ff, false);
-- 
2.43.0

Re: [PATCH 11/30] fuse: Convert fuse_readpages_end() to use folio_end_read()

[Miklos Szeredi]

On Sat, 20 Apr 2024 at 04:50, Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
>
> Nobody checks the error flag on fuse folios, so stop setting it.
> Optimise the (optional) setting of the uptodate flag and clearing
> of the lock flag by using folio_end_read().
>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Applied, thanks.

Miklos

[PATCH 12/30] hostfs: Convert hostfs_read_folio() to use a folio

[Matthew Wilcox (Oracle)]

Remove the use of page APIs, including setting/clearing the error flag
which is never checked on hostfs folios.  This does not include support
for large folios as kmap_local_folio() maps only one page at a time.

Cc: Richard Weinberger <richard@nod.at>
Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: linux-um@lists.infradead.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/hostfs/hostfs_kern.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
index a73d27c4dd58..e7c72f2634f6 100644
--- a/fs/hostfs/hostfs_kern.c
+++ b/fs/hostfs/hostfs_kern.c
@@ -432,31 +432,20 @@ static int hostfs_writepage(struct page *page, struct writeback_control *wbc)
 
 static int hostfs_read_folio(struct file *file, struct folio *folio)
 {
-	struct page *page = &folio->page;
 	char *buffer;
-	loff_t start = page_offset(page);
+	loff_t start = folio_pos(folio);
 	int bytes_read, ret = 0;
 
-	buffer = kmap_local_page(page);
+	buffer = kmap_local_folio(folio, 0);
 	bytes_read = read_file(FILE_HOSTFS_I(file)->fd, &start, buffer,
 			PAGE_SIZE);
-	if (bytes_read < 0) {
-		ClearPageUptodate(page);
-		SetPageError(page);
+	if (bytes_read < 0)
 		ret = bytes_read;
-		goto out;
-	}
-
-	memset(buffer + bytes_read, 0, PAGE_SIZE - bytes_read);
-
-	ClearPageError(page);
-	SetPageUptodate(page);
-
- out:
-	flush_dcache_page(page);
+	else
+		buffer = folio_zero_tail(folio, bytes_read, buffer);
 	kunmap_local(buffer);
-	unlock_page(page);
 
+	folio_end_read(folio, ret == 0);
 	return ret;
 }
 
-- 
2.43.0

[PATCH 13/30] isofs: Remove calls to set/clear the error flag

[Matthew Wilcox (Oracle)]

Nobody checks the error flag on isofs folios, so stop setting and
clearing it.

Cc: Jan Kara <jack@suse.cz>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/isofs/compress.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/isofs/compress.c b/fs/isofs/compress.c
index c4da3f634b92..34d5baa5d88a 100644
--- a/fs/isofs/compress.c
+++ b/fs/isofs/compress.c
@@ -346,8 +346,6 @@ static int zisofs_read_folio(struct file *file, struct folio *folio)
 	for (i = 0; i < pcount; i++, index++) {
 		if (i != full_page)
 			pages[i] = grab_cache_page_nowait(mapping, index);
-		if (pages[i])
-			ClearPageError(pages[i]);
 	}
 
 	err = zisofs_fill_pages(inode, full_page, pcount, pages);
@@ -356,8 +354,6 @@ static int zisofs_read_folio(struct file *file, struct folio *folio)
 	for (i = 0; i < pcount; i++) {
 		if (pages[i]) {
 			flush_dcache_page(pages[i]);
-			if (i == full_page && err)
-				SetPageError(pages[i]);
 			unlock_page(pages[i]);
 			if (i != full_page)
 				put_page(pages[i]);
-- 
2.43.0

Re: [PATCH 13/30] isofs: Remove calls to set/clear the error flag

[Jan Kara]

On Sat 20-04-24 03:50:08, Matthew Wilcox (Oracle) wrote:
> Nobody checks the error flag on isofs folios, so stop setting and
> clearing it.
> 
> Cc: Jan Kara <jack@suse.cz>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Do you plan to merge this together or should I pick this up myself?

Feel free to add:

Acked-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/isofs/compress.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/fs/isofs/compress.c b/fs/isofs/compress.c
> index c4da3f634b92..34d5baa5d88a 100644
> --- a/fs/isofs/compress.c
> +++ b/fs/isofs/compress.c
> @@ -346,8 +346,6 @@ static int zisofs_read_folio(struct file *file, struct folio *folio)
>  	for (i = 0; i < pcount; i++, index++) {
>  		if (i != full_page)
>  			pages[i] = grab_cache_page_nowait(mapping, index);
> -		if (pages[i])
> -			ClearPageError(pages[i]);
>  	}
>  
>  	err = zisofs_fill_pages(inode, full_page, pcount, pages);
> @@ -356,8 +354,6 @@ static int zisofs_read_folio(struct file *file, struct folio *folio)
>  	for (i = 0; i < pcount; i++) {
>  		if (pages[i]) {
>  			flush_dcache_page(pages[i]);
> -			if (i == full_page && err)
> -				SetPageError(pages[i]);
>  			unlock_page(pages[i]);
>  			if (i != full_page)
>  				put_page(pages[i]);
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 13/30] isofs: Remove calls to set/clear the error flag

[Matthew Wilcox]

On Mon, Apr 22, 2024 at 11:57:53PM +0200, Jan Kara wrote:
> On Sat 20-04-24 03:50:08, Matthew Wilcox (Oracle) wrote:
> > Nobody checks the error flag on isofs folios, so stop setting and
> > clearing it.
> > 
> > Cc: Jan Kara <jack@suse.cz>
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> 
> Do you plan to merge this together or should I pick this up myself?

Please take it through your tree; I'll prepare a pull request for the
remainder, but having more patches go through fs maintainers means
better testing.

Re: [PATCH 13/30] isofs: Remove calls to set/clear the error flag

[Jan Kara]

On Tue 23-04-24 18:51:46, Matthew Wilcox wrote:
> On Mon, Apr 22, 2024 at 11:57:53PM +0200, Jan Kara wrote:
> > On Sat 20-04-24 03:50:08, Matthew Wilcox (Oracle) wrote:
> > > Nobody checks the error flag on isofs folios, so stop setting and
> > > clearing it.
> > > 
> > > Cc: Jan Kara <jack@suse.cz>
> > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> > 
> > Do you plan to merge this together or should I pick this up myself?
> 
> Please take it through your tree; I'll prepare a pull request for the
> remainder, but having more patches go through fs maintainers means
> better testing.

OK, picked up ext2 and isofs patches into my tree.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 14/30] jffs2: Remove calls to set/clear the folio error flag

[Matthew Wilcox (Oracle)]

Nobody checks the error flag on jffs2 folios, so stop setting and
clearing it.  We can also remove the call to clear the uptodate
flag; it will already be clear.

Convert one of these into a call to mapping_set_error() which will
actually be checked by other parts of the kernel.

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Richard Weinberger <richard@nod.at>
Cc: linux-mtd@lists.infradead.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/jffs2/file.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
index 62ea76da7fdf..e12cb145147e 100644
--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c
@@ -95,13 +95,8 @@ static int jffs2_do_readpage_nolock (struct inode *inode, struct page *pg)
 	ret = jffs2_read_inode_range(c, f, pg_buf, pg->index << PAGE_SHIFT,
 				     PAGE_SIZE);
 
-	if (ret) {
-		ClearPageUptodate(pg);
-		SetPageError(pg);
-	} else {
+	if (!ret)
 		SetPageUptodate(pg);
-		ClearPageError(pg);
-	}
 
 	flush_dcache_page(pg);
 	kunmap(pg);
@@ -304,10 +299,8 @@ static int jffs2_write_end(struct file *filp, struct address_space *mapping,
 
 	kunmap(pg);
 
-	if (ret) {
-		/* There was an error writing. */
-		SetPageError(pg);
-	}
+	if (ret)
+		mapping_set_error(mapping, ret);
 
 	/* Adjust writtenlen for the padding we did, so we don't confuse our caller */
 	writtenlen -= min(writtenlen, (start - aligned_start));
@@ -330,7 +323,6 @@ static int jffs2_write_end(struct file *filp, struct address_space *mapping,
 		   it gets reread */
 		jffs2_dbg(1, "%s(): Not all bytes written. Marking page !uptodate\n",
 			__func__);
-		SetPageError(pg);
 		ClearPageUptodate(pg);
 	}
 
-- 
2.43.0

Re: [PATCH 14/30] jffs2: Remove calls to set/clear the folio error flag

[Zhihao Cheng]

在 2024/4/20 10:50, Matthew Wilcox (Oracle) 写道:
> Nobody checks the error flag on jffs2 folios, so stop setting and
> clearing it.  We can also remove the call to clear the uptodate
> flag; it will already be clear.
> 
> Convert one of these into a call to mapping_set_error() which will
> actually be checked by other parts of the kernel.
> 
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: linux-mtd@lists.infradead.org
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/jffs2/file.c | 14 +++-----------
>   1 file changed, 3 insertions(+), 11 deletions(-)

xfstests passed. Looks like the change is harmless.

Tested-by: Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> 
> diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
> index 62ea76da7fdf..e12cb145147e 100644
> --- a/fs/jffs2/file.c
> +++ b/fs/jffs2/file.c
> @@ -95,13 +95,8 @@ static int jffs2_do_readpage_nolock (struct inode *inode, struct page *pg)
>   	ret = jffs2_read_inode_range(c, f, pg_buf, pg->index << PAGE_SHIFT,
>   				     PAGE_SIZE);
>   
> -	if (ret) {
> -		ClearPageUptodate(pg);
> -		SetPageError(pg);
> -	} else {
> +	if (!ret)
>   		SetPageUptodate(pg);
> -		ClearPageError(pg);
> -	}
>   
>   	flush_dcache_page(pg);
>   	kunmap(pg);
> @@ -304,10 +299,8 @@ static int jffs2_write_end(struct file *filp, struct address_space *mapping,
>   
>   	kunmap(pg);
>   
> -	if (ret) {
> -		/* There was an error writing. */
> -		SetPageError(pg);
> -	}
> +	if (ret)
> +		mapping_set_error(mapping, ret);
>   
>   	/* Adjust writtenlen for the padding we did, so we don't confuse our caller */
>   	writtenlen -= min(writtenlen, (start - aligned_start));
> @@ -330,7 +323,6 @@ static int jffs2_write_end(struct file *filp, struct address_space *mapping,
>   		   it gets reread */
>   		jffs2_dbg(1, "%s(): Not all bytes written. Marking page !uptodate\n",
>   			__func__);
> -		SetPageError(pg);
>   		ClearPageUptodate(pg);
>   	}
>   
> 

Re: [PATCH 14/30] jffs2: Remove calls to set/clear the folio error flag

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "chengzhihao1" <chengzhihao1@huawei.com>
> An: "Matthew Wilcox" <willy@infradead.org>, "linux-fsdevel" <linux-fsdevel@vger.kernel.org>
> CC: "David Woodhouse" <dwmw2@infradead.org>, "richard" <richard@nod.at>, "linux-mtd" <linux-mtd@lists.infradead.org>
> Gesendet: Montag, 22. April 2024 16:46:37
> Betreff: Re: [PATCH 14/30] jffs2: Remove calls to set/clear the folio error flag

> 在 2024/4/20 10:50, Matthew Wilcox (Oracle) 写道:
>> Nobody checks the error flag on jffs2 folios, so stop setting and
>> clearing it.  We can also remove the call to clear the uptodate
>> flag; it will already be clear.
>> 
>> Convert one of these into a call to mapping_set_error() which will
>> actually be checked by other parts of the kernel.
>> 
>> Cc: David Woodhouse <dwmw2@infradead.org>
>> Cc: Richard Weinberger <richard@nod.at>
>> Cc: linux-mtd@lists.infradead.org
>> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>> ---
>>   fs/jffs2/file.c | 14 +++-----------
>>   1 file changed, 3 insertions(+), 11 deletions(-)
> 
> xfstests passed. Looks like the change is harmless.
> 
> Tested-by: Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

Acked-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

[PATCH 15/30] nfs: Remove calls to folio_set_error

[Matthew Wilcox (Oracle)]

Common code doesn't test the error flag, so we don't need to set it in
nfs.  We can use folio_end_read() to combine the setting (or not)
of the uptodate flag and clearing the lock flag.

Cc: Trond Myklebust <trond.myklebust@hammerspace.com>
Cc: Anna Schumaker <anna@kernel.org>
Cc: linux-nfs@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/nfs/read.c    |  2 --
 fs/nfs/symlink.c | 12 ++----------
 fs/nfs/write.c   |  1 -
 3 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index a142287d86f6..cca80b5f54e0 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -122,8 +122,6 @@ static void nfs_readpage_release(struct nfs_page *req, int error)
 {
 	struct folio *folio = nfs_page_to_folio(req);
 
-	if (nfs_error_is_fatal_on_server(error) && error != -ETIMEDOUT)
-		folio_set_error(folio);
 	if (nfs_page_group_sync_on_bit(req, PG_UNLOCKPAGE))
 		if (nfs_netfs_folio_unlock(folio))
 			folio_unlock(folio);
diff --git a/fs/nfs/symlink.c b/fs/nfs/symlink.c
index 0e27a2e4e68b..1c62a5a9f51d 100644
--- a/fs/nfs/symlink.c
+++ b/fs/nfs/symlink.c
@@ -32,16 +32,8 @@ static int nfs_symlink_filler(struct file *file, struct folio *folio)
 	int error;
 
 	error = NFS_PROTO(inode)->readlink(inode, &folio->page, 0, PAGE_SIZE);
-	if (error < 0)
-		goto error;
-	folio_mark_uptodate(folio);
-	folio_unlock(folio);
-	return 0;
-
-error:
-	folio_set_error(folio);
-	folio_unlock(folio);
-	return -EIO;
+	folio_end_read(folio, error == 0);
+	return error;
 }
 
 static const char *nfs_get_link(struct dentry *dentry,
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 2329cbb0e446..a91463ab87a0 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -311,7 +311,6 @@ static void nfs_mapping_set_error(struct folio *folio, int error)
 {
 	struct address_space *mapping = folio_file_mapping(folio);
 
-	folio_set_error(folio);
 	filemap_set_wb_err(mapping, error);
 	if (mapping->host)
 		errseq_set(&mapping->host->i_sb->s_wb_err,
-- 
2.43.0

[PATCH 16/30] nilfs2: Remove calls to folio_set_error() and folio_clear_error()

[Matthew Wilcox (Oracle)]

Nobody checks this flag on nilfs2 folios, stop setting and clearing it.
That lets us simplify nilfs_end_folio_io() slightly.

Cc: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Cc: linux-nilfs@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/nilfs2/dir.c     | 1 -
 fs/nilfs2/segment.c | 8 +-------
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index aee40db7a036..a002a44ff161 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -174,7 +174,6 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
 		    dir->i_ino, (folio->index << PAGE_SHIFT) + offs,
 		    (unsigned long)le64_to_cpu(p->inode));
 fail:
-	folio_set_error(folio);
 	return false;
 }
 
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index aa5290cb7467..8654ab8ad534 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -1725,14 +1725,8 @@ static void nilfs_end_folio_io(struct folio *folio, int err)
 		return;
 	}
 
-	if (!err) {
-		if (!nilfs_folio_buffers_clean(folio))
-			filemap_dirty_folio(folio->mapping, folio);
-		folio_clear_error(folio);
-	} else {
+	if (err || !nilfs_folio_buffers_clean(folio))
 		filemap_dirty_folio(folio->mapping, folio);
-		folio_set_error(folio);
-	}
 
 	folio_end_writeback(folio);
 }
-- 
2.43.0

Re: [PATCH 16/30] nilfs2: Remove calls to folio_set_error() and folio_clear_error()

[Ryusuke Konishi]

On Sat, Apr 20, 2024 at 11:50 AM Matthew Wilcox (Oracle) wrote:
>
> Nobody checks this flag on nilfs2 folios, stop setting and clearing it.
> That lets us simplify nilfs_end_folio_io() slightly.
>
> Cc: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> Cc: linux-nilfs@vger.kernel.org
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Looks good to me.  Feel free to send this for merging along with other
PG_error removal patches:

Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>

Or if you would like me to pick it up independently (e.g. to gradually
reduce the changes required for removal), I will do so.

Thanks,
Ryusuke Konishi

> ---
>  fs/nilfs2/dir.c     | 1 -
>  fs/nilfs2/segment.c | 8 +-------
>  2 files changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
> index aee40db7a036..a002a44ff161 100644
> --- a/fs/nilfs2/dir.c
> +++ b/fs/nilfs2/dir.c
> @@ -174,7 +174,6 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
>                     dir->i_ino, (folio->index << PAGE_SHIFT) + offs,
>                     (unsigned long)le64_to_cpu(p->inode));
>  fail:
> -       folio_set_error(folio);
>         return false;
>  }
>
> diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
> index aa5290cb7467..8654ab8ad534 100644
> --- a/fs/nilfs2/segment.c
> +++ b/fs/nilfs2/segment.c
> @@ -1725,14 +1725,8 @@ static void nilfs_end_folio_io(struct folio *folio, int err)
>                 return;
>         }
>
> -       if (!err) {
> -               if (!nilfs_folio_buffers_clean(folio))
> -                       filemap_dirty_folio(folio->mapping, folio);
> -               folio_clear_error(folio);
> -       } else {
> +       if (err || !nilfs_folio_buffers_clean(folio))
>                 filemap_dirty_folio(folio->mapping, folio);
> -               folio_set_error(folio);
> -       }
>
>         folio_end_writeback(folio);
>  }
> --
> 2.43.0

Re: [PATCH 16/30] nilfs2: Remove calls to folio_set_error() and folio_clear_error()

[Matthew Wilcox]

On Wed, Apr 24, 2024 at 01:36:52AM +0900, Ryusuke Konishi wrote:
> On Sat, Apr 20, 2024 at 11:50 AM Matthew Wilcox (Oracle) wrote:
> >
> > Nobody checks this flag on nilfs2 folios, stop setting and clearing it.
> > That lets us simplify nilfs_end_folio_io() slightly.
> >
> > Cc: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> > Cc: linux-nilfs@vger.kernel.org
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> 
> Looks good to me.  Feel free to send this for merging along with other
> PG_error removal patches:
> 
> Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> 
> Or if you would like me to pick it up independently (e.g. to gradually
> reduce the changes required for removal), I will do so.

Please take it through your tree; I'll prepare a pull request for the
remainder, but having more patches go through fs maintainers means
better testing.

Re: [PATCH 16/30] nilfs2: Remove calls to folio_set_error() and folio_clear_error()

[Ryusuke Konishi]

On Wed, Apr 24, 2024 at 2:51 AM Matthew Wilcox wrote:
>
> On Wed, Apr 24, 2024 at 01:36:52AM +0900, Ryusuke Konishi wrote:
> > On Sat, Apr 20, 2024 at 11:50 AM Matthew Wilcox (Oracle) wrote:
> > >
> > > Nobody checks this flag on nilfs2 folios, stop setting and clearing it.
> > > That lets us simplify nilfs_end_folio_io() slightly.
> > >
> > > Cc: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> > > Cc: linux-nilfs@vger.kernel.org
> > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> >
> > Looks good to me.  Feel free to send this for merging along with other
> > PG_error removal patches:
> >
> > Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> >
> > Or if you would like me to pick it up independently (e.g. to gradually
> > reduce the changes required for removal), I will do so.
>
> Please take it through your tree; I'll prepare a pull request for the
> remainder, but having more patches go through fs maintainers means
> better testing.

I got it, thanks.

Ryusuke Konishi

[PATCH 17/30] ntfs3: Remove calls to set/clear the error flag

[Matthew Wilcox (Oracle)]

Nobody checks the error flag on ntfs3 folios, so stop setting and
clearing it.

Cc: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
Cc: ntfs3@lists.linux.dev
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ntfs3/frecord.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 7f27382e0ce2..e9836170e2be 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -2143,9 +2143,6 @@ int ni_readpage_cmpr(struct ntfs_inode *ni, struct page *page)
 	err = ni_read_frame(ni, frame_vbo, pages, pages_per_frame);
 
 out1:
-	if (err)
-		SetPageError(page);
-
 	for (i = 0; i < pages_per_frame; i++) {
 		pg = pages[i];
 		if (i == idx || !pg)
@@ -2718,7 +2715,6 @@ int ni_read_frame(struct ntfs_inode *ni, u64 frame_vbo, struct page **pages,
 	for (i = 0; i < pages_per_frame; i++) {
 		pg = pages[i];
 		kunmap(pg);
-		ClearPageError(pg);
 		SetPageUptodate(pg);
 	}
 
-- 
2.43.0

[PATCH 18/30] orangefs: Remove calls to set/clear the error flag

[Matthew Wilcox (Oracle)]

Nobody checks the error flag on orangefs folios, so stop setting and
clearing it.  We can also use folio_end_read() to simplify
orangefs_read_folio().

Cc: Mike Marshall <hubcap@omnibond.com>
Cc: Martin Brandenburg <martin@omnibond.com>
Cc: devel@lists.orangefs.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/orangefs/inode.c           | 13 +++----------
 fs/orangefs/orangefs-bufmap.c |  4 +---
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index 085912268442..fdb9b65db1de 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -56,7 +56,6 @@ static int orangefs_writepage_locked(struct page *page,
 	ret = wait_for_direct_io(ORANGEFS_IO_WRITE, inode, &off, &iter, wlen,
 	    len, wr, NULL, NULL);
 	if (ret < 0) {
-		SetPageError(page);
 		mapping_set_error(page->mapping, ret);
 	} else {
 		ret = 0;
@@ -119,7 +118,6 @@ static int orangefs_writepages_work(struct orangefs_writepages *ow,
 	    0, &wr, NULL, NULL);
 	if (ret < 0) {
 		for (i = 0; i < ow->npages; i++) {
-			SetPageError(ow->pages[i]);
 			mapping_set_error(ow->pages[i]->mapping, ret);
 			if (PagePrivate(ow->pages[i])) {
 				wrp = (struct orangefs_write_range *)
@@ -303,15 +301,10 @@ static int orangefs_read_folio(struct file *file, struct folio *folio)
 	iov_iter_zero(~0U, &iter);
 	/* takes care of potential aliasing */
 	flush_dcache_folio(folio);
-	if (ret < 0) {
-		folio_set_error(folio);
-	} else {
-		folio_mark_uptodate(folio);
+	if (ret > 0)
 		ret = 0;
-	}
-	/* unlock the folio after the ->read_folio() routine completes */
-	folio_unlock(folio);
-        return ret;
+	folio_end_read(folio, ret == 0);
+	return ret;
 }
 
 static int orangefs_write_begin(struct file *file,
diff --git a/fs/orangefs/orangefs-bufmap.c b/fs/orangefs/orangefs-bufmap.c
index b501dc07f922..edcca4beb765 100644
--- a/fs/orangefs/orangefs-bufmap.c
+++ b/fs/orangefs/orangefs-bufmap.c
@@ -274,10 +274,8 @@ orangefs_bufmap_map(struct orangefs_bufmap *bufmap,
 		gossip_err("orangefs error: asked for %d pages, only got %d.\n",
 				bufmap->page_count, ret);
 
-		for (i = 0; i < ret; i++) {
-			SetPageError(bufmap->page_array[i]);
+		for (i = 0; i < ret; i++)
 			unpin_user_page(bufmap->page_array[i]);
-		}
 		return -ENOMEM;
 	}
 
-- 
2.43.0

Re: [PATCH 18/30] orangefs: Remove calls to set/clear the error flag

[Mike Marshall]

I added this patch to 6.9.0-rc5 and ran it through xfstests with no problems...

-Mike

On Fri, Apr 19, 2024 at 10:50 PM Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
>
> Nobody checks the error flag on orangefs folios, so stop setting and
> clearing it.  We can also use folio_end_read() to simplify
> orangefs_read_folio().
>
> Cc: Mike Marshall <hubcap@omnibond.com>
> Cc: Martin Brandenburg <martin@omnibond.com>
> Cc: devel@lists.orangefs.org
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/orangefs/inode.c           | 13 +++----------
>  fs/orangefs/orangefs-bufmap.c |  4 +---
>  2 files changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
> index 085912268442..fdb9b65db1de 100644
> --- a/fs/orangefs/inode.c
> +++ b/fs/orangefs/inode.c
> @@ -56,7 +56,6 @@ static int orangefs_writepage_locked(struct page *page,
>         ret = wait_for_direct_io(ORANGEFS_IO_WRITE, inode, &off, &iter, wlen,
>             len, wr, NULL, NULL);
>         if (ret < 0) {
> -               SetPageError(page);
>                 mapping_set_error(page->mapping, ret);
>         } else {
>                 ret = 0;
> @@ -119,7 +118,6 @@ static int orangefs_writepages_work(struct orangefs_writepages *ow,
>             0, &wr, NULL, NULL);
>         if (ret < 0) {
>                 for (i = 0; i < ow->npages; i++) {
> -                       SetPageError(ow->pages[i]);
>                         mapping_set_error(ow->pages[i]->mapping, ret);
>                         if (PagePrivate(ow->pages[i])) {
>                                 wrp = (struct orangefs_write_range *)
> @@ -303,15 +301,10 @@ static int orangefs_read_folio(struct file *file, struct folio *folio)
>         iov_iter_zero(~0U, &iter);
>         /* takes care of potential aliasing */
>         flush_dcache_folio(folio);
> -       if (ret < 0) {
> -               folio_set_error(folio);
> -       } else {
> -               folio_mark_uptodate(folio);
> +       if (ret > 0)
>                 ret = 0;
> -       }
> -       /* unlock the folio after the ->read_folio() routine completes */
> -       folio_unlock(folio);
> -        return ret;
> +       folio_end_read(folio, ret == 0);
> +       return ret;
>  }
>
>  static int orangefs_write_begin(struct file *file,
> diff --git a/fs/orangefs/orangefs-bufmap.c b/fs/orangefs/orangefs-bufmap.c
> index b501dc07f922..edcca4beb765 100644
> --- a/fs/orangefs/orangefs-bufmap.c
> +++ b/fs/orangefs/orangefs-bufmap.c
> @@ -274,10 +274,8 @@ orangefs_bufmap_map(struct orangefs_bufmap *bufmap,
>                 gossip_err("orangefs error: asked for %d pages, only got %d.\n",
>                                 bufmap->page_count, ret);
>
> -               for (i = 0; i < ret; i++) {
> -                       SetPageError(bufmap->page_array[i]);
> +               for (i = 0; i < ret; i++)
>                         unpin_user_page(bufmap->page_array[i]);
> -               }
>                 return -ENOMEM;
>         }
>
> --
> 2.43.0
>

[PATCH 19/30] reiserfs: Remove call to folio_set_error()

[Matthew Wilcox (Oracle)]

The error flag is never tested for reiserfs folios, so stop setting it.

Cc: reiserfs-devel@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/reiserfs/inode.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index c1daedc50f4c..9b43a81a6488 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -2699,7 +2699,6 @@ static int reiserfs_write_folio(struct folio *folio,
 		}
 		bh = bh->b_this_page;
 	} while (bh != head);
-	folio_set_error(folio);
 	BUG_ON(folio_test_writeback(folio));
 	folio_start_writeback(folio);
 	folio_unlock(folio);
-- 
2.43.0

[PATCH 20/30] romfs: Convert romfs_read_folio() to use a folio

[Matthew Wilcox (Oracle)]

Remove the conversion back to struct page and use the folio APIs instead
of the page APIs.  It's probably more trouble than it's worth to support
large folios in romfs, so there are still PAGE_SIZE assumptions in
this function.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/romfs/super.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/fs/romfs/super.c b/fs/romfs/super.c
index 2cbb92462074..68758b6fed94 100644
--- a/fs/romfs/super.c
+++ b/fs/romfs/super.c
@@ -101,19 +101,15 @@ static struct inode *romfs_iget(struct super_block *sb, unsigned long pos);
  */
 static int romfs_read_folio(struct file *file, struct folio *folio)
 {
-	struct page *page = &folio->page;
-	struct inode *inode = page->mapping->host;
+	struct inode *inode = folio->mapping->host;
 	loff_t offset, size;
 	unsigned long fillsize, pos;
 	void *buf;
 	int ret;
 
-	buf = kmap(page);
-	if (!buf)
-		return -ENOMEM;
+	buf = kmap_local_folio(folio, 0);
 
-	/* 32 bit warning -- but not for us :) */
-	offset = page_offset(page);
+	offset = folio_pos(folio);
 	size = i_size_read(inode);
 	fillsize = 0;
 	ret = 0;
@@ -125,20 +121,14 @@ static int romfs_read_folio(struct file *file, struct folio *folio)
 
 		ret = romfs_dev_read(inode->i_sb, pos, buf, fillsize);
 		if (ret < 0) {
-			SetPageError(page);
 			fillsize = 0;
 			ret = -EIO;
 		}
 	}
 
-	if (fillsize < PAGE_SIZE)
-		memset(buf + fillsize, 0, PAGE_SIZE - fillsize);
-	if (ret == 0)
-		SetPageUptodate(page);
-
-	flush_dcache_page(page);
-	kunmap(page);
-	unlock_page(page);
+	buf = folio_zero_tail(folio, fillsize, buf);
+	kunmap_local(buf);
+	folio_end_read(folio, ret == 0);
 	return ret;
 }
 
-- 
2.43.0

[PATCH 21/30] smb: Remove calls to set folio error flag

[Matthew Wilcox (Oracle)]

The folio error flag is never checked for smb folios, so remove
calls to set it.

Cc: Steve French <sfrench@samba.org>
Cc: linux-cifs@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/smb/client/file.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index cf02d981fc13..dce121c5a86a 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -124,7 +124,6 @@ void cifs_pages_write_failed(struct inode *inode, loff_t start, unsigned int len
 			continue;
 		}
 
-		folio_set_error(folio);
 		folio_end_writeback(folio);
 	}
 
@@ -3156,7 +3155,6 @@ cifs_writepage_locked(struct page *page, struct writeback_control *wbc)
 			goto retry_write;
 		redirty_page_for_writepage(wbc, page);
 	} else if (rc != 0) {
-		SetPageError(page);
 		mapping_set_error(page->mapping, rc);
 	} else {
 		SetPageUptodate(page);
-- 
2.43.0

[PATCH 22/30] squashfs: Convert squashfs_symlink_read_folio to use folio APIs

[Matthew Wilcox (Oracle)]

Remove use of page APIs, return the errno instead of 0, switch from
kmap_atomic to kmap_local and use folio_end_read() to unify the two
exit paths.

Cc: Phillip Lougher <phillip@squashfs.org.uk>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/squashfs/symlink.c | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/fs/squashfs/symlink.c b/fs/squashfs/symlink.c
index 2bf977a52c2c..6ef735bd841a 100644
--- a/fs/squashfs/symlink.c
+++ b/fs/squashfs/symlink.c
@@ -32,20 +32,19 @@
 
 static int squashfs_symlink_read_folio(struct file *file, struct folio *folio)
 {
-	struct page *page = &folio->page;
-	struct inode *inode = page->mapping->host;
+	struct inode *inode = folio->mapping->host;
 	struct super_block *sb = inode->i_sb;
 	struct squashfs_sb_info *msblk = sb->s_fs_info;
-	int index = page->index << PAGE_SHIFT;
+	int index = folio_pos(folio);
 	u64 block = squashfs_i(inode)->start;
 	int offset = squashfs_i(inode)->offset;
 	int length = min_t(int, i_size_read(inode) - index, PAGE_SIZE);
-	int bytes, copied;
+	int bytes, copied, error;
 	void *pageaddr;
 	struct squashfs_cache_entry *entry;
 
 	TRACE("Entered squashfs_symlink_readpage, page index %ld, start block "
-			"%llx, offset %x\n", page->index, block, offset);
+			"%llx, offset %x\n", folio->index, block, offset);
 
 	/*
 	 * Skip index bytes into symlink metadata.
@@ -57,14 +56,15 @@ static int squashfs_symlink_read_folio(struct file *file, struct folio *folio)
 			ERROR("Unable to read symlink [%llx:%x]\n",
 				squashfs_i(inode)->start,
 				squashfs_i(inode)->offset);
-			goto error_out;
+			error = bytes;
+			goto out;
 		}
 	}
 
 	/*
 	 * Read length bytes from symlink metadata.  Squashfs_read_metadata
 	 * is not used here because it can sleep and we want to use
-	 * kmap_atomic to map the page.  Instead call the underlying
+	 * kmap_local to map the folio.  Instead call the underlying
 	 * squashfs_cache_get routine.  As length bytes may overlap metadata
 	 * blocks, we may need to call squashfs_cache_get multiple times.
 	 */
@@ -75,29 +75,26 @@ static int squashfs_symlink_read_folio(struct file *file, struct folio *folio)
 				squashfs_i(inode)->start,
 				squashfs_i(inode)->offset);
 			squashfs_cache_put(entry);
-			goto error_out;
+			error = entry->error;
+			goto out;
 		}
 
-		pageaddr = kmap_atomic(page);
+		pageaddr = kmap_local_folio(folio, 0);
 		copied = squashfs_copy_data(pageaddr + bytes, entry, offset,
 								length - bytes);
 		if (copied == length - bytes)
 			memset(pageaddr + length, 0, PAGE_SIZE - length);
 		else
 			block = entry->next_index;
-		kunmap_atomic(pageaddr);
+		kunmap_local(pageaddr);
 		squashfs_cache_put(entry);
 	}
 
-	flush_dcache_page(page);
-	SetPageUptodate(page);
-	unlock_page(page);
-	return 0;
-
-error_out:
-	SetPageError(page);
-	unlock_page(page);
-	return 0;
+	flush_dcache_folio(folio);
+	error = 0;
+out:
+	folio_end_read(folio, error == 0);
+	return error;
 }
 
 
-- 
2.43.0

Re: [PATCH 22/30] squashfs: Convert squashfs_symlink_read_folio to use folio APIs

[Phillip Lougher]

On 20/04/2024 03:50, Matthew Wilcox (Oracle) wrote:
> Remove use of page APIs, return the errno instead of 0, switch from
> kmap_atomic to kmap_local and use folio_end_read() to unify the two
> exit paths.
> 
> Cc: Phillip Lougher <phillip@squashfs.org.uk>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Tested-by: Phillip Lougher <phillip@squashfs.org.uk>
Reviewed-by: Phillip Lougher <phillip@squashfs.org.uk>

You've mentioned a couple of times you prefer the patches in
the series to go through the fs maintainers.  Andrew Morton is
currently handling submission of Squashfs patches for me, and
I'm happy with either Andrew or you merging it.

CC'ing Andrew.

Regards

Phillip

> ---
>   fs/squashfs/symlink.c | 35 ++++++++++++++++-------------------
>   1 file changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/squashfs/symlink.c b/fs/squashfs/symlink.c
> index 2bf977a52c2c..6ef735bd841a 100644
> --- a/fs/squashfs/symlink.c
> +++ b/fs/squashfs/symlink.c
> @@ -32,20 +32,19 @@
>   
>   static int squashfs_symlink_read_folio(struct file *file, struct folio *folio)
>   {
> -	struct page *page = &folio->page;
> -	struct inode *inode = page->mapping->host;
> +	struct inode *inode = folio->mapping->host;
>   	struct super_block *sb = inode->i_sb;
>   	struct squashfs_sb_info *msblk = sb->s_fs_info;
> -	int index = page->index << PAGE_SHIFT;
> +	int index = folio_pos(folio);
>   	u64 block = squashfs_i(inode)->start;
>   	int offset = squashfs_i(inode)->offset;
>   	int length = min_t(int, i_size_read(inode) - index, PAGE_SIZE);
> -	int bytes, copied;
> +	int bytes, copied, error;
>   	void *pageaddr;
>   	struct squashfs_cache_entry *entry;
>   
>   	TRACE("Entered squashfs_symlink_readpage, page index %ld, start block "
> -			"%llx, offset %x\n", page->index, block, offset);
> +			"%llx, offset %x\n", folio->index, block, offset);
>   
>   	/*
>   	 * Skip index bytes into symlink metadata.
> @@ -57,14 +56,15 @@ static int squashfs_symlink_read_folio(struct file *file, struct folio *folio)
>   			ERROR("Unable to read symlink [%llx:%x]\n",
>   				squashfs_i(inode)->start,
>   				squashfs_i(inode)->offset);
> -			goto error_out;
> +			error = bytes;
> +			goto out;
>   		}
>   	}
>   
>   	/*
>   	 * Read length bytes from symlink metadata.  Squashfs_read_metadata
>   	 * is not used here because it can sleep and we want to use
> -	 * kmap_atomic to map the page.  Instead call the underlying
> +	 * kmap_local to map the folio.  Instead call the underlying
>   	 * squashfs_cache_get routine.  As length bytes may overlap metadata
>   	 * blocks, we may need to call squashfs_cache_get multiple times.
>   	 */
> @@ -75,29 +75,26 @@ static int squashfs_symlink_read_folio(struct file *file, struct folio *folio)
>   				squashfs_i(inode)->start,
>   				squashfs_i(inode)->offset);
>   			squashfs_cache_put(entry);
> -			goto error_out;
> +			error = entry->error;
> +			goto out;
>   		}
>   
> -		pageaddr = kmap_atomic(page);
> +		pageaddr = kmap_local_folio(folio, 0);
>   		copied = squashfs_copy_data(pageaddr + bytes, entry, offset,
>   								length - bytes);
>   		if (copied == length - bytes)
>   			memset(pageaddr + length, 0, PAGE_SIZE - length);
>   		else
>   			block = entry->next_index;
> -		kunmap_atomic(pageaddr);
> +		kunmap_local(pageaddr);
>   		squashfs_cache_put(entry);
>   	}
>   
> -	flush_dcache_page(page);
> -	SetPageUptodate(page);
> -	unlock_page(page);
> -	return 0;
> -
> -error_out:
> -	SetPageError(page);
> -	unlock_page(page);
> -	return 0;
> +	flush_dcache_folio(folio);
> +	error = 0;
> +out:
> +	folio_end_read(folio, error == 0);
> +	return error;
>   }
>   
>   

[PATCH 23/30] squashfs: Remove calls to set the folio error flag

[Matthew Wilcox (Oracle)]

Nobody checks the error flag on squashfs folios, so stop setting it.

Cc: Phillip Lougher <phillip@squashfs.org.uk>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/squashfs/file.c        | 6 +-----
 fs/squashfs/file_direct.c | 3 +--
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index e8df6430444b..a8c1e7f9a609 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -375,8 +375,6 @@ void squashfs_fill_page(struct page *page, struct squashfs_cache_entry *buffer,
 	flush_dcache_page(page);
 	if (copied == avail)
 		SetPageUptodate(page);
-	else
-		SetPageError(page);
 }
 
 /* Copy data into page cache  */
@@ -471,7 +469,7 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
 
 		res = read_blocklist(inode, index, &block);
 		if (res < 0)
-			goto error_out;
+			goto out;
 
 		if (res == 0)
 			res = squashfs_readpage_sparse(page, expected);
@@ -483,8 +481,6 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
 	if (!res)
 		return 0;
 
-error_out:
-	SetPageError(page);
 out:
 	pageaddr = kmap_atomic(page);
 	memset(pageaddr, 0, PAGE_SIZE);
diff --git a/fs/squashfs/file_direct.c b/fs/squashfs/file_direct.c
index 763a3f7a75f6..2a689ce71de9 100644
--- a/fs/squashfs/file_direct.c
+++ b/fs/squashfs/file_direct.c
@@ -106,14 +106,13 @@ int squashfs_readpage_block(struct page *target_page, u64 block, int bsize,
 	return 0;
 
 mark_errored:
-	/* Decompression failed, mark pages as errored.  Target_page is
+	/* Decompression failed.  Target_page is
 	 * dealt with by the caller
 	 */
 	for (i = 0; i < pages; i++) {
 		if (page[i] == NULL || page[i] == target_page)
 			continue;
 		flush_dcache_page(page[i]);
-		SetPageError(page[i]);
 		unlock_page(page[i]);
 		put_page(page[i]);
 	}
-- 
2.43.0

Re: [PATCH 23/30] squashfs: Remove calls to set the folio error flag

[Phillip Lougher]

On 20/04/2024 03:50, Matthew Wilcox (Oracle) wrote:
> Nobody checks the error flag on squashfs folios, so stop setting it.
> 
> Cc: Phillip Lougher <phillip@squashfs.org.uk>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Tested-by: Phillip Lougher <phillip@squashfs.org.uk>
Reviewed-by: Phillip Lougher <phillip@squashfs.org.uk>

You've mentioned a couple of times you prefer the patches in
the series to go through the fs maintainers.  Andrew Morton is
currently handling submission of Squashfs patches for me, and
I'm happy with either Andrew or you merging it.

CC'ing Andrew.

Regards

Phillip

> ---
>   fs/squashfs/file.c        | 6 +-----
>   fs/squashfs/file_direct.c | 3 +--
>   2 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index e8df6430444b..a8c1e7f9a609 100644
> --- a/fs/squashfs/file.c
> +++ b/fs/squashfs/file.c
> @@ -375,8 +375,6 @@ void squashfs_fill_page(struct page *page, struct squashfs_cache_entry *buffer,
>   	flush_dcache_page(page);
>   	if (copied == avail)
>   		SetPageUptodate(page);
> -	else
> -		SetPageError(page);
>   }
>   
>   /* Copy data into page cache  */
> @@ -471,7 +469,7 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
>   
>   		res = read_blocklist(inode, index, &block);
>   		if (res < 0)
> -			goto error_out;
> +			goto out;
>   
>   		if (res == 0)
>   			res = squashfs_readpage_sparse(page, expected);
> @@ -483,8 +481,6 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
>   	if (!res)
>   		return 0;
>   
> -error_out:
> -	SetPageError(page);
>   out:
>   	pageaddr = kmap_atomic(page);
>   	memset(pageaddr, 0, PAGE_SIZE);
> diff --git a/fs/squashfs/file_direct.c b/fs/squashfs/file_direct.c
> index 763a3f7a75f6..2a689ce71de9 100644
> --- a/fs/squashfs/file_direct.c
> +++ b/fs/squashfs/file_direct.c
> @@ -106,14 +106,13 @@ int squashfs_readpage_block(struct page *target_page, u64 block, int bsize,
>   	return 0;
>   
>   mark_errored:
> -	/* Decompression failed, mark pages as errored.  Target_page is
> +	/* Decompression failed.  Target_page is
>   	 * dealt with by the caller
>   	 */
>   	for (i = 0; i < pages; i++) {
>   		if (page[i] == NULL || page[i] == target_page)
>   			continue;
>   		flush_dcache_page(page[i]);
> -		SetPageError(page[i]);
>   		unlock_page(page[i]);
>   		put_page(page[i]);
>   	}

[PATCH 24/30] ufs: Remove call to set the folio error flag

[Matthew Wilcox (Oracle)]

Nobody checks the error flag on ufs folios, so stop setting it.

Cc: Evgeniy Dushistov <dushistov@mail.ru>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ufs/dir.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/ufs/dir.c b/fs/ufs/dir.c
index 27c85d92d1dc..61f25d3cf3f7 100644
--- a/fs/ufs/dir.c
+++ b/fs/ufs/dir.c
@@ -188,7 +188,6 @@ static bool ufs_check_page(struct page *page)
 		   "offset=%lu",
 		   dir->i_ino, (page->index<<PAGE_SHIFT)+offs);
 fail:
-	SetPageError(page);
 	return false;
 }
 
-- 
2.43.0

[PATCH 25/30] vboxsf: Convert vboxsf_read_folio() to use a folio

[Matthew Wilcox (Oracle)]

Remove conversion to a page and use folio APIs throughout.  This includes
a removal of setting the error flag as nobody checks the error flag on
vboxsf folios.  This does not include large folio support as we would
have to map each page individually.

Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/vboxsf/file.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/fs/vboxsf/file.c b/fs/vboxsf/file.c
index 118dedef8ebe..e149158b105d 100644
--- a/fs/vboxsf/file.c
+++ b/fs/vboxsf/file.c
@@ -228,26 +228,19 @@ const struct inode_operations vboxsf_reg_iops = {
 
 static int vboxsf_read_folio(struct file *file, struct folio *folio)
 {
-	struct page *page = &folio->page;
 	struct vboxsf_handle *sf_handle = file->private_data;
-	loff_t off = page_offset(page);
+	loff_t off = folio_pos(folio);
 	u32 nread = PAGE_SIZE;
 	u8 *buf;
 	int err;
 
-	buf = kmap(page);
+	buf = kmap_local_folio(folio, 0);
 
 	err = vboxsf_read(sf_handle->root, sf_handle->handle, off, &nread, buf);
-	if (err == 0) {
-		memset(&buf[nread], 0, PAGE_SIZE - nread);
-		flush_dcache_page(page);
-		SetPageUptodate(page);
-	} else {
-		SetPageError(page);
-	}
+	buf = folio_zero_tail(folio, nread, buf);
 
-	kunmap(page);
-	unlock_page(page);
+	kunmap_local(buf);
+	folio_end_read(folio, err == 0);
 	return err;
 }
 
@@ -295,7 +288,6 @@ static int vboxsf_writepage(struct page *page, struct writeback_control *wbc)
 	kref_put(&sf_handle->refcount, vboxsf_handle_release);
 
 	if (err == 0) {
-		ClearPageError(page);
 		/* mtime changed */
 		sf_i->force_restat = 1;
 	} else {
-- 
2.43.0

Re: [PATCH 25/30] vboxsf: Convert vboxsf_read_folio() to use a folio

[Hans de Goede]

Hi Matthew,

On 4/20/24 4:50 AM, Matthew Wilcox (Oracle) wrote:
> Remove conversion to a page and use folio APIs throughout.  This includes
> a removal of setting the error flag as nobody checks the error flag on
> vboxsf folios.  This does not include large folio support as we would
> have to map each page individually.
> 
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/vboxsf/file.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/vboxsf/file.c b/fs/vboxsf/file.c
> index 118dedef8ebe..e149158b105d 100644
> --- a/fs/vboxsf/file.c
> +++ b/fs/vboxsf/file.c
> @@ -228,26 +228,19 @@ const struct inode_operations vboxsf_reg_iops = {
>  
>  static int vboxsf_read_folio(struct file *file, struct folio *folio)
>  {
> -	struct page *page = &folio->page;
>  	struct vboxsf_handle *sf_handle = file->private_data;
> -	loff_t off = page_offset(page);
> +	loff_t off = folio_pos(folio);
>  	u32 nread = PAGE_SIZE;
>  	u8 *buf;
>  	int err;
>  
> -	buf = kmap(page);
> +	buf = kmap_local_folio(folio, 0);
>  
>  	err = vboxsf_read(sf_handle->root, sf_handle->handle, off, &nread, buf);
> -	if (err == 0) {
> -		memset(&buf[nread], 0, PAGE_SIZE - nread);
> -		flush_dcache_page(page);
> -		SetPageUptodate(page);
> -	} else {
> -		SetPageError(page);
> -	}
> +	buf = folio_zero_tail(folio, nread, buf);
>  
> -	kunmap(page);
> -	unlock_page(page);
> +	kunmap_local(buf);
> +	folio_end_read(folio, err == 0);
>  	return err;
>  }
>  

Thanks you for the patch.

I have this a test spin, but I got all 0 content for the kernel's README when trying
to read that from a directory shared through vboxsf.

I came up with the following fix for this, which I assume is the correct fix,
but please check:

--- a/fs/vboxsf/file.c
+++ b/fs/vboxsf/file.c
@@ -237,7 +237,7 @@ static int vboxsf_read_folio(struct file *file, struct folio *folio)
 	buf = kmap_local_folio(folio, 0);
 
 	err = vboxsf_read(sf_handle->root, sf_handle->handle, off, &nread, buf);
-	buf = folio_zero_tail(folio, nread, buf);
+	buf = folio_zero_tail(folio, nread, buf + nread);
 
 	kunmap_local(buf);
 	folio_end_read(folio, err == 0);


With this fix squashed in, this looks good to me and you can add:

Tested-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Feel free to merge this together with the other folio patches from this series.

Regards,

Hans






> @@ -295,7 +288,6 @@ static int vboxsf_writepage(struct page *page, struct writeback_control *wbc)
>  	kref_put(&sf_handle->refcount, vboxsf_handle_release);
>  
>  	if (err == 0) {
> -		ClearPageError(page);
>  		/* mtime changed */
>  		sf_i->force_restat = 1;
>  	} else {

[PATCH 26/30] mm/memory-failure: Stop setting the folio error flag

[Matthew Wilcox (Oracle)]

Nobody checks the error flag any more, so setting it accomplishes
nothing.  Remove the obsolete parts of this comment; it hasn't
been true since errseq_t was used to track writeback errors in 2017.

Cc: Miaohe Lin <linmiaohe@huawei.com>
Cc: linux-mm@kvack.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/memory-failure.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index ec30611ec5ad..e065dd9be21e 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1092,7 +1092,6 @@ static int me_pagecache_dirty(struct page_state *ps, struct page *p)
 {
 	struct address_space *mapping = page_mapping(p);
 
-	SetPageError(p);
 	/* TBD: print more information about the file. */
 	if (mapping) {
 		/*
@@ -1100,34 +1099,6 @@ static int me_pagecache_dirty(struct page_state *ps, struct page *p)
 		 * who check the mapping.
 		 * This way the application knows that something went
 		 * wrong with its dirty file data.
-		 *
-		 * There's one open issue:
-		 *
-		 * The EIO will be only reported on the next IO
-		 * operation and then cleared through the IO map.
-		 * Normally Linux has two mechanisms to pass IO error
-		 * first through the AS_EIO flag in the address space
-		 * and then through the PageError flag in the page.
-		 * Since we drop pages on memory failure handling the
-		 * only mechanism open to use is through AS_AIO.
-		 *
-		 * This has the disadvantage that it gets cleared on
-		 * the first operation that returns an error, while
-		 * the PageError bit is more sticky and only cleared
-		 * when the page is reread or dropped.  If an
-		 * application assumes it will always get error on
-		 * fsync, but does other operations on the fd before
-		 * and the page is dropped between then the error
-		 * will not be properly reported.
-		 *
-		 * This can already happen even without hwpoisoned
-		 * pages: first on metadata IO errors (which only
-		 * report through AS_EIO) or when the page is dropped
-		 * at the wrong time.
-		 *
-		 * So right now we assume that the application DTRT on
-		 * the first EIO, but we're not worse than other parts
-		 * of the kernel.
 		 */
 		mapping_set_error(mapping, -EIO);
 	}
-- 
2.43.0

[PATCH 27/30] iomap: Remove calls to set and clear folio error flag

[Matthew Wilcox (Oracle)]

The folio error flag is not checked anywhere, so we can remove the calls
to set and clear it.

Cc: Christian Brauner <brauner@kernel.org>
Cc: Darrick J. Wong <djwong@kernel.org>
Cc: linux-xfs@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/iomap/buffered-io.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 4e8e41c8b3c0..41352601f939 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -306,8 +306,6 @@ static void iomap_finish_folio_read(struct folio *folio, size_t off,
 		spin_unlock_irqrestore(&ifs->state_lock, flags);
 	}
 
-	if (error)
-		folio_set_error(folio);
 	if (finished)
 		folio_end_read(folio, uptodate);
 }
@@ -460,9 +458,6 @@ int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops)
 	while ((ret = iomap_iter(&iter, ops)) > 0)
 		iter.processed = iomap_readpage_iter(&iter, &ctx, 0);
 
-	if (ret < 0)
-		folio_set_error(folio);
-
 	if (ctx.bio) {
 		submit_bio(ctx.bio);
 		WARN_ON_ONCE(!ctx.cur_folio_in_bio);
@@ -697,7 +692,6 @@ static int __iomap_write_begin(const struct iomap_iter *iter, loff_t pos,
 
 	if (folio_test_uptodate(folio))
 		return 0;
-	folio_clear_error(folio);
 
 	do {
 		iomap_adjust_read_range(iter->inode, folio, &block_start,
@@ -1528,8 +1522,6 @@ iomap_finish_ioend(struct iomap_ioend *ioend, int error)
 
 	/* walk all folios in bio, ending page IO on them */
 	bio_for_each_folio_all(fi, bio) {
-		if (error)
-			folio_set_error(fi.folio);
 		iomap_finish_folio_write(inode, fi.folio, fi.length);
 		folio_count++;
 	}
-- 
2.43.0

Re: [PATCH 27/30] iomap: Remove calls to set and clear folio error flag

[Christoph Hellwig]

On Sat, Apr 20, 2024 at 03:50:22AM +0100, Matthew Wilcox (Oracle) wrote:
> The folio error flag is not checked anywhere, so we can remove the calls
> to set and clear it.

This patch on it's own looks good, but seeing this is a 27/30 I have
no chance to actually fully review it.

Re: [PATCH 27/30] iomap: Remove calls to set and clear folio error flag

[Matthew Wilcox]

On Sun, Apr 21, 2024 at 11:16:01PM -0700, Christoph Hellwig wrote:
> On Sat, Apr 20, 2024 at 03:50:22AM +0100, Matthew Wilcox (Oracle) wrote:
> > The folio error flag is not checked anywhere, so we can remove the calls
> > to set and clear it.
> 
> This patch on it's own looks good, but seeing this is a 27/30 I have
> no chance to actually fully review it.

You were bcc'd on 0/30 which fully explained this.

Re: [PATCH 27/30] iomap: Remove calls to set and clear folio error flag

[Christoph Hellwig]

On Mon, Apr 22, 2024 at 04:05:59PM +0100, Matthew Wilcox wrote:
> On Sun, Apr 21, 2024 at 11:16:01PM -0700, Christoph Hellwig wrote:
> > On Sat, Apr 20, 2024 at 03:50:22AM +0100, Matthew Wilcox (Oracle) wrote:
> > > The folio error flag is not checked anywhere, so we can remove the calls
> > > to set and clear it.
> > 
> > This patch on it's own looks good, but seeing this is a 27/30 I have
> > no chance to actually fully review it.
> 
> You were bcc'd on 0/30 which fully explained this.

Not on the XFS list through which I'm reading this at least.  If it
was to me personally those all go to >/dev/null anyway for mails
Cced to mailing lists.

Please always send the damn series to everyone, fishing individual
mails out of it is just a giant pain in the butt.

Re: [PATCH 27/30] iomap: Remove calls to set and clear folio error flag

[Matthew Wilcox]

On Mon, Apr 22, 2024 at 08:26:34AM -0700, Christoph Hellwig wrote:
> On Mon, Apr 22, 2024 at 04:05:59PM +0100, Matthew Wilcox wrote:
> > On Sun, Apr 21, 2024 at 11:16:01PM -0700, Christoph Hellwig wrote:
> > > On Sat, Apr 20, 2024 at 03:50:22AM +0100, Matthew Wilcox (Oracle) wrote:
> > > > The folio error flag is not checked anywhere, so we can remove the calls
> > > > to set and clear it.
> > > 
> > > This patch on it's own looks good, but seeing this is a 27/30 I have
> > > no chance to actually fully review it.
> > 
> > You were bcc'd on 0/30 which fully explained this.
> 
> Not on the XFS list through which I'm reading this at least.  If it
> was to me personally those all go to >/dev/null anyway for mails
> Cced to mailing lists.
> 
> Please always send the damn series to everyone, fishing individual
> mails out of it is just a giant pain in the butt.

If I do that then half the mailing lists bounce them for having too
many recipients.  b4 can fetch the entire series for you if you've
decided to break your email workflow.  And yes, 0/30 was bcc'd to
linux-xfs as well.

Re: [PATCH 27/30] iomap: Remove calls to set and clear folio error flag

[Christoph Hellwig]

On Mon, Apr 22, 2024 at 06:51:37PM +0100, Matthew Wilcox wrote:
> If I do that then half the mailing lists bounce them for having too
> many recipients.  b4 can fetch the entire series for you if you've
> decided to break your email workflow.  And yes, 0/30 was bcc'd to
> linux-xfs as well.

I can't find it on linux-xfs still.  And please just don't make up
your own workflow or require odd tools.

Re: [PATCH 27/30] iomap: Remove calls to set and clear folio error flag

[Matthew Wilcox]

On Thu, Apr 25, 2024 at 05:23:44AM -0700, Christoph Hellwig wrote:
> On Mon, Apr 22, 2024 at 06:51:37PM +0100, Matthew Wilcox wrote:
> > If I do that then half the mailing lists bounce them for having too
> > many recipients.  b4 can fetch the entire series for you if you've
> > decided to break your email workflow.  And yes, 0/30 was bcc'd to
> > linux-xfs as well.
> 
> I can't find it on linux-xfs still.  And please just don't make up
> your own workflow or require odd tools.

You even quoted the bit where I explained that the workflow you insist I
follow doesn't work.

Re: [PATCH 27/30] iomap: Remove calls to set and clear folio error flag

[Christoph Hellwig]

On Thu, Apr 25, 2024 at 01:44:49PM +0100, Matthew Wilcox wrote:
> On Thu, Apr 25, 2024 at 05:23:44AM -0700, Christoph Hellwig wrote:
> > On Mon, Apr 22, 2024 at 06:51:37PM +0100, Matthew Wilcox wrote:
> > > If I do that then half the mailing lists bounce them for having too
> > > many recipients.  b4 can fetch the entire series for you if you've
> > > decided to break your email workflow.  And yes, 0/30 was bcc'd to
> > > linux-xfs as well.
> > 
> > I can't find it on linux-xfs still.  And please just don't make up
> > your own workflow or require odd tools.
> 
> You even quoted the bit where I explained that the workflow you insist I
> follow doesn't work.

I've regularly sent series to more list than you'd need for 30
patches even if they were entirely unrelated.  But if they are
entirely unrelated it shouldn't be a series to start with..

Re: [PATCH 27/30] iomap: Remove calls to set and clear folio error flag

[Darrick J. Wong]

On Thu, Apr 25, 2024 at 05:51:45AM -0700, Christoph Hellwig wrote:
> On Thu, Apr 25, 2024 at 01:44:49PM +0100, Matthew Wilcox wrote:
> > On Thu, Apr 25, 2024 at 05:23:44AM -0700, Christoph Hellwig wrote:
> > > On Mon, Apr 22, 2024 at 06:51:37PM +0100, Matthew Wilcox wrote:
> > > > If I do that then half the mailing lists bounce them for having too
> > > > many recipients.  b4 can fetch the entire series for you if you've
> > > > decided to break your email workflow.  And yes, 0/30 was bcc'd to
> > > > linux-xfs as well.
> > > 
> > > I can't find it on linux-xfs still.  And please just don't make up
> > > your own workflow or require odd tools.
> > 
> > You even quoted the bit where I explained that the workflow you insist I
> > follow doesn't work.
> 
> I've regularly sent series to more list than you'd need for 30
> patches even if they were entirely unrelated.  But if they are
> entirely unrelated it shouldn't be a series to start with..

One thing I didn't realize until willy pointed this out separately is
that some of the list processing softwares will silently ignore an email
if it has too many entries (~10) in the to/cc list, because spam
heuristics.  I think vger/linux.dev is fairly forgiving about that, but
indie listservs might not be, and that adds friction to treewide
changes.

At least the whole series made it to fsdevel, but fsdevel is such a
firehose now that I can't keep up with it.  It's too bad that linux-xfs
can't simply mirror patchsets sent to mm/fsdevel with "xfs:" in the
title, and then I wouldn't have to look at the firehose.

All I'm really trying to say is, patchbombs are crap for collaboration.

--D

Re: [PATCH 27/30] iomap: Remove calls to set and clear folio error flag

[Darrick J. Wong]

On Sat, Apr 20, 2024 at 03:50:22AM +0100, Matthew Wilcox (Oracle) wrote:
> The folio error flag is not checked anywhere, so we can remove the calls
> to set and clear it.
> 
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Darrick J. Wong <djwong@kernel.org>
> Cc: linux-xfs@vger.kernel.org
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Looks fine to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/iomap/buffered-io.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 4e8e41c8b3c0..41352601f939 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -306,8 +306,6 @@ static void iomap_finish_folio_read(struct folio *folio, size_t off,
>  		spin_unlock_irqrestore(&ifs->state_lock, flags);
>  	}
>  
> -	if (error)
> -		folio_set_error(folio);
>  	if (finished)
>  		folio_end_read(folio, uptodate);
>  }
> @@ -460,9 +458,6 @@ int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops)
>  	while ((ret = iomap_iter(&iter, ops)) > 0)
>  		iter.processed = iomap_readpage_iter(&iter, &ctx, 0);
>  
> -	if (ret < 0)
> -		folio_set_error(folio);
> -
>  	if (ctx.bio) {
>  		submit_bio(ctx.bio);
>  		WARN_ON_ONCE(!ctx.cur_folio_in_bio);
> @@ -697,7 +692,6 @@ static int __iomap_write_begin(const struct iomap_iter *iter, loff_t pos,
>  
>  	if (folio_test_uptodate(folio))
>  		return 0;
> -	folio_clear_error(folio);
>  
>  	do {
>  		iomap_adjust_read_range(iter->inode, folio, &block_start,
> @@ -1528,8 +1522,6 @@ iomap_finish_ioend(struct iomap_ioend *ioend, int error)
>  
>  	/* walk all folios in bio, ending page IO on them */
>  	bio_for_each_folio_all(fi, bio) {
> -		if (error)
> -			folio_set_error(fi.folio);
>  		iomap_finish_folio_write(inode, fi.folio, fi.length);
>  		folio_count++;
>  	}
> -- 
> 2.43.0
> 
> 

[PATCH 28/30] buffer: Remove calls to set and clear the folio error flag

[Matthew Wilcox (Oracle)]

The folio error flag is not tested anywhere, so we can stop setting
and clearing it.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/buffer.c |  7 +------
 fs/mpage.c  | 13 +++----------
 2 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 592741d4b88a..26face6ccb3a 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -258,7 +258,6 @@ static void end_buffer_async_read(struct buffer_head *bh, int uptodate)
 	} else {
 		clear_buffer_uptodate(bh);
 		buffer_io_error(bh, ", async page read");
-		folio_set_error(folio);
 	}
 
 	/*
@@ -391,7 +390,6 @@ static void end_buffer_async_write(struct buffer_head *bh, int uptodate)
 		buffer_io_error(bh, ", lost async page write");
 		mark_buffer_write_io_error(bh);
 		clear_buffer_uptodate(bh);
-		folio_set_error(folio);
 	}
 
 	first = folio_buffers(folio);
@@ -1960,7 +1958,6 @@ int __block_write_full_folio(struct inode *inode, struct folio *folio,
 			clear_buffer_dirty(bh);
 		}
 	} while ((bh = bh->b_this_page) != head);
-	folio_set_error(folio);
 	BUG_ON(folio_test_writeback(folio));
 	mapping_set_error(folio->mapping, err);
 	folio_start_writeback(folio);
@@ -2406,10 +2403,8 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block)
 			if (iblock < lblock) {
 				WARN_ON(bh->b_size != blocksize);
 				err = get_block(inode, iblock, bh, 0);
-				if (err) {
-					folio_set_error(folio);
+				if (err)
 					page_error = true;
-				}
 			}
 			if (!buffer_mapped(bh)) {
 				folio_zero_range(folio, i * blocksize,
diff --git a/fs/mpage.c b/fs/mpage.c
index fa8b99a199fa..b5b5ddf9d513 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -48,13 +48,8 @@ static void mpage_read_end_io(struct bio *bio)
 	struct folio_iter fi;
 	int err = blk_status_to_errno(bio->bi_status);
 
-	bio_for_each_folio_all(fi, bio) {
-		if (err)
-			folio_set_error(fi.folio);
-		else
-			folio_mark_uptodate(fi.folio);
-		folio_unlock(fi.folio);
-	}
+	bio_for_each_folio_all(fi, bio)
+		folio_end_read(fi.folio, err == 0);
 
 	bio_put(bio);
 }
@@ -65,10 +60,8 @@ static void mpage_write_end_io(struct bio *bio)
 	int err = blk_status_to_errno(bio->bi_status);
 
 	bio_for_each_folio_all(fi, bio) {
-		if (err) {
-			folio_set_error(fi.folio);
+		if (err)
 			mapping_set_error(fi.folio->mapping, err);
-		}
 		folio_end_writeback(fi.folio);
 	}
 
-- 
2.43.0

[PATCH 29/30] fs: Remove calls to set and clear the folio error flag

[Matthew Wilcox (Oracle)]

Nobody checks the folio error flag any more, so we can stop setting
and clearing it.  Also remove the documentation suggesting to not
bother setting the error bit.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 Documentation/filesystems/vfs.rst | 3 +--
 mm/filemap.c                      | 8 --------
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/Documentation/filesystems/vfs.rst b/Documentation/filesystems/vfs.rst
index 6e903a903f8f..a6022ec59a2d 100644
--- a/Documentation/filesystems/vfs.rst
+++ b/Documentation/filesystems/vfs.rst
@@ -913,8 +913,7 @@ cache in your filesystem.  The following members are defined:
 	stop attempting I/O, it can simply return.  The caller will
 	remove the remaining pages from the address space, unlock them
 	and decrement the page refcount.  Set PageUptodate if the I/O
-	completes successfully.  Setting PageError on any page will be
-	ignored; simply unlock the page if an I/O error occurs.
+	completes successfully.
 
 ``write_begin``
 	Called by the generic buffered write code to ask the filesystem
diff --git a/mm/filemap.c b/mm/filemap.c
index fc784259f278..f8d0cc980044 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -530,7 +530,6 @@ static void __filemap_fdatawait_range(struct address_space *mapping,
 			struct folio *folio = fbatch.folios[i];
 
 			folio_wait_writeback(folio);
-			folio_clear_error(folio);
 		}
 		folio_batch_release(&fbatch);
 		cond_resched();
@@ -2342,13 +2341,6 @@ static int filemap_read_folio(struct file *file, filler_t filler,
 	unsigned long pflags;
 	int error;
 
-	/*
-	 * A previous I/O error may have been due to temporary failures,
-	 * eg. multipath errors.  PG_error will be set again if read_folio
-	 * fails.
-	 */
-	folio_clear_error(folio);
-
 	/* Start the actual read. The read will unlock the page. */
 	if (unlikely(workingset))
 		psi_memstall_enter(&pflags);
-- 
2.43.0

[PATCH 30/30] mm: Remove PG_error

[Matthew Wilcox (Oracle)]

The PG_error bit is now unused; delete it and free up a bit in
page->flags.

Cc: linux-mm@kvack.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/proc/page.c                         | 1 -
 include/linux/page-flags.h             | 6 +-----
 include/trace/events/mmflags.h         | 1 -
 include/uapi/linux/kernel-page-flags.h | 2 +-
 mm/migrate.c                           | 2 --
 5 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/fs/proc/page.c b/fs/proc/page.c
index 2fb64bdb64eb..05eb6617668a 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -186,7 +186,6 @@ u64 stable_page_flags(const struct page *page)
 #endif
 
 	u |= kpf_copy_bit(k, KPF_LOCKED,	PG_locked);
-	u |= kpf_copy_bit(k, KPF_ERROR,		PG_error);
 	u |= kpf_copy_bit(k, KPF_DIRTY,		PG_dirty);
 	u |= kpf_copy_bit(k, KPF_UPTODATE,	PG_uptodate);
 	u |= kpf_copy_bit(k, KPF_WRITEBACK,	PG_writeback);
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index a9a6e0f7367e..931820c8ea95 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -71,8 +71,6 @@
  * PG_referenced, PG_reclaim are used for page reclaim for anonymous and
  * file-backed pagecache (see mm/vmscan.c).
  *
- * PG_error is set to indicate that an I/O error occurred on this page.
- *
  * PG_arch_1 is an architecture specific page state bit.  The generic code
  * guarantees that this bit is cleared for a page when it first is entered into
  * the page cache.
@@ -108,7 +106,6 @@ enum pageflags {
 	PG_waiters,		/* Page has waiters, check its waitqueue. Must be bit #7 and in the same byte as "PG_locked" */
 	PG_active,
 	PG_workingset,
-	PG_error,
 	PG_owner_priv_1,	/* Owner use. If pagecache, fs may use*/
 	PG_arch_1,
 	PG_reserved,
@@ -188,7 +185,7 @@ enum pageflags {
 	 */
 
 	/* At least one page in this folio has the hwpoison flag set */
-	PG_has_hwpoisoned = PG_error,
+	PG_has_hwpoisoned = PG_active,
 	PG_large_rmappable = PG_workingset, /* anon or file-backed */
 };
 
@@ -511,7 +508,6 @@ static inline int TestClearPage##uname(struct page *page) { return 0; }
 
 __PAGEFLAG(Locked, locked, PF_NO_TAIL)
 FOLIO_FLAG(waiters, FOLIO_HEAD_PAGE)
-PAGEFLAG(Error, error, PF_NO_TAIL) TESTCLEARFLAG(Error, error, PF_NO_TAIL)
 PAGEFLAG(Referenced, referenced, PF_HEAD)
 	TESTCLEARFLAG(Referenced, referenced, PF_HEAD)
 	__SETPAGEFLAG(Referenced, referenced, PF_HEAD)
diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
index e46d6e82765e..4f2a18a11970 100644
--- a/include/trace/events/mmflags.h
+++ b/include/trace/events/mmflags.h
@@ -100,7 +100,6 @@
 #define __def_pageflag_names						\
 	DEF_PAGEFLAG_NAME(locked),					\
 	DEF_PAGEFLAG_NAME(waiters),					\
-	DEF_PAGEFLAG_NAME(error),					\
 	DEF_PAGEFLAG_NAME(referenced),					\
 	DEF_PAGEFLAG_NAME(uptodate),					\
 	DEF_PAGEFLAG_NAME(dirty),					\
diff --git a/include/uapi/linux/kernel-page-flags.h b/include/uapi/linux/kernel-page-flags.h
index 6f2f2720f3ac..ff8032227876 100644
--- a/include/uapi/linux/kernel-page-flags.h
+++ b/include/uapi/linux/kernel-page-flags.h
@@ -7,7 +7,7 @@
  */
 
 #define KPF_LOCKED		0
-#define KPF_ERROR		1
+#define KPF_ERROR		1	/* Now unused */
 #define KPF_REFERENCED		2
 #define KPF_UPTODATE		3
 #define KPF_DIRTY		4
diff --git a/mm/migrate.c b/mm/migrate.c
index 2594a00279f1..c0acb52ac4e4 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -561,8 +561,6 @@ void folio_migrate_flags(struct folio *newfolio, struct folio *folio)
 {
 	int cpupid;
 
-	if (folio_test_error(folio))
-		folio_set_error(newfolio);
 	if (folio_test_referenced(folio))
 		folio_set_referenced(newfolio);
 	if (folio_test_uptodate(folio))
-- 
2.43.0

Re: [PATCH 00/30] Remove PG_error flag

[Theodore Ts'o]

On Sat, 20 Apr 2024 03:49:55 +0100, Matthew Wilcox (Oracle) wrote:
> We've been steadily reducing the number of places which rely on
> PG_error.  There are only two left, so the first five patches remove
> those dependencies.
> 
> Every patch after the jfs patch is independent, and can be taken by the
> respective maintainer immediately.  They might depend on patches I sent
> in the last week or two (eg jfs, ntfs3).
> 
> [...]

Applied, thanks!

[10/30] ext4: Remove calls to to set/clear the folio error flag
        commit: ea4fd933ab4310822e244af28d22ff63785dea0e

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>