| Message ID | 20240530202110.2653630-13-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Prepare to remove PG_error | expand |
Hi Mathew, On 31/5/24 06:21, Matthew Wilcox (Oracle) wrote: > 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> This is breaking for me on my m68k/ColdFire 5475 target. That is a full MMU CPU. The usual test build I do (m5475evb_defconfig) fails to boot, not being able to load and start init or sh at startup from a ROMfs resident in RAM within the uclinux MTD mapper: ... Freeing unused kernel image (initmem) memory: 80K This architecture does not have kernel memory protection. Run /sbin/init as init process Run /etc/init as init process Run /bin/init as init process Starting init: /bin/init exists but couldn't execute it (error -13) Run /bin/sh as init process Starting init: /bin/sh exists but couldn't execute it (error -13) Kernel panic - not syncing: No working init found. Try passing init= option to kernel. See Linux Documentation/admin-guide/init.rst for guidance. CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.11.0-rc3 #4 Stack from 00829f74: 00829f74 003d8508 003d8508 00000000 0000000a 00000003 0032b864 003d8508 00325126 00000001 00002700 00000003 00829fb0 00324e9e 00000000 00000000 00000000 0032ba9a 003ce1bd 0032b9b8 000218a4 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00002000 00000000 Call Trace: [<0032b864>] dump_stack+0xc/0x10 [<00325126>] panic+0xca/0x236 [<00324e9e>] try_to_run_init_process+0x0/0x36 [<0032ba9a>] kernel_init+0xe2/0xe8 [<0032b9b8>] kernel_init+0x0/0xe8 [<000218a4>] ret_from_kernel_thread+0xc/0x14 Reverting this change gets it going again. Any idea what might be going on? Regards Greg > --- > 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; > } >
On Mon, Aug 12, 2024 at 11:46:34AM +1000, Greg Ungerer wrote: > > @@ -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); I think this should have been: buf = folio_zero_tail(folio, fillsize, buf + fillsize); Can you give that a try?
On 12/8/24 13:30, Matthew Wilcox wrote: > On Mon, Aug 12, 2024 at 11:46:34AM +1000, Greg Ungerer wrote: >>> @@ -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); > > I think this should have been: > > buf = folio_zero_tail(folio, fillsize, buf + fillsize); > > Can you give that a try? Yep, that fixes it. Thanks Greg
On Mon, Aug 12, 2024 at 02:36:57PM +1000, Greg Ungerer wrote:
> Yep, that fixes it.
Christian, can you apply this fix, please?
diff --git a/fs/romfs/super.c b/fs/romfs/super.c
index 68758b6fed94..0addcc849ff2 100644
--- a/fs/romfs/super.c
+++ b/fs/romfs/super.c
@@ -126,7 +126,7 @@ static int romfs_read_folio(struct file *file, struct folio *folio)
}
}
- buf = folio_zero_tail(folio, fillsize, buf);
+ buf = folio_zero_tail(folio, fillsize, buf + fillsize);
kunmap_local(buf);
folio_end_read(folio, ret == 0);
return ret;
On Wed, Aug 14, 2024 at 08:32:30PM GMT, Matthew Wilcox wrote: > On Mon, Aug 12, 2024 at 02:36:57PM +1000, Greg Ungerer wrote: > > Yep, that fixes it. > > Christian, can you apply this fix, please? > > diff --git a/fs/romfs/super.c b/fs/romfs/super.c > index 68758b6fed94..0addcc849ff2 100644 > --- a/fs/romfs/super.c > +++ b/fs/romfs/super.c > @@ -126,7 +126,7 @@ static int romfs_read_folio(struct file *file, struct folio *folio) > } > } > > - buf = folio_zero_tail(folio, fillsize, buf); > + buf = folio_zero_tail(folio, fillsize, buf + fillsize); > kunmap_local(buf); > folio_end_read(folio, ret == 0); > return ret; Yep, please see #vfs.fixes. The whole series is already upstream.
On 15/8/24 22:42, Christian Brauner wrote: > On Wed, Aug 14, 2024 at 08:32:30PM GMT, Matthew Wilcox wrote: >> On Mon, Aug 12, 2024 at 02:36:57PM +1000, Greg Ungerer wrote: >>> Yep, that fixes it. >> >> Christian, can you apply this fix, please? >> >> diff --git a/fs/romfs/super.c b/fs/romfs/super.c >> index 68758b6fed94..0addcc849ff2 100644 >> --- a/fs/romfs/super.c >> +++ b/fs/romfs/super.c >> @@ -126,7 +126,7 @@ static int romfs_read_folio(struct file *file, struct folio *folio) >> } >> } >> >> - buf = folio_zero_tail(folio, fillsize, buf); >> + buf = folio_zero_tail(folio, fillsize, buf + fillsize); >> kunmap_local(buf); >> folio_end_read(folio, ret == 0); >> return ret; > > Yep, please see #vfs.fixes. The whole series is already upstream. Just a heads up, this is still broken in 6.11-rc5.
On Mon, Aug 26, 2024 at 11:34:21AM GMT, Greg Ungerer wrote: > > On 15/8/24 22:42, Christian Brauner wrote: > > On Wed, Aug 14, 2024 at 08:32:30PM GMT, Matthew Wilcox wrote: > > > On Mon, Aug 12, 2024 at 02:36:57PM +1000, Greg Ungerer wrote: > > > > Yep, that fixes it. > > > > > > Christian, can you apply this fix, please? > > > > > > diff --git a/fs/romfs/super.c b/fs/romfs/super.c > > > index 68758b6fed94..0addcc849ff2 100644 > > > --- a/fs/romfs/super.c > > > +++ b/fs/romfs/super.c > > > @@ -126,7 +126,7 @@ static int romfs_read_folio(struct file *file, struct folio *folio) > > > } > > > } > > > - buf = folio_zero_tail(folio, fillsize, buf); > > > + buf = folio_zero_tail(folio, fillsize, buf + fillsize); > > > kunmap_local(buf); > > > folio_end_read(folio, ret == 0); > > > return ret; > > > > Yep, please see #vfs.fixes. The whole series is already upstream. > > Just a heads up, this is still broken in 6.11-rc5. Pull request will be becoming today. Some fixes in the netfs library caused some delays because they had to be redone a few times.
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; }
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(-)