[PATCH 0/6] Further set_page_dirty cleanups

[PATCH 0/6] Further set_page_dirty cleanups

[Matthew Wilcox (Oracle)]

Prompted by Christoph's recent patches, here are some more patches to
improve the state of set_page_dirty().  They're all from the folio tree,
so they've been tested to a certain extent.

Matthew Wilcox (Oracle) (6):
  mm/writeback: Move __set_page_dirty() to core mm
  mm/writeback: Use __set_page_dirty in __set_page_dirty_nobuffers
  iomap: Use __set_page_dirty_nobuffers
  fs: Remove anon_set_page_dirty()
  fs: Remove noop_set_page_dirty()
  mm: Move page dirtying prototypes from mm.h

 drivers/dax/device.c    |  2 +-
 fs/buffer.c             | 24 ------------------------
 fs/ext2/inode.c         |  2 +-
 fs/ext4/inode.c         |  2 +-
 fs/fuse/dax.c           |  3 ++-
 fs/gfs2/aops.c          |  2 +-
 fs/iomap/buffered-io.c  | 27 +--------------------------
 fs/libfs.c              | 27 +--------------------------
 fs/xfs/xfs_aops.c       |  4 ++--
 fs/zonefs/super.c       |  4 ++--
 include/linux/fs.h      |  1 -
 include/linux/iomap.h   |  1 -
 include/linux/mm.h      |  4 ----
 include/linux/pagemap.h |  4 ++++
 mm/page-writeback.c     | 37 +++++++++++++++++++++++++++----------
 15 files changed, 43 insertions(+), 101 deletions(-)

-- 
2.30.2

[PATCH 1/6] mm/writeback: Move __set_page_dirty() to core mm

[Matthew Wilcox (Oracle)]

Nothing in __set_page_dirty() is specific to buffer_head, so
move it to mm/page-writeback.c.  That removes the only caller of
account_page_dirtied() outside of page-writeback.c, so make it static.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/buffer.c         | 24 ------------------------
 include/linux/mm.h  |  1 -
 mm/page-writeback.c | 27 ++++++++++++++++++++++++++-
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 8c02e44f16f2..f5384cff7e0c 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -588,30 +588,6 @@ void mark_buffer_dirty_inode(struct buffer_head *bh, struct inode *inode)
 }
 EXPORT_SYMBOL(mark_buffer_dirty_inode);
 
-/*
- * Mark the page dirty, and set it dirty in the page cache, and mark the inode
- * dirty.
- *
- * If warn is true, then emit a warning if the page is not uptodate and has
- * not been truncated.
- *
- * The caller must hold lock_page_memcg().
- */
-void __set_page_dirty(struct page *page, struct address_space *mapping,
-			     int warn)
-{
-	unsigned long flags;
-
-	xa_lock_irqsave(&mapping->i_pages, flags);
-	if (page->mapping) {	/* Race with truncate? */
-		WARN_ON_ONCE(warn && !PageUptodate(page));
-		account_page_dirtied(page, mapping);
-		__xa_set_mark(&mapping->i_pages, page_index(page),
-				PAGECACHE_TAG_DIRTY);
-	}
-	xa_unlock_irqrestore(&mapping->i_pages, flags);
-}
-
 /*
  * Add a page to the dirty page list.
  *
diff --git a/include/linux/mm.h b/include/linux/mm.h
index d64f8a1284d9..1086b556961a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1858,7 +1858,6 @@ int __set_page_dirty_nobuffers(struct page *page);
 int __set_page_dirty_no_writeback(struct page *page);
 int redirty_page_for_writepage(struct writeback_control *wbc,
 				struct page *page);
-void account_page_dirtied(struct page *page, struct address_space *mapping);
 void account_page_cleaned(struct page *page, struct address_space *mapping,
 			  struct bdi_writeback *wb);
 int set_page_dirty(struct page *page);
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index f6ee085767cb..0c2c8355f97f 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2420,7 +2420,8 @@ int __set_page_dirty_no_writeback(struct page *page)
  *
  * NOTE: This relies on being atomic wrt interrupts.
  */
-void account_page_dirtied(struct page *page, struct address_space *mapping)
+static void account_page_dirtied(struct page *page,
+		struct address_space *mapping)
 {
 	struct inode *inode = mapping->host;
 
@@ -2461,6 +2462,30 @@ void account_page_cleaned(struct page *page, struct address_space *mapping,
 	}
 }
 
