fs/udf/inode.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
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
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
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
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
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
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
© 2016 - 2025 Red Hat, Inc.