[PATCH 0/2] Remove PG_error

[PATCH 0/2] Remove PG_error

[Matthew Wilcox (Oracle)]

Hi Linus,

There were four pulls I had to wait for before I could send this to you,
but PG_error is now unused and can be reclaimed.  It could wait until
next merge window, but I don't see the point.

The patches to do all this have been out for review since mid-April,
https://lore.kernel.org/linux-fsdevel/20240420025029.2166544-1-willy@infradead.org/
Current git with these patches builds with allmodconfig.

Matthew Wilcox (Oracle) (2):
  fs: Remove calls to set and clear the folio error flag
  mm: Remove PG_error

 Documentation/filesystems/vfs.rst | 3 +--
 include/linux/page-flags.h        | 6 +-----
 mm/filemap.c                      | 8 --------
 mm/migrate.c                      | 2 --
 mm/page_io.c                      | 4 +---
 5 files changed, 3 insertions(+), 20 deletions(-)

-- 
2.43.0

[PATCH 1/2] 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 --------
 mm/migrate.c                      | 2 --
 mm/page_io.c                      | 4 +---
 4 files changed, 2 insertions(+), 15 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 d62150418b91..c7bef2247a68 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);
diff --git a/mm/migrate.c b/mm/migrate.c
index e7296c0fb5d5..e4431c157bc9 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -585,8 +585,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))
diff --git a/mm/page_io.c b/mm/page_io.c
index ff8c99ee3af7..9d4edf8bc422 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -273,9 +273,7 @@ static void sio_write_complete(struct kiocb *iocb, long ret)
 		 * memory for allocating transmit buffers.
 		 * Mark the page dirty and avoid
 		 * folio_rotate_reclaimable but rate-limit the
-		 * messages but do not flag PageError like
-		 * the normal direct-to-bio case as it could
-		 * be temporary.
+		 * messages.
 		 */
 		pr_err_ratelimited("Write error %ld on dio swapfile (%llu)\n",
 				   ret, swap_dev_pos(page_swap_entry(page)));
-- 
2.43.0

[PATCH 2/2] 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>
---
 include/linux/page-flags.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 5769fe6e4950..a0a29bd092f8 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -66,8 +66,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.
@@ -103,7 +101,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,
@@ -183,7 +180,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 */
 };
 
@@ -506,7 +503,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)
 FOLIO_FLAG(referenced, FOLIO_HEAD_PAGE)
 	FOLIO_TEST_CLEAR_FLAG(referenced, FOLIO_HEAD_PAGE)
 	__FOLIO_SET_FLAG(referenced, FOLIO_HEAD_PAGE)
-- 
2.43.0

Re: [PATCH 0/2] Remove PG_error

[Linus Torvalds]

On Tue, 23 Jul 2024 at 18:05, Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
>
> There were four pulls I had to wait for before I could send this to you,
> but PG_error is now unused and can be reclaimed.  It could wait until
> next merge window, but I don't see the point.

Well, I was initially inclined to agree with that, but then I did a
test-compile, and that immediately notices that you never tested this
at all and that it's completely broken.

There's still uses of PG_error in fs/proc/page.c, and in
include/trace/events/mmflags.h, and neither of those two cases are
new. One is unchanged from 2009, and the other one has gone through a
few code movements and re-orgs but also goes back over a decade ago.

So clearly this needs to be in linux-next, and as such this very much
is "next merge window".

                 Linus

Re: [PATCH 2/2] mm: Remove PG_error

[kernel test robot]

Hi Matthew,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linux/master linus/master v6.10 next-20240724]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Matthew-Wilcox-Oracle/fs-Remove-calls-to-set-and-clear-the-folio-error-flag/20240724-111138
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20240724010550.1755992-3-willy%40infradead.org
patch subject: [PATCH 2/2] mm: Remove PG_error
config: parisc-allnoconfig (https://download.01.org/0day-ci/archive/20240725/202407250459.K6byaMoI-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240725/202407250459.K6byaMoI-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407250459.K6byaMoI-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from mm/debug.c:13:
>> include/trace/events/mmflags.h:98:44: error: 'PG_error' undeclared here (not in a function); did you mean 'ma_error'?
      98 | #define DEF_PAGEFLAG_NAME(_name) { 1UL <<  PG_##_name, __stringify(_name) }
         |                                            ^~~
   include/trace/events/mmflags.h:103:9: note: in expansion of macro 'DEF_PAGEFLAG_NAME'
     103 |         DEF_PAGEFLAG_NAME(error),                                       \
         |         ^~~~~~~~~~~~~~~~~
   mm/debug.c:35:9: note: in expansion of macro '__def_pageflag_names'
      35 |         __def_pageflag_names,
         |         ^~~~~~~~~~~~~~~~~~~~
--
   fs/proc/page.c: In function 'stable_page_flags':
>> fs/proc/page.c:185:49: error: 'PG_error' undeclared (first use in this function); did you mean 'ma_error'?
     185 |         u |= kpf_copy_bit(k, KPF_ERROR,         PG_error);
         |                                                 ^~~~~~~~
         |                                                 ma_error
   fs/proc/page.c:185:49: note: each undeclared identifier is reported only once for each function it appears in


vim +98 include/trace/events/mmflags.h

4beba9486abd2f8 Steven Price  2020-04-22  97  
e26fcc02c7f6c76 Hyeonggon Yoo 2023-01-30 @98  #define DEF_PAGEFLAG_NAME(_name) { 1UL <<  PG_##_name, __stringify(_name) }
e26fcc02c7f6c76 Hyeonggon Yoo 2023-01-30  99  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] mm: Remove PG_error