+/*
+ * Mark the page dirty, and set it dirty in the page cache, and mark the inode
+ * dirty.
+ *
+ * If warn is true, then emit a warning if the page is not uptodate and has
+ * not been truncated.
+ *
+ * The caller must hold lock_page_memcg().
+ */
+void __set_page_dirty(struct page *page, struct address_space *mapping,
+			     int warn)
+{
+	unsigned long flags;
+
+	xa_lock_irqsave(&mapping->i_pages, flags);
+	if (page->mapping) {	/* Race with truncate? */
+		WARN_ON_ONCE(warn && !PageUptodate(page));
+		account_page_dirtied(page, mapping);
+		__xa_set_mark(&mapping->i_pages, page_index(page),
+				PAGECACHE_TAG_DIRTY);
+	}
+	xa_unlock_irqrestore(&mapping->i_pages, flags);
+}
+
 /*
  * For address_spaces which do not use buffers.  Just tag the page as dirty in
  * the xarray.
-- 
2.30.2

Re: [PATCH 1/6] mm/writeback: Move __set_page_dirty() to core mm

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/6] mm/writeback: Move __set_page_dirty() to core mm

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 05:23:37PM +0100, Matthew Wilcox (Oracle) wrote:
> Nothing in __set_page_dirty() is specific to buffer_head, so
> move it to mm/page-writeback.c.  That removes the only caller of
> account_page_dirtied() outside of page-writeback.c, so make it static.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/buffer.c         | 24 ------------------------
>  include/linux/mm.h  |  1 -
>  mm/page-writeback.c | 27 ++++++++++++++++++++++++++-
>  3 files changed, 26 insertions(+), 26 deletions(-)
> 

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 1/6] mm/writeback: Move __set_page_dirty() to core mm

[Matthew Wilcox]

On Tue, Jun 15, 2021 at 05:23:37PM +0100, Matthew Wilcox (Oracle) wrote:
> -/*
> - * Mark the page dirty, and set it dirty in the page cache, and mark the inode
> - * dirty.
> - *
> - * If warn is true, then emit a warning if the page is not uptodate and has
> - * not been truncated.
> - *
> - * The caller must hold lock_page_memcg().
> - */

Checking against my folio tree, I found a bit of extra documentation
that I had added and didn't make it into this submission.  Let me know
if it's useful and if so I can submit it as a fixup patch:

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 73b937955cc1..2072787d9b44 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2466,7 +2466,11 @@ void account_page_cleaned(struct page *page, struct addre
ss_space *mapping,
  * If warn is true, then emit a warning if the page is not uptodate and has
  * not been truncated.
  *
- * The caller must hold lock_page_memcg().
+ * The caller must hold lock_page_memcg().  Most callers have the page
+ * locked.  A few have the page blocked from truncation through other
+ * means (eg zap_page_range() has it mapped and is holding the page table
+ * lock).  This can also be called from mark_buffer_dirty(), which I
+ * cannot prove is always protected against truncate.
  */
 void __set_page_dirty(struct page *page, struct address_space *mapping,
                             int warn)


... it's a bit "notes to self", so perhaps someone can clean it up.
In particular, someone who knows the buffer code better than I do can
prove that mark_buffer_dirty() is always protected against truncate.

[PATCH 2/6] mm/writeback: Use __set_page_dirty in __set_page_dirty_nobuffers

[Matthew Wilcox (Oracle)]

This is fundamentally the same code, so just call it instead of
duplicating it.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/page-writeback.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0c2c8355f97f..980a6cb9cbd9 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2503,20 +2503,12 @@ int __set_page_dirty_nobuffers(struct page *page)
 	lock_page_memcg(page);
 	if (!TestSetPageDirty(page)) {
 		struct address_space *mapping = page_mapping(page);
-		unsigned long flags;
 
 		if (!mapping) {
 			unlock_page_memcg(page);
 			return 1;
 		}
-
-		xa_lock_irqsave(&mapping->i_pages, flags);
-		BUG_ON(page_mapping(page) != mapping);
-		WARN_ON_ONCE(!PagePrivate(page) && !PageUptodate(page));
-		account_page_dirtied(page, mapping);
-		__xa_set_mark(&mapping->i_pages, page_index(page),
-				   PAGECACHE_TAG_DIRTY);
-		xa_unlock_irqrestore(&mapping->i_pages, flags);
+		__set_page_dirty(page, mapping, !PagePrivate(page));
 		unlock_page_memcg(page);
 
 		if (mapping->host) {
-- 
2.30.2

Re: [PATCH 2/6] mm/writeback: Use __set_page_dirty in __set_page_dirty_nobuffers

[Christoph Hellwig]

On Tue, Jun 15, 2021 at 05:23:38PM +0100, Matthew Wilcox (Oracle) wrote:
> This is fundamentally the same code, so just call it instead of
> duplicating it.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/6] mm/writeback: Use __set_page_dirty in __set_page_dirty_nobuffers

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 05:23:38PM +0100, Matthew Wilcox (Oracle) wrote:
> This is fundamentally the same code, so just call it instead of
> duplicating it.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/page-writeback.c | 10 +---------

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH 3/6] iomap: Use __set_page_dirty_nobuffers

[Matthew Wilcox (Oracle)]

The only difference between iomap_set_page_dirty() and
__set_page_dirty_nobuffers() is that the latter includes a debugging
check that a !Uptodate page has private data.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/gfs2/aops.c         |  2 +-
 fs/iomap/buffered-io.c | 27 +--------------------------
 fs/xfs/xfs_aops.c      |  2 +-
 fs/zonefs/super.c      |  2 +-
 include/linux/iomap.h  |  1 -
 5 files changed, 4 insertions(+), 30 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 50dd1771d00c..746b78c3a91d 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -784,7 +784,7 @@ static const struct address_space_operations gfs2_aops = {
 	.writepages = gfs2_writepages,
 	.readpage = gfs2_readpage,
 	.readahead = gfs2_readahead,
-	.set_page_dirty = iomap_set_page_dirty,
+	.set_page_dirty = __set_page_dirty_nobuffers,
 	.releasepage = iomap_releasepage,
 	.invalidatepage = iomap_invalidatepage,
 	.bmap = gfs2_bmap,
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 2bf4778f2098..41da4f14c00b 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -640,31 +640,6 @@ iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, unsigned flags,
 	return status;
 }
 
-int
-iomap_set_page_dirty(struct page *page)
-{
-	struct address_space *mapping = page_mapping(page);
-	int newly_dirty;
-
-	if (unlikely(!mapping))
-		return !TestSetPageDirty(page);
-
-	/*
-	 * Lock out page's memcg migration to keep PageDirty
-	 * synchronized with per-memcg dirty page counters.
-	 */
-	lock_page_memcg(page);
-	newly_dirty = !TestSetPageDirty(page);
-	if (newly_dirty)
-		__set_page_dirty(page, mapping, 0);
-	unlock_page_memcg(page);
-
-	if (newly_dirty)
-		__mark_inode_dirty(mapping->host, I_DIRTY_PAGES);
-	return newly_dirty;
-}
-EXPORT_SYMBOL_GPL(iomap_set_page_dirty);
-
 static size_t __iomap_write_end(struct inode *inode, loff_t pos, size_t len,
 		size_t copied, struct page *page)
 {
@@ -684,7 +659,7 @@ static size_t __iomap_write_end(struct inode *inode, loff_t pos, size_t len,
 	if (unlikely(copied < len && !PageUptodate(page)))
 		return 0;
 	iomap_set_range_uptodate(page, offset_in_page(pos), len);
-	iomap_set_page_dirty(page);
+	__set_page_dirty_nobuffers(page);
 	return copied;
 }
 
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 826caa6b4a5a..a335d79dcff8 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -561,7 +561,7 @@ const struct address_space_operations xfs_address_space_operations = {
 	.readahead		= xfs_vm_readahead,
 	.writepage		= xfs_vm_writepage,
 	.writepages		= xfs_vm_writepages,
-	.set_page_dirty		= iomap_set_page_dirty,
+	.set_page_dirty		= __set_page_dirty_nobuffers,
 	.releasepage		= iomap_releasepage,
 	.invalidatepage		= iomap_invalidatepage,
 	.bmap			= xfs_vm_bmap,
diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
index cd145d318b17..3aacf016c7c2 100644
--- a/fs/zonefs/super.c
+++ b/fs/zonefs/super.c
@@ -185,7 +185,7 @@ static const struct address_space_operations zonefs_file_aops = {
 	.readahead		= zonefs_readahead,
 	.writepage		= zonefs_writepage,
 	.writepages		= zonefs_writepages,
-	.set_page_dirty		= iomap_set_page_dirty,
+	.set_page_dirty		= __set_page_dirty_nobuffers,
 	.releasepage		= iomap_releasepage,
 	.invalidatepage		= iomap_invalidatepage,
 	.migratepage		= iomap_migrate_page,
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index c87d0cb0de6d..479c1da3e221 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -159,7 +159,6 @@ ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from,
 		const struct iomap_ops *ops);
 int iomap_readpage(struct page *page, const struct iomap_ops *ops);
 void iomap_readahead(struct readahead_control *, const struct iomap_ops *ops);
-int iomap_set_page_dirty(struct page *page);
 int iomap_is_partially_uptodate(struct page *page, unsigned long from,
 		unsigned long count);
 int iomap_releasepage(struct page *page, gfp_t gfp_mask);
-- 
2.30.2

Re: [PATCH 3/6] iomap: Use __set_page_dirty_nobuffers

[Christoph Hellwig]

On Tue, Jun 15, 2021 at 05:23:39PM +0100, Matthew Wilcox (Oracle) wrote:
> The only difference between iomap_set_page_dirty() and
> __set_page_dirty_nobuffers() is that the latter includes a debugging
> check that a !Uptodate page has private data.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/6] iomap: Use __set_page_dirty_nobuffers

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 05:23:39PM +0100, Matthew Wilcox (Oracle) wrote:
> The only difference between iomap_set_page_dirty() and
> __set_page_dirty_nobuffers() is that the latter includes a debugging
> check that a !Uptodate page has private data.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/gfs2/aops.c         |  2 +-
>  fs/iomap/buffered-io.c | 27 +--------------------------
>  fs/xfs/xfs_aops.c      |  2 +-
>  fs/zonefs/super.c      |  2 +-
>  include/linux/iomap.h  |  1 -
>  5 files changed, 4 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
> index 50dd1771d00c..746b78c3a91d 100644
> --- a/fs/gfs2/aops.c
> +++ b/fs/gfs2/aops.c
> @@ -784,7 +784,7 @@ static const struct address_space_operations gfs2_aops = {
>  	.writepages = gfs2_writepages,
>  	.readpage = gfs2_readpage,
>  	.readahead = gfs2_readahead,
> -	.set_page_dirty = iomap_set_page_dirty,
> +	.set_page_dirty = __set_page_dirty_nobuffers,

Using __ functions in structures in different modules feels odd to me.
Why not just have iomap_set_page_dirty be a #define to this function now
if you want to do this?

Or take the __ off of the function name?

Anyway, logic here is fine, but feels odd.

greg k-h

Re: [PATCH 3/6] iomap: Use __set_page_dirty_nobuffers

[Matthew Wilcox]

On Tue, Jun 15, 2021 at 07:19:59PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 15, 2021 at 05:23:39PM +0100, Matthew Wilcox (Oracle) wrote:
> Using __ functions in structures in different modules feels odd to me.
> Why not just have iomap_set_page_dirty be a #define to this function now
> if you want to do this?
> 
> Or take the __ off of the function name?
> 
> Anyway, logic here is fine, but feels odd.

heh, that was how I did it the first time.  Then I thought that it was
better to follow Christoph's patch:

 static const struct address_space_operations adfs_aops = {
+       .set_page_dirty = __set_page_dirty_buffers,
(etc)

Re: [PATCH 3/6] iomap: Use __set_page_dirty_nobuffers

[Christoph Hellwig]

On Tue, Jun 15, 2021 at 06:32:37PM +0100, Matthew Wilcox wrote:
> On Tue, Jun 15, 2021 at 07:19:59PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 15, 2021 at 05:23:39PM +0100, Matthew Wilcox (Oracle) wrote:
> > Using __ functions in structures in different modules feels odd to me.
> > Why not just have iomap_set_page_dirty be a #define to this function now
> > if you want to do this?
> > 
> > Or take the __ off of the function name?
> > 
> > Anyway, logic here is fine, but feels odd.
> 
> heh, that was how I did it the first time.  Then I thought that it was
> better to follow Christoph's patch:
> 
>  static const struct address_space_operations adfs_aops = {
> +       .set_page_dirty = __set_page_dirty_buffers,
> (etc)

Eventually everything around set_page_dirty should be changed to operate
on folios, and that will be a good time to come up with a sane
naming scheme without introducing extra churn.

Re: [PATCH 3/6] iomap: Use __set_page_dirty_nobuffers

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 07:34:53PM +0200, Christoph Hellwig wrote:
> On Tue, Jun 15, 2021 at 06:32:37PM +0100, Matthew Wilcox wrote:
> > On Tue, Jun 15, 2021 at 07:19:59PM +0200, Greg Kroah-Hartman wrote:
> > > On Tue, Jun 15, 2021 at 05:23:39PM +0100, Matthew Wilcox (Oracle) wrote:
> > > Using __ functions in structures in different modules feels odd to me.
> > > Why not just have iomap_set_page_dirty be a #define to this function now
> > > if you want to do this?
> > > 
> > > Or take the __ off of the function name?
> > > 
> > > Anyway, logic here is fine, but feels odd.
> > 
> > heh, that was how I did it the first time.  Then I thought that it was
> > better to follow Christoph's patch:
> > 
> >  static const struct address_space_operations adfs_aops = {
> > +       .set_page_dirty = __set_page_dirty_buffers,
> > (etc)
> 
> Eventually everything around set_page_dirty should be changed to operate
> on folios, and that will be a good time to come up with a sane
> naming scheme without introducing extra churn.

Ok, that's fine, I don't normally touch these files, so it's not an
issue for me :)

Re: [PATCH 3/6] iomap: Use __set_page_dirty_nobuffers

[Matthew Wilcox]

On Tue, Jun 15, 2021 at 07:34:53PM +0200, Christoph Hellwig wrote:
> On Tue, Jun 15, 2021 at 06:32:37PM +0100, Matthew Wilcox wrote:
> > On Tue, Jun 15, 2021 at 07:19:59PM +0200, Greg Kroah-Hartman wrote:
> > > On Tue, Jun 15, 2021 at 05:23:39PM +0100, Matthew Wilcox (Oracle) wrote:
> > > Using __ functions in structures in different modules feels odd to me.
> > > Why not just have iomap_set_page_dirty be a #define to this function now
> > > if you want to do this?
> > > 
> > > Or take the __ off of the function name?
> > > 
> > > Anyway, logic here is fine, but feels odd.
> > 
> > heh, that was how I did it the first time.  Then I thought that it was
> > better to follow Christoph's patch:
> > 
> >  static const struct address_space_operations adfs_aops = {
> > +       .set_page_dirty = __set_page_dirty_buffers,
> > (etc)
> 
> Eventually everything around set_page_dirty should be changed to operate
> on folios, and that will be a good time to come up with a sane
> naming scheme without introducing extra churn.

The way it currently looks in my tree ...

set_page_dirty(page) is a thin wrapper that calls folio_mark_dirty(folio).
folio_mark_dirty() calls a_ops->dirty_folio(mapping, folio) (which
	returns bool).
__set_page_dirty_nobuffers() becomes filemap_dirty_folio()
__set_page_dirty_buffers() becomes block_dirty_folio()
__set_page_dirty_no_writeback() becomes dirty_folio_no_writeback()

Now I look at it, maybe that last should be nowb_dirty_folio().

Re: [PATCH 3/6] iomap: Use __set_page_dirty_nobuffers

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 07:13:16PM +0100, Matthew Wilcox wrote:
> On Tue, Jun 15, 2021 at 07:34:53PM +0200, Christoph Hellwig wrote:
> > On Tue, Jun 15, 2021 at 06:32:37PM +0100, Matthew Wilcox wrote:
> > > On Tue, Jun 15, 2021 at 07:19:59PM +0200, Greg Kroah-Hartman wrote:
> > > > On Tue, Jun 15, 2021 at 05:23:39PM +0100, Matthew Wilcox (Oracle) wrote:
> > > > Using __ functions in structures in different modules feels odd to me.
> > > > Why not just have iomap_set_page_dirty be a #define to this function now
> > > > if you want to do this?
> > > > 
> > > > Or take the __ off of the function name?
> > > > 
> > > > Anyway, logic here is fine, but feels odd.
> > > 
> > > heh, that was how I did it the first time.  Then I thought that it was
> > > better to follow Christoph's patch:
> > > 
> > >  static const struct address_space_operations adfs_aops = {
> > > +       .set_page_dirty = __set_page_dirty_buffers,
> > > (etc)
> > 
> > Eventually everything around set_page_dirty should be changed to operate
> > on folios, and that will be a good time to come up with a sane
> > naming scheme without introducing extra churn.
> 
> The way it currently looks in my tree ...
> 
> set_page_dirty(page) is a thin wrapper that calls folio_mark_dirty(folio).
> folio_mark_dirty() calls a_ops->dirty_folio(mapping, folio) (which
> 	returns bool).
> __set_page_dirty_nobuffers() becomes filemap_dirty_folio()
> __set_page_dirty_buffers() becomes block_dirty_folio()
> __set_page_dirty_no_writeback() becomes dirty_folio_no_writeback()
> 
> Now I look at it, maybe that last should be nowb_dirty_folio().

Not to be a pain, but you are mixing "folio" at the front and back of
the api name?  We messed up in the driver core with this for some things
(get_device() being one), I would recommend just sticking with one
naming scheme now as you are getting to pick what you want to use.

So perhaps for the above:
	folio_mark_dirty()
	folio_dirty_no_writeback()
	folio_dirty_filemap()
	folio_dirty_block()

much like "set_page" is used today?

Anyway, just bikeshedding, it's your code, your choice :)

thanks for doing this work overall.

greg k-h

Re: [PATCH 3/6] iomap: Use __set_page_dirty_nobuffers

[Matthew Wilcox]

On Wed, Jun 16, 2021 at 08:50:40AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 15, 2021 at 07:13:16PM +0100, Matthew Wilcox wrote:
> > On Tue, Jun 15, 2021 at 07:34:53PM +0200, Christoph Hellwig wrote:
> > > Eventually everything around set_page_dirty should be changed to operate
> > > on folios, and that will be a good time to come up with a sane
> > > naming scheme without introducing extra churn.
> > 
> > The way it currently looks in my tree ...
> > 
> > set_page_dirty(page) is a thin wrapper that calls folio_mark_dirty(folio).
> > folio_mark_dirty() calls a_ops->dirty_folio(mapping, folio) (which
> > 	returns bool).
> > __set_page_dirty_nobuffers() becomes filemap_dirty_folio()
> > __set_page_dirty_buffers() becomes block_dirty_folio()
> > __set_page_dirty_no_writeback() becomes dirty_folio_no_writeback()
> > 
> > Now I look at it, maybe that last should be nowb_dirty_folio().
> 
> Not to be a pain, but you are mixing "folio" at the front and back of
> the api name?  We messed up in the driver core with this for some things
> (get_device() being one), I would recommend just sticking with one
> naming scheme now as you are getting to pick what you want to use.

That is mostly what I'm doing.  eg,

get_page -> folio_get
lock_page -> folio_lock
PageUptodate -> folio_uptodate
set_page_dirty -> folio_mark_dirty

What I haven't dealt with yet is the naming of the
address_space_operations.  My thinking with those is that they should
be verb_folio, since they _aren't_ the functions that get called.
ie it looks like this:

folio_mark_dirty()
  aops->dirty_folio()
    ext4_dirty_folio()
      buffer_dirty_folio()

I actually see the inconsistency here as a good thing -- these are
implementations of the aop, so foo_verb_folio() means you're doing
something weird and internal instead of going through the vfs/mm.

That implies doing things like renaming ->readpage to ->read_folio, but
if we're changing the API from passing a struct page to a struct folio,
that can all be done at the same time with no additional disruption.

Re: [PATCH 3/6] iomap: Use __set_page_dirty_nobuffers

[Greg Kroah-Hartman]

On Wed, Jun 16, 2021 at 05:28:14PM +0100, Matthew Wilcox wrote:
> On Wed, Jun 16, 2021 at 08:50:40AM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 15, 2021 at 07:13:16PM +0100, Matthew Wilcox wrote:
> > > On Tue, Jun 15, 2021 at 07:34:53PM +0200, Christoph Hellwig wrote:
> > > > Eventually everything around set_page_dirty should be changed to operate
> > > > on folios, and that will be a good time to come up with a sane
> > > > naming scheme without introducing extra churn.
> > > 
> > > The way it currently looks in my tree ...
> > > 
> > > set_page_dirty(page) is a thin wrapper that calls folio_mark_dirty(folio).
> > > folio_mark_dirty() calls a_ops->dirty_folio(mapping, folio) (which
> > > 	returns bool).
> > > __set_page_dirty_nobuffers() becomes filemap_dirty_folio()
> > > __set_page_dirty_buffers() becomes block_dirty_folio()
> > > __set_page_dirty_no_writeback() becomes dirty_folio_no_writeback()
> > > 
> > > Now I look at it, maybe that last should be nowb_dirty_folio().
> > 
> > Not to be a pain, but you are mixing "folio" at the front and back of
> > the api name?  We messed up in the driver core with this for some things
> > (get_device() being one), I would recommend just sticking with one
> > naming scheme now as you are getting to pick what you want to use.
> 
> That is mostly what I'm doing.  eg,
> 
> get_page -> folio_get
> lock_page -> folio_lock
> PageUptodate -> folio_uptodate
> set_page_dirty -> folio_mark_dirty

Nice.

> What I haven't dealt with yet is the naming of the
> address_space_operations.  My thinking with those is that they should
> be verb_folio, since they _aren't_ the functions that get called.
> ie it looks like this:
> 
> folio_mark_dirty()
>   aops->dirty_folio()
>     ext4_dirty_folio()
>       buffer_dirty_folio()
> 
> I actually see the inconsistency here as a good thing -- these are
> implementations of the aop, so foo_verb_folio() means you're doing
> something weird and internal instead of going through the vfs/mm.
> 
> That implies doing things like renaming ->readpage to ->read_folio, but
> if we're changing the API from passing a struct page to a struct folio,
> that can all be done at the same time with no additional disruption.

Ok, as long as there's a reason for the naming scheme, I'm happy as
hopefully it will make sense to others as well.

thanks,

greg k-h

[PATCH 4/6] fs: Remove anon_set_page_dirty()

[Matthew Wilcox (Oracle)]

Use __set_page_dirty_no_writeback() instead.  This will set the dirty
bit on the page, which will be used to avoid calling set_page_dirty()
in the future.  It will have no effect on actually writing the page
back, as the pages are not on any LRU lists.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/libfs.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c
index 2d7f086b93d6..3fdd89b156d6 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1217,19 +1217,10 @@ void kfree_link(void *p)
 }
 EXPORT_SYMBOL(kfree_link);
 
-/*
- * nop .set_page_dirty method so that people can use .page_mkwrite on
- * anon inodes.
- */
-static int anon_set_page_dirty(struct page *page)
-{
-	return 0;
-};
-
 struct inode *alloc_anon_inode(struct super_block *s)
 {
 	static const struct address_space_operations anon_aops = {
-		.set_page_dirty = anon_set_page_dirty,
+		.set_page_dirty = __set_page_dirty_no_writeback,
 	};
 	struct inode *inode = new_inode_pseudo(s);
 
-- 
2.30.2

Re: [PATCH 4/6] fs: Remove anon_set_page_dirty()

[Christoph Hellwig]

On Tue, Jun 15, 2021 at 05:23:40PM +0100, Matthew Wilcox (Oracle) wrote:
> Use __set_page_dirty_no_writeback() instead.  This will set the dirty
> bit on the page, which will be used to avoid calling set_page_dirty()
> in the future.  It will have no effect on actually writing the page
> back, as the pages are not on any LRU lists.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 5/6] fs: Remove noop_set_page_dirty()

[Matthew Wilcox (Oracle)]

Use __set_page_dirty_no_writeback() instead.  This will set the dirty
bit on the page, which will be used to avoid calling set_page_dirty()
in the future.  It will have no effect on actually writing the page
back, as the pages are not on any LRU lists.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 drivers/dax/device.c |  2 +-
 fs/ext2/inode.c      |  2 +-
 fs/ext4/inode.c      |  2 +-
 fs/fuse/dax.c        |  2 +-
 fs/libfs.c           | 16 ----------------
 fs/xfs/xfs_aops.c    |  2 +-
 include/linux/fs.h   |  1 -
 7 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/dax/device.c b/drivers/dax/device.c
index db92573c94e8..dd8222a42808 100644
--- a/drivers/dax/device.c
+++ b/drivers/dax/device.c
@@ -337,7 +337,7 @@ static unsigned long dax_get_unmapped_area(struct file *filp,
 }
 
 static const struct address_space_operations dev_dax_aops = {
-	.set_page_dirty		= noop_set_page_dirty,
+	.set_page_dirty		= __set_page_dirty_no_writeback,
 	.invalidatepage		= noop_invalidatepage,
 };
 
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index bf41f579ed3e..dadb121beb22 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -992,7 +992,7 @@ const struct address_space_operations ext2_nobh_aops = {
 static const struct address_space_operations ext2_dax_aops = {
 	.writepages		= ext2_dax_writepages,
 	.direct_IO		= noop_direct_IO,
-	.set_page_dirty		= noop_set_page_dirty,
+	.set_page_dirty		= __set_page_dirty_no_writeback,
 	.invalidatepage		= noop_invalidatepage,
 };
 
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index f44800361a38..6dd58c14ef1f 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3701,7 +3701,7 @@ static const struct address_space_operations ext4_da_aops = {
 static const struct address_space_operations ext4_dax_aops = {
 	.writepages		= ext4_dax_writepages,
 	.direct_IO		= noop_direct_IO,
-	.set_page_dirty		= noop_set_page_dirty,
+	.set_page_dirty		= __set_page_dirty_no_writeback,
 	.bmap			= ext4_bmap,
 	.invalidatepage		= noop_invalidatepage,
 	.swap_activate		= ext4_iomap_swap_activate,
diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index ff99ab2a3c43..515ad0895345 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -1329,7 +1329,7 @@ bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi)
 static const struct address_space_operations fuse_dax_file_aops  = {
 	.writepages	= fuse_dax_writepages,
 	.direct_IO	= noop_direct_IO,
-	.set_page_dirty	= noop_set_page_dirty,
+	.set_page_dirty	= __set_page_dirty_no_writeback,
 	.invalidatepage	= noop_invalidatepage,
 };
 
diff --git a/fs/libfs.c b/fs/libfs.c
index 3fdd89b156d6..51b4de3b3447 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1171,22 +1171,6 @@ int noop_fsync(struct file *file, loff_t start, loff_t end, int datasync)
 }
 EXPORT_SYMBOL(noop_fsync);
 
-int noop_set_page_dirty(struct page *page)
-{
-	/*
-	 * Unlike __set_page_dirty_no_writeback that handles dirty page
-	 * tracking in the page object, dax does all dirty tracking in
-	 * the inode address_space in response to mkwrite faults. In the
-	 * dax case we only need to worry about potentially dirty CPU
-	 * caches, not dirty page cache pages to write back.
-	 *
-	 * This callback is defined to prevent fallback to
-	 * __set_page_dirty_buffers() in set_page_dirty().
-	 */
-	return 0;
-}
-EXPORT_SYMBOL_GPL(noop_set_page_dirty);
-
 void noop_invalidatepage(struct page *page, unsigned int offset,
 		unsigned int length)
 {
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index a335d79dcff8..cb4e0fcf4c76 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -575,7 +575,7 @@ const struct address_space_operations xfs_address_space_operations = {
 const struct address_space_operations xfs_dax_aops = {
 	.writepages		= xfs_dax_writepages,
 	.direct_IO		= noop_direct_IO,
-	.set_page_dirty		= noop_set_page_dirty,
+	.set_page_dirty		= __set_page_dirty_no_writeback,
 	.invalidatepage		= noop_invalidatepage,
 	.swap_activate		= xfs_iomap_swapfile_activate,
 };
diff --git a/include/linux/fs.h b/include/linux/fs.h
index ef5c1d93994c..9dfd1e15b0e1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3416,7 +3416,6 @@ extern int simple_rename(struct user_namespace *, struct inode *,
 extern void simple_recursive_removal(struct dentry *,
                               void (*callback)(struct dentry *));
 extern int noop_fsync(struct file *, loff_t, loff_t, int);
-extern int noop_set_page_dirty(struct page *page);
 extern void noop_invalidatepage(struct page *page, unsigned int offset,
 		unsigned int length);
 extern ssize_t noop_direct_IO(struct kiocb *iocb, struct iov_iter *iter);
-- 
2.30.2

Re: [PATCH 5/6] fs: Remove noop_set_page_dirty()

[Christoph Hellwig]

On Tue, Jun 15, 2021 at 05:23:41PM +0100, Matthew Wilcox (Oracle) wrote:
> Use __set_page_dirty_no_writeback() instead.  This will set the dirty
> bit on the page, which will be used to avoid calling set_page_dirty()
> in the future.  It will have no effect on actually writing the page
> back, as the pages are not on any LRU lists.

Looks sane to me, but I'd really like to have Dan to look over this..

Re: [PATCH 5/6] fs: Remove noop_set_page_dirty()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1681 bytes --]

Hi "Matthew,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210615]
[cannot apply to hnaz-linux-mm/master xfs-linux/for-next fuse/for-next linus/master v5.13-rc6 v5.13-rc5 v5.13-rc4 v5.13-rc6]
[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]

url:    https://github.com/0day-ci/linux/commits/Matthew-Wilcox-Oracle/Further-set_page_dirty-cleanups/20210616-205504
base:    19ae1f2bd9c091059f80646604ccef8a1e614f57
config: m68k-defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9367b704615caea2752bf1ec461f19e891936c55
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Matthew-Wilcox-Oracle/Further-set_page_dirty-cleanups/20210616-205504
        git checkout 9367b704615caea2752bf1ec461f19e891936c55
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__set_page_dirty_no_writeback" [fs/xfs/xfs.ko] undefined!

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 17164 bytes --]

Re: [PATCH 5/6] fs: Remove noop_set_page_dirty()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]

Hi "Matthew,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210615]
[cannot apply to hnaz-linux-mm/master xfs-linux/for-next fuse/for-next linus/master v5.13-rc6 v5.13-rc5 v5.13-rc4 v5.13-rc6]
[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]

url:    https://github.com/0day-ci/linux/commits/Matthew-Wilcox-Oracle/Further-set_page_dirty-cleanups/20210616-205504
base:    19ae1f2bd9c091059f80646604ccef8a1e614f57
config: m68k-defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9367b704615caea2752bf1ec461f19e891936c55
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Matthew-Wilcox-Oracle/Further-set_page_dirty-cleanups/20210616-205504
        git checkout 9367b704615caea2752bf1ec461f19e891936c55
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__set_page_dirty_no_writeback" [fs/xfs/xfs.ko] undefined!

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 17164 bytes --]

Re: [PATCH 5/6] fs: Remove noop_set_page_dirty()

[Matthew Wilcox]

On Thu, Jun 17, 2021 at 02:10:04AM +0800, kernel test robot wrote:
> >> ERROR: modpost: "__set_page_dirty_no_writeback" [fs/xfs/xfs.ko] undefined!

Thanks; Andrew already fixed this one.

Re: [PATCH 5/6] fs: Remove noop_set_page_dirty()

[Matthew Wilcox]

[-- Attachment #1: Type: text/plain, Size: 191 bytes --]

On Thu, Jun 17, 2021 at 02:10:04AM +0800, kernel test robot wrote:
> >> ERROR: modpost: "__set_page_dirty_no_writeback" [fs/xfs/xfs.ko] undefined!

Thanks; Andrew already fixed this one.

Re: [PATCH 5/6] fs: Remove noop_set_page_dirty()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]

Hi "Matthew,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210615]
[cannot apply to hnaz-linux-mm/master xfs-linux/for-next fuse/for-next linus/master v5.13-rc6 v5.13-rc5 v5.13-rc4 v5.13-rc6]
[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]

url:    https://github.com/0day-ci/linux/commits/Matthew-Wilcox-Oracle/Further-set_page_dirty-cleanups/20210616-205504
base:    19ae1f2bd9c091059f80646604ccef8a1e614f57
config: i386-randconfig-a015-20210615 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/9367b704615caea2752bf1ec461f19e891936c55
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Matthew-Wilcox-Oracle/Further-set_page_dirty-cleanups/20210616-205504
        git checkout 9367b704615caea2752bf1ec461f19e891936c55
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__set_page_dirty_no_writeback" [fs/ext4/ext4.ko] undefined!

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36680 bytes --]

Re: [PATCH 5/6] fs: Remove noop_set_page_dirty()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1541 bytes --]

Hi "Matthew,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210615]
[cannot apply to hnaz-linux-mm/master xfs-linux/for-next fuse/for-next linus/master v5.13-rc6 v5.13-rc5 v5.13-rc4 v5.13-rc6]
[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]

url:    https://github.com/0day-ci/linux/commits/Matthew-Wilcox-Oracle/Further-set_page_dirty-cleanups/20210616-205504
base:    19ae1f2bd9c091059f80646604ccef8a1e614f57
config: i386-randconfig-a015-20210615 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/9367b704615caea2752bf1ec461f19e891936c55
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Matthew-Wilcox-Oracle/Further-set_page_dirty-cleanups/20210616-205504
        git checkout 9367b704615caea2752bf1ec461f19e891936c55
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__set_page_dirty_no_writeback" [fs/ext4/ext4.ko] undefined!

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36680 bytes --]

[PATCH 6/6] mm: Move page dirtying prototypes from mm.h

[Matthew Wilcox (Oracle)]

These functions implement the address_space ->set_page_dirty operation
and should live in pagemap.h, not mm.h so that the rest of the kernel
doesn't get funny ideas about calling them directly.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/fuse/dax.c           | 1 +
 fs/zonefs/super.c       | 2 +-
 include/linux/mm.h      | 3 ---
 include/linux/pagemap.h | 4 ++++
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 515ad0895345..fb733eb5aead 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -9,6 +9,7 @@
 #include <linux/delay.h>
 #include <linux/dax.h>
 #include <linux/uio.h>
+#include <linux/pagemap.h>
 #include <linux/pfn_t.h>
 #include <linux/iomap.h>
 #include <linux/interval_tree.h>
diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
index 3aacf016c7c2..dbf03635869c 100644
--- a/fs/zonefs/super.c
+++ b/fs/zonefs/super.c
@@ -5,7 +5,7 @@
  * Copyright (C) 2019 Western Digital Corporation or its affiliates.
  */
 #include <linux/module.h>
-#include <linux/fs.h>
+#include <linux/pagemap.h>
 #include <linux/magic.h>
 #include <linux/iomap.h>
 #include <linux/init.h>
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1086b556961a..5608f75ecc80 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1853,9 +1853,6 @@ extern int try_to_release_page(struct page * page, gfp_t gfp_mask);
 extern void do_invalidatepage(struct page *page, unsigned int offset,
 			      unsigned int length);
 
-void __set_page_dirty(struct page *, struct address_space *, int warn);
-int __set_page_dirty_nobuffers(struct page *page);
-int __set_page_dirty_no_writeback(struct page *page);
 int redirty_page_for_writepage(struct writeback_control *wbc,
 				struct page *page);
 void account_page_cleaned(struct page *page, struct address_space *mapping,
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 5f0582de24e7..090e45616172 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -752,6 +752,10 @@ int wait_on_page_writeback_killable(struct page *page);
 extern void end_page_writeback(struct page *page);
 void wait_for_stable_page(struct page *page);
 
+void __set_page_dirty(struct page *, struct address_space *, int warn);
+int __set_page_dirty_nobuffers(struct page *page);
+int __set_page_dirty_no_writeback(struct page *page);
+
 void page_endio(struct page *page, bool is_write, int err);
 
 /**
-- 
2.30.2

Re: [PATCH 6/6] mm: Move page dirtying prototypes from mm.h

[Christoph Hellwig]

On Tue, Jun 15, 2021 at 05:23:42PM +0100, Matthew Wilcox (Oracle) wrote:
> These functions implement the address_space ->set_page_dirty operation
> and should live in pagemap.h, not mm.h so that the rest of the kernel
> doesn't get funny ideas about calling them directly.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>