| Message ID | 20230126202415.1682629-6-willy@infradead.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Convert most of ext4 to folios | expand |
"Matthew Wilcox (Oracle)" <willy@infradead.org> writes: > Prepare for multi-page folios and save some instructions by converting > to the folio API. Mostly a straight forward change. The changes looks good to me. Please feel free to add - Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> In later few patches I see ext4_readpage converted to ext4_read_folio(). I think the reason why we have not changed ext4_writepage() to ext4_write_folio() is because we anyway would like to get rid of ->writepage ops eventually in future, so no point. I think there is even patch series from Jan which tries to kill ext4_writepage() completely. -ritesh
On Tue, Mar 07, 2023 at 12:15:13AM +0530, Ritesh Harjani wrote: > "Matthew Wilcox (Oracle)" <willy@infradead.org> writes: > > > Prepare for multi-page folios and save some instructions by converting > > to the folio API. > > Mostly a straight forward change. The changes looks good to me. > Please feel free to add - > > Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > > In later few patches I see ext4_readpage converted to ext4_read_folio(). > I think the reason why we have not changed ext4_writepage() to > ext4_write_folio() is because we anyway would like to get rid of > ->writepage ops eventually in future, so no point. > I think there is even patch series from Jan which tries to kill > ext4_writepage() completely. Indeed, Jan's patch series[1] is about to land in the ext4 tree, and that's going to remove ext4_writepages. The main reason why this hadn't landed yet was due to some conflicts with some other folio changes, so you should be able to drop this patch when you rebase this patch series. - Ted [1] https://lore.kernel.org/all/20230228051319.4085470-1-tytso@mit.edu/
On Tue, Mar 14, 2023 at 06:26:46PM -0400, Theodore Ts'o wrote: > On Tue, Mar 07, 2023 at 12:15:13AM +0530, Ritesh Harjani wrote: > > "Matthew Wilcox (Oracle)" <willy@infradead.org> writes: > > > > > Prepare for multi-page folios and save some instructions by converting > > > to the folio API. > > > > Mostly a straight forward change. The changes looks good to me. > > Please feel free to add - > > > > Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > > > > In later few patches I see ext4_readpage converted to ext4_read_folio(). > > I think the reason why we have not changed ext4_writepage() to > > ext4_write_folio() is because we anyway would like to get rid of > > ->writepage ops eventually in future, so no point. > > I think there is even patch series from Jan which tries to kill > > ext4_writepage() completely. > > Indeed, Jan's patch series[1] is about to land in the ext4 tree, and > that's going to remove ext4_writepages. The main reason why this > hadn't landed yet was due to some conflicts with some other folio > changes, so you should be able to drop this patch when you rebase this > patch series. Correct; in the rebase, I ended up just dropping this patch.
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index b8b3e2e0d9fd..8e3d2cca1e0c 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -2027,26 +2027,25 @@ static int ext4_writepage(struct page *page, trace_ext4_writepage(page); size = i_size_read(inode); - if (page->index == size >> PAGE_SHIFT && + len = folio_size(folio); + if (folio_pos(folio) + len > size && !ext4_verity_in_progress(inode)) - len = size & ~PAGE_MASK; - else - len = PAGE_SIZE; + len = size - folio_pos(folio); + page_bufs = folio_buffers(folio); /* Should never happen but for bugs in other kernel subsystems */ - if (!page_has_buffers(page)) { + if (!page_bufs) { ext4_warning_inode(inode, - "page %lu does not have buffers attached", page->index); - ClearPageDirty(page); - unlock_page(page); + "page %lu does not have buffers attached", folio->index); + folio_clear_dirty(folio); + folio_unlock(folio); return 0; } - page_bufs = page_buffers(page); /* * We cannot do block allocation or other extent handling in this * function. If there are buffers needing that, we have to redirty - * the page. But we may reach here when we do a journal commit via + * the folio. But we may reach here when we do a journal commit via * journal_submit_inode_data_buffers() and in that case we must write * allocated buffers to achieve data=ordered mode guarantees. * @@ -2062,7 +2061,7 @@ static int ext4_writepage(struct page *page, */ if (ext4_walk_page_buffers(NULL, inode, page_bufs, 0, len, NULL, ext4_bh_delay_or_unwritten)) { - redirty_page_for_writepage(wbc, page); + folio_redirty_for_writepage(wbc, folio); if ((current->flags & PF_MEMALLOC) || (inode->i_sb->s_blocksize == PAGE_SIZE)) { /* @@ -2072,12 +2071,12 @@ static int ext4_writepage(struct page *page, */ WARN_ON_ONCE((current->flags & (PF_MEMALLOC|PF_KSWAPD)) == PF_MEMALLOC); - unlock_page(page); + folio_unlock(folio); return 0; } } - if (PageChecked(page) && ext4_should_journal_data(inode)) + if (folio_test_checked(folio) && ext4_should_journal_data(inode)) /* * It's mmapped pagecache. Add buffers and journal it. There * doesn't seem much point in redirtying the page here. @@ -2087,8 +2086,8 @@ static int ext4_writepage(struct page *page, ext4_io_submit_init(&io_submit, wbc); io_submit.io_end = ext4_init_io_end(inode, GFP_NOFS); if (!io_submit.io_end) { - redirty_page_for_writepage(wbc, page); - unlock_page(page); + folio_redirty_for_writepage(wbc, folio); + folio_unlock(folio); return -ENOMEM; } ret = ext4_bio_write_page(&io_submit, page, len);
Prepare for multi-page folios and save some instructions by converting to the folio API. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/ext4/inode.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-)