[kernel test robot]

Hi Matthew,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linux/master linus/master v6.10 next-20240724]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Matthew-Wilcox-Oracle/fs-Remove-calls-to-set-and-clear-the-folio-error-flag/20240724-111138
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20240724010550.1755992-3-willy%40infradead.org
patch subject: [PATCH 2/2] mm: Remove PG_error
config: s390-allnoconfig (https://download.01.org/0day-ci/archive/20240725/202407250522.sImVfZXf-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project ad154281230d83ee551e12d5be48bb956ef47ed3)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240725/202407250522.sImVfZXf-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407250522.sImVfZXf-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from mm/debug.c:10:
   In file included from include/linux/mm.h:2206:
   include/linux/vmstat.h:498:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     498 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   In file included from mm/debug.c:18:
   In file included from mm/internal.h:13:
   include/linux/mm_inline.h:47:41: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
      47 |         __mod_lruvec_state(lruvec, NR_LRU_BASE + lru, nr_pages);
         |                                    ~~~~~~~~~~~ ^ ~~~
   include/linux/mm_inline.h:49:22: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
      49 |                                 NR_ZONE_LRU_BASE + lru, nr_pages);
         |                                 ~~~~~~~~~~~~~~~~ ^ ~~~
>> mm/debug.c:35:2: error: use of undeclared identifier 'PG_error'; did you mean 'ma_error'?
      35 |         __def_pageflag_names,
         |         ^
   include/trace/events/mmflags.h:103:2: note: expanded from macro '__def_pageflag_names'
     103 |         DEF_PAGEFLAG_NAME(error),                                       \
         |         ^
   include/trace/events/mmflags.h:98:44: note: expanded from macro 'DEF_PAGEFLAG_NAME'
      98 | #define DEF_PAGEFLAG_NAME(_name) { 1UL <<  PG_##_name, __stringify(_name) }
         |                                            ^
   <scratch space>:35:1: note: expanded from here
      35 | PG_error
         | ^
   include/linux/maple_tree.h:383:2: note: 'ma_error' declared here
     383 |         ma_error,
         |         ^
   3 warnings and 1 error generated.
--
   In file included from fs/proc/page.c:2:
   In file included from include/linux/memblock.h:12:
   In file included from include/linux/mm.h:2206:
   include/linux/vmstat.h:498:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     498 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   In file included from fs/proc/page.c:2:
   In file included from include/linux/memblock.h:13:
   In file included from arch/s390/include/asm/dma.h:5:
   In file included from include/linux/io.h:14:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
         |                                                           ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
     102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
         |                                                      ^
   In file included from fs/proc/page.c:2:
   In file included from include/linux/memblock.h:13:
   In file included from arch/s390/include/asm/dma.h:5:
   In file included from include/linux/io.h:14:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
         |                                                           ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
     115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
         |                                                      ^
   In file included from fs/proc/page.c:2:
   In file included from include/linux/memblock.h:13:
   In file included from arch/s390/include/asm/dma.h:5:
   In file included from include/linux/io.h:14:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     693 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     701 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     709 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     718 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     727 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     736 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
>> fs/proc/page.c:185:35: error: use of undeclared identifier 'PG_error'; did you mean 'ma_error'?
     185 |         u |= kpf_copy_bit(k, KPF_ERROR,         PG_error);
         |                                                 ^~~~~~~~
         |                                                 ma_error
   include/linux/maple_tree.h:383:2: note: 'ma_error' declared here
     383 |         ma_error,
         |         ^
   13 warnings and 1 error generated.


vim +35 mm/debug.c

7cd12b4abfd2f8f Vlastimil Babka 2016-03-15  33  
edf14cdbf9a0e5a Vlastimil Babka 2016-03-15  34  const struct trace_print_flags pageflag_names[] = {
edf14cdbf9a0e5a Vlastimil Babka 2016-03-15 @35  	__def_pageflag_names,
edf14cdbf9a0e5a Vlastimil Babka 2016-03-15  36  	{0, NULL}
edf14cdbf9a0e5a Vlastimil Babka 2016-03-15  37  };
edf14cdbf9a0e5a Vlastimil Babka 2016-03-15  38  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki