linux-next: build failure after merge of the mm tree

Stephen Rothwell posted 1 patch 2 years, 7 months ago
There is a newer version of this series
fs/udf/inode.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
linux-next: build failure after merge of the mm tree
Posted by Stephen Rothwell 2 years, 7 months ago
Hi all,

After merging the mm tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

fs/udf/inode.c: In function 'udf_writepages':
fs/udf/inode.c:211:48: error: passing argument 3 of 'write_cache_pages' from incompatible pointer type [-Werror=incompatible-pointer-types]
  211 |         return write_cache_pages(mapping, wbc, udf_adinicb_writepage, NULL);
      |                                                ^~~~~~~~~~~~~~~~~~~~~
      |                                                |
      |                                                int (*)(struct page *, struct writeback_control *, void *)
In file included from fs/udf/inode.c:36:
include/linux/writeback.h:375:66: note: expected 'writepage_t' {aka 'int (*)(struct folio *, struct writeback_control *, void *)'} but argument is of type 'int (*)(struct page *, struct writeback_control *, void *)'
  375 |                       struct writeback_control *wbc, writepage_t writepage,
      |                                                      ~~~~~~~~~~~~^~~~~~~~~

Caused by commit

  a36a897cc496 ("fs: convert writepage_t callback to pass a folio")

interacting with commit

  79d3c6dbada4 ("udf: Convert in-ICB files to use udf_writepages()")

from the ext3 tree.

I have applied the following merge fix patch (I wasn't sure what to do
with the PageLocked()).

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 27 Jan 2023 16:50:34 +1100
Subject: [PATCH] udf: fix up for "fs: convert writepage_t callback to pass a folio"

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/udf/inode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 3b2adf4cbc57..b47bf9c73f4d 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -185,16 +185,17 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
 	}
 }
 
-static int udf_adinicb_writepage(struct page *page,
+static int udf_adinicb_writepage(struct folio *folio,
 				 struct writeback_control *wbc, void *data)
 {
+	struct page *page = &folio->page;
 	struct inode *inode = page->mapping->host;
 	struct udf_inode_info *iinfo = UDF_I(inode);
 
-	BUG_ON(!PageLocked(page));
+//	BUG_ON(!PageLocked(page));
 	memcpy_to_page(page, 0, iinfo->i_data + iinfo->i_lenEAttr,
 		       i_size_read(inode));
-	unlock_page(page);
+	folio_unlock(folio);
 	mark_inode_dirty(inode);
 
 	return 0;
-- 
2.35.1

-- 
Cheers,
Stephen Rothwell
Re: linux-next: build failure after merge of the mm tree
Posted by Jan Kara 2 years, 7 months ago
On Fri 27-01-23 16:59:12, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the mm tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
> 
> fs/udf/inode.c: In function 'udf_writepages':
> fs/udf/inode.c:211:48: error: passing argument 3 of 'write_cache_pages' from incompatible pointer type [-Werror=incompatible-pointer-types]
>   211 |         return write_cache_pages(mapping, wbc, udf_adinicb_writepage, NULL);
>       |                                                ^~~~~~~~~~~~~~~~~~~~~
>       |                                                |
>       |                                                int (*)(struct page *, struct writeback_control *, void *)
> In file included from fs/udf/inode.c:36:
> include/linux/writeback.h:375:66: note: expected 'writepage_t' {aka 'int (*)(struct folio *, struct writeback_control *, void *)'} but argument is of type 'int (*)(struct page *, struct writeback_control *, void *)'
>   375 |                       struct writeback_control *wbc, writepage_t writepage,
>       |                                                      ~~~~~~~~~~~~^~~~~~~~~
> 
> Caused by commit
> 
>   a36a897cc496 ("fs: convert writepage_t callback to pass a folio")
> 
> interacting with commit
> 
>   79d3c6dbada4 ("udf: Convert in-ICB files to use udf_writepages()")
> 
> from the ext3 tree.
> 
> I have applied the following merge fix patch (I wasn't sure what to do
> with the PageLocked()).

Thanks for the fixup! The right function to replace PageLocked() with is
folio_test_locked(). Anyway, I'll prepare a suggested conflict resolution
for Linus when pushing the changes.

								Honza

> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Fri, 27 Jan 2023 16:50:34 +1100
> Subject: [PATCH] udf: fix up for "fs: convert writepage_t callback to pass a folio"
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  fs/udf/inode.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index 3b2adf4cbc57..b47bf9c73f4d 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -185,16 +185,17 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
>  	}
>  }
>  
> -static int udf_adinicb_writepage(struct page *page,
> +static int udf_adinicb_writepage(struct folio *folio,
>  				 struct writeback_control *wbc, void *data)
>  {
> +	struct page *page = &folio->page;
>  	struct inode *inode = page->mapping->host;
>  	struct udf_inode_info *iinfo = UDF_I(inode);
>  
> -	BUG_ON(!PageLocked(page));
> +//	BUG_ON(!PageLocked(page));
>  	memcpy_to_page(page, 0, iinfo->i_data + iinfo->i_lenEAttr,
>  		       i_size_read(inode));
> -	unlock_page(page);
> +	folio_unlock(folio);
>  	mark_inode_dirty(inode);
>  
>  	return 0;
> -- 
> 2.35.1
> 
> -- 
> Cheers,
> Stephen Rothwell


-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: linux-next: build failure after merge of the mm tree
Posted by Stephen Rothwell 2 years, 7 months ago
Hi Jan,

On Fri, 27 Jan 2023 14:11:42 +0100 Jan Kara <jack@suse.cz> wrote:
>
> Thanks for the fixup! The right function to replace PageLocked() with is
> folio_test_locked(). Anyway, I'll prepare a suggested conflict resolution
> for Linus when pushing the changes.

Thanks for the hint.  This is what I am using now:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 27 Jan 2023 16:50:34 +1100
Subject: [PATCH] udf: fix up for "fs: convert writepage_t callback to pass a folio"

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/udf/inode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 3b2adf4cbc57..b47bf9c73f4d 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -185,16 +185,17 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
 	}
 }
 
-static int udf_adinicb_writepage(struct page *page,
+static int udf_adinicb_writepage(struct folio *folio,
 				 struct writeback_control *wbc, void *data)
 {
+	struct page *page = &folio->page;
 	struct inode *inode = page->mapping->host;
 	struct udf_inode_info *iinfo = UDF_I(inode);
 
-	BUG_ON(!PageLocked(page));
+	BUG_ON(!folio_test_locked(folio));
 	memcpy_to_page(page, 0, iinfo->i_data + iinfo->i_lenEAttr,
 		       i_size_read(inode));
-	unlock_page(page);
+	folio_unlock(folio);
 	mark_inode_dirty(inode);
 
 	return 0;
-- 
2.35.1

-- 
Cheers,
Stephen Rothwell
Re: linux-next: build failure after merge of the mm tree
Posted by Stephen Rothwell 2 years, 6 months ago
Hi all,

On Wed, 1 Feb 2023 08:47:41 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Jan,
> 
> On Fri, 27 Jan 2023 14:11:42 +0100 Jan Kara <jack@suse.cz> wrote:
> >
> > Thanks for the fixup! The right function to replace PageLocked() with is
> > folio_test_locked(). Anyway, I'll prepare a suggested conflict resolution
> > for Linus when pushing the changes.  
> 
> Thanks for the hint.  This is what I am using now:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Fri, 27 Jan 2023 16:50:34 +1100
> Subject: [PATCH] udf: fix up for "fs: convert writepage_t callback to pass a folio"
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  fs/udf/inode.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index 3b2adf4cbc57..b47bf9c73f4d 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -185,16 +185,17 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
>  	}
>  }
>  
> -static int udf_adinicb_writepage(struct page *page,
> +static int udf_adinicb_writepage(struct folio *folio,
>  				 struct writeback_control *wbc, void *data)
>  {
> +	struct page *page = &folio->page;
>  	struct inode *inode = page->mapping->host;
>  	struct udf_inode_info *iinfo = UDF_I(inode);
>  
> -	BUG_ON(!PageLocked(page));
> +	BUG_ON(!folio_test_locked(folio));
>  	memcpy_to_page(page, 0, iinfo->i_data + iinfo->i_lenEAttr,
>  		       i_size_read(inode));
> -	unlock_page(page);
> +	folio_unlock(folio);
>  	mark_inode_dirty(inode);
>  
>  	return 0;
> -- 
> 2.35.1

I think Linus may have missed the last 2 changes in this merge
resolution ...
-- 
Cheers,
Stephen Rothwell
Re: linux-next: build failure after merge of the mm tree
Posted by Linus Torvalds 2 years, 6 months ago
On Thu, Feb 23, 2023 at 8:40 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> I think Linus may have missed the last 2 changes in this merge
> resolution ...

Well, not exactly missed, in that I didn't actually even look at it,
because that one wasn't one I worried about (unlike the cifs one that
I'm very leery of).

And I actually really dislike your particular resolution and wouldn't
prefer it done that way anyway. It mixes folios and pages in ugly
ways.

Either do it all with the page pointer (like I did), or convert it
*all* to be folios, but don't do that odd "use a mixture of both
intertwined".

Of course, I do see _why_ you are mixing 'page' and 'folio' - there's
no memcpy_to_folio() helper (although once it eventually exists it
might be called "memcpy_to_file_folio()" to match the naming of the
"from" version).

But that's exactly why I stopped where I stopped - I think it's
pointless doing the other conversions when you can't easily do that
memcpy_to_page() one.

So I think the UDF folio conversion needs a bit more infrastructure to
really work well.

(There's also the whole kmap issue - we don't kmap whole folios, only
individual pages, so a "real" folio conversion would have to have a
loop).

End result: I explicitly left it in its minimal form, because I think
anything else is a "future endeavor". The udf code only works with
page-sized folios, and pretending anything else (using
"folio_unlock()" etc) would be just that - pretending

               Linus
Re: linux-next: build failure after merge of the mm tree
Posted by Stephen Rothwell 2 years, 6 months ago
Hi Linus,

On Thu, 23 Feb 2023 22:01:25 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> End result: I explicitly left it in its minimal form, because I think
> anything else is a "future endeavor". The udf code only works with
> page-sized folios, and pretending anything else (using
> "folio_unlock()" etc) would be just that - pretending

OK, thanks.

-- 
Cheers,
Stephen Rothwell