| Message ID | 20230605165029.2908304-2-willy@infradead.org |
|---|---|
| State | Not Applicable |
| Delegated to: | Richard Weinberger |
| Headers | show |
| Series | ubifs: Convert writeback to use folios | expand |
在 2023/6/6 0:50, Matthew Wilcox (Oracle) 写道: Hi, > This is a simplistic conversion to separate out any effects of > no longer having a writepage method. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > fs/ubifs/file.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c > index 979ab1d9d0c3..8bb4cb9d528f 100644 > --- a/fs/ubifs/file.c > +++ b/fs/ubifs/file.c > @@ -1003,8 +1003,10 @@ static int do_writepage(struct page *page, int len) > * on the page lock and it would not write the truncated inode node to the > * journal before we have finished. > */ > -static int ubifs_writepage(struct page *page, struct writeback_control *wbc) > +static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc, > + void *data) > { > + struct page *page = &folio->page; > struct inode *inode = page->mapping->host; > struct ubifs_info *c = inode->i_sb->s_fs_info; > struct ubifs_inode *ui = ubifs_inode(inode); > @@ -1076,6 +1078,12 @@ static int ubifs_writepage(struct page *page, struct writeback_control *wbc) > return err; > } > > +static int ubifs_writepages(struct address_space *mapping, > + struct writeback_control *wbc) > +{ > + return write_cache_pages(mapping, wbc, ubifs_writepage, NULL); > +} > + There is a small difference. before patch applied: do_writepages -> write_cache_pages -> writepage_cb: ubifs_writepage mapping_set_error(mapping, ret) So, we can get error returned from errseq_check_and_advance in syncfs syscall if ubifs_writepage occurs EIO. after patch applied: do_writepages -> ubifs_writepages -> write_cache_pages -> ubifs_writepage, mapping won't be set error if ubifs_writepage failed. Unfortunately, ubifs is not a block filesystem, so sync_filesystem->sync_blockdev_nowait will return 0. Finally, syncfs syscall will return 0. > /** > * do_attr_changes - change inode attributes. > * @inode: inode to change attributes for > @@ -1636,7 +1644,7 @@ static int ubifs_symlink_getattr(struct mnt_idmap *idmap, > > const struct address_space_operations ubifs_file_address_operations = { > .read_folio = ubifs_read_folio, > - .writepage = ubifs_writepage, > + .writepages = ubifs_writepages, > .write_begin = ubifs_write_begin, > .write_end = ubifs_write_end, > .invalidate_folio = ubifs_invalidate_folio, >
在 2023/6/6 22:37, Zhihao Cheng 写道: > 在 2023/6/6 0:50, Matthew Wilcox (Oracle) 写道: > Hi, >> This is a simplistic conversion to separate out any effects of >> no longer having a writepage method. >> >> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> >> --- >> fs/ubifs/file.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c >> index 979ab1d9d0c3..8bb4cb9d528f 100644 >> --- a/fs/ubifs/file.c >> +++ b/fs/ubifs/file.c >> @@ -1003,8 +1003,10 @@ static int do_writepage(struct page *page, int >> len) >> * on the page lock and it would not write the truncated inode node >> to the >> * journal before we have finished. >> */ >> -static int ubifs_writepage(struct page *page, struct >> writeback_control *wbc) >> +static int ubifs_writepage(struct folio *folio, struct >> writeback_control *wbc, >> + void *data) >> { >> + struct page *page = &folio->page; >> struct inode *inode = page->mapping->host; >> struct ubifs_info *c = inode->i_sb->s_fs_info; >> struct ubifs_inode *ui = ubifs_inode(inode); >> @@ -1076,6 +1078,12 @@ static int ubifs_writepage(struct page *page, >> struct writeback_control *wbc) >> return err; >> } >> +static int ubifs_writepages(struct address_space *mapping, >> + struct writeback_control *wbc) >> +{ >> + return write_cache_pages(mapping, wbc, ubifs_writepage, NULL); >> +} >> + > > There is a small difference. > before patch applied: > do_writepages -> write_cache_pages -> writepage_cb: > ubifs_writepage > mapping_set_error(mapping, ret) > > So, we can get error returned from errseq_check_and_advance in syncfs > syscall if ubifs_writepage occurs EIO. > > after patch applied: > > do_writepages -> ubifs_writepages -> write_cache_pages -> > ubifs_writepage, mapping won't be set error if ubifs_writepage failed. > Unfortunately, ubifs is not a block filesystem, so > sync_filesystem->sync_blockdev_nowait will return 0. Finally, syncfs > syscall will return 0. > I think we can add mapping_set_error in error branch of ubifs_writepage to solve it. BTW, I notice that shrink_folio_list -> pageout will try to shrink page by writepage method, if we remove '->writepage', the dirty page won't be shrinked in that way? > >> /** >> * do_attr_changes - change inode attributes. >> * @inode: inode to change attributes for >> @@ -1636,7 +1644,7 @@ static int ubifs_symlink_getattr(struct >> mnt_idmap *idmap, >> const struct address_space_operations ubifs_file_address_operations = { >> .read_folio = ubifs_read_folio, >> - .writepage = ubifs_writepage, >> + .writepages = ubifs_writepages, >> .write_begin = ubifs_write_begin, >> .write_end = ubifs_write_end, >> .invalidate_folio = ubifs_invalidate_folio, >> > > > .
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c index 979ab1d9d0c3..8bb4cb9d528f 100644 --- a/fs/ubifs/file.c +++ b/fs/ubifs/file.c @@ -1003,8 +1003,10 @@ static int do_writepage(struct page *page, int len) * on the page lock and it would not write the truncated inode node to the * journal before we have finished. */ -static int ubifs_writepage(struct page *page, struct writeback_control *wbc) +static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc, + void *data) { + struct page *page = &folio->page; struct inode *inode = page->mapping->host; struct ubifs_info *c = inode->i_sb->s_fs_info; struct ubifs_inode *ui = ubifs_inode(inode); @@ -1076,6 +1078,12 @@ static int ubifs_writepage(struct page *page, struct writeback_control *wbc) return err; } +static int ubifs_writepages(struct address_space *mapping, + struct writeback_control *wbc) +{ + return write_cache_pages(mapping, wbc, ubifs_writepage, NULL); +} + /** * do_attr_changes - change inode attributes. * @inode: inode to change attributes for @@ -1636,7 +1644,7 @@ static int ubifs_symlink_getattr(struct mnt_idmap *idmap, const struct address_space_operations ubifs_file_address_operations = { .read_folio = ubifs_read_folio, - .writepage = ubifs_writepage, + .writepages = ubifs_writepages, .write_begin = ubifs_write_begin, .write_end = ubifs_write_end, .invalidate_folio = ubifs_invalidate_folio,
This is a simplistic conversion to separate out any effects of no longer having a writepage method. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/ubifs/file.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)