| Message ID | 20230126202415.1682629-4-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Convert most of ext4 to folios | expand |
Hi Matthew,
I love your patch! Yet something to improve:
[auto build test ERROR on next-20230127]
[cannot apply to tytso-ext4/dev xfs-linux/for-next linus/master v6.2-rc5 v6.2-rc4 v6.2-rc3 v6.2-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Matthew-Wilcox-Oracle/fs-Add-FGP_WRITEBEGIN/20230128-150212
patch link: https://lore.kernel.org/r/20230126202415.1682629-4-willy%40infradead.org
patch subject: [PATCH 03/31] ext4: Convert ext4_bio_write_page() to use a folio
config: loongarch-randconfig-r001-20230123 (https://download.01.org/0day-ci/archive/20230129/202301290044.oKaK49Hs-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f6e4c5cfaf2ef7b8ee6c5354bbbd5f1ee758746f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Matthew-Wilcox-Oracle/fs-Add-FGP_WRITEBEGIN/20230128-150212
git checkout f6e4c5cfaf2ef7b8ee6c5354bbbd5f1ee758746f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "bio_add_folio" [fs/ext4/ext4.ko] undefined!
Hi Matthew,
I love your patch! Yet something to improve:
[auto build test ERROR on next-20230127]
[cannot apply to tytso-ext4/dev xfs-linux/for-next linus/master v6.2-rc5 v6.2-rc4 v6.2-rc3 v6.2-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Matthew-Wilcox-Oracle/fs-Add-FGP_WRITEBEGIN/20230128-150212
patch link: https://lore.kernel.org/r/20230126202415.1682629-4-willy%40infradead.org
patch subject: [PATCH 03/31] ext4: Convert ext4_bio_write_page() to use a folio
config: x86_64-randconfig-a013-20230123 (https://download.01.org/0day-ci/archive/20230129/202301290216.BcfP0oGK-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f6e4c5cfaf2ef7b8ee6c5354bbbd5f1ee758746f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Matthew-Wilcox-Oracle/fs-Add-FGP_WRITEBEGIN/20230128-150212
git checkout f6e4c5cfaf2ef7b8ee6c5354bbbd5f1ee758746f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "bio_add_folio" [fs/ext4/ext4.ko] undefined!
"Matthew Wilcox (Oracle)" <willy@infradead.org> writes: > Remove several calls to compound_head() and the last caller of > set_page_writeback_keepwrite(), so remove the wrapper too. Straight forward conversion. Looks good to me. Please feel free to add - Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > fs/ext4/page-io.c | 58 ++++++++++++++++++-------------------- > include/linux/page-flags.h | 5 ---- > 2 files changed, 27 insertions(+), 36 deletions(-) > > diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c > index beaec6d81074..982791050892 100644 > --- a/fs/ext4/page-io.c > +++ b/fs/ext4/page-io.c > @@ -409,11 +409,9 @@ static void io_submit_init_bio(struct ext4_io_submit *io, > > static void io_submit_add_bh(struct ext4_io_submit *io, > struct inode *inode, > - struct page *page, > + struct folio *folio, > struct buffer_head *bh) > { > - int ret; > - > if (io->io_bio && (bh->b_blocknr != io->io_next_block || > !fscrypt_mergeable_bio_bh(io->io_bio, bh))) { > submit_and_retry: > @@ -421,10 +419,9 @@ static void io_submit_add_bh(struct ext4_io_submit *io, > } > if (io->io_bio == NULL) > io_submit_init_bio(io, bh); > - ret = bio_add_page(io->io_bio, page, bh->b_size, bh_offset(bh)); > - if (ret != bh->b_size) > + if (!bio_add_folio(io->io_bio, folio, bh->b_size, bh_offset(bh))) > goto submit_and_retry; > - wbc_account_cgroup_owner(io->io_wbc, page, bh->b_size); > + wbc_account_cgroup_owner(io->io_wbc, &folio->page, bh->b_size); > io->io_next_block++; > } > > @@ -432,8 +429,9 @@ int ext4_bio_write_page(struct ext4_io_submit *io, > struct page *page, > int len) > { > - struct page *bounce_page = NULL; > - struct inode *inode = page->mapping->host; > + struct folio *folio = page_folio(page); > + struct folio *io_folio = folio; > + struct inode *inode = folio->mapping->host; > unsigned block_start; > struct buffer_head *bh, *head; > int ret = 0; > @@ -441,30 +439,30 @@ int ext4_bio_write_page(struct ext4_io_submit *io, > struct writeback_control *wbc = io->io_wbc; > bool keep_towrite = false; > > - BUG_ON(!PageLocked(page)); > - BUG_ON(PageWriteback(page)); > + BUG_ON(!folio_test_locked(folio)); > + BUG_ON(folio_test_writeback(folio)); > > - ClearPageError(page); > + folio_clear_error(folio); > > /* > * Comments copied from block_write_full_page: > * > - * The page straddles i_size. It must be zeroed out on each and every > + * The folio straddles i_size. It must be zeroed out on each and every > * writepage invocation because it may be mmapped. "A file is mapped > * in multiples of the page size. For a file that is not a multiple of > * the page size, the remaining memory is zeroed when mapped, and > * writes to that region are not written out to the file." > */ > - if (len < PAGE_SIZE) > - zero_user_segment(page, len, PAGE_SIZE); > + if (len < folio_size(folio)) > + folio_zero_segment(folio, len, folio_size(folio)); > /* > * In the first loop we prepare and mark buffers to submit. We have to > - * mark all buffers in the page before submitting so that > - * end_page_writeback() cannot be called from ext4_end_bio() when IO > + * mark all buffers in the folio before submitting so that > + * folio_end_writeback() cannot be called from ext4_end_bio() when IO > * on the first buffer finishes and we are still working on submitting > * the second buffer. > */ > - bh = head = page_buffers(page); > + bh = head = folio_buffers(folio); > do { > block_start = bh_offset(bh); > if (block_start >= len) { > @@ -479,14 +477,14 @@ int ext4_bio_write_page(struct ext4_io_submit *io, > clear_buffer_dirty(bh); > /* > * Keeping dirty some buffer we cannot write? Make sure > - * to redirty the page and keep TOWRITE tag so that > - * racing WB_SYNC_ALL writeback does not skip the page. > + * to redirty the folio and keep TOWRITE tag so that > + * racing WB_SYNC_ALL writeback does not skip the folio. > * This happens e.g. when doing writeout for > * transaction commit. > */ > if (buffer_dirty(bh)) { > - if (!PageDirty(page)) > - redirty_page_for_writepage(wbc, page); > + if (!folio_test_dirty(folio)) > + folio_redirty_for_writepage(wbc, folio); > keep_towrite = true; > } > continue; > @@ -498,11 +496,11 @@ int ext4_bio_write_page(struct ext4_io_submit *io, > nr_to_submit++; > } while ((bh = bh->b_this_page) != head); > > - /* Nothing to submit? Just unlock the page... */ > + /* Nothing to submit? Just unlock the folio... */ > if (!nr_to_submit) > goto unlock; > > - bh = head = page_buffers(page); > + bh = head = folio_buffers(folio); > > /* > * If any blocks are being written to an encrypted file, encrypt them > @@ -514,6 +512,7 @@ int ext4_bio_write_page(struct ext4_io_submit *io, > if (fscrypt_inode_uses_fs_layer_crypto(inode) && nr_to_submit) { > gfp_t gfp_flags = GFP_NOFS; > unsigned int enc_bytes = round_up(len, i_blocksize(inode)); > + struct page *bounce_page; > > /* > * Since bounce page allocation uses a mempool, we can only use > @@ -540,7 +539,7 @@ int ext4_bio_write_page(struct ext4_io_submit *io, > } > > printk_ratelimited(KERN_ERR "%s: ret = %d\n", __func__, ret); > - redirty_page_for_writepage(wbc, page); > + folio_redirty_for_writepage(wbc, folio); > do { > if (buffer_async_write(bh)) { > clear_buffer_async_write(bh); > @@ -550,21 +549,18 @@ int ext4_bio_write_page(struct ext4_io_submit *io, > } while (bh != head); > goto unlock; > } > + io_folio = page_folio(bounce_page); > } > > - if (keep_towrite) > - set_page_writeback_keepwrite(page); > - else > - set_page_writeback(page); > + __folio_start_writeback(folio, keep_towrite); > > /* Now submit buffers to write */ > do { > if (!buffer_async_write(bh)) > continue; > - io_submit_add_bh(io, inode, > - bounce_page ? bounce_page : page, bh); > + io_submit_add_bh(io, inode, io_folio, bh); > } while ((bh = bh->b_this_page) != head); > unlock: > - unlock_page(page); > + folio_unlock(folio); > return ret; > } > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > index 0425f22a9c82..bba2a32031a2 100644 > --- a/include/linux/page-flags.h > +++ b/include/linux/page-flags.h > @@ -766,11 +766,6 @@ bool set_page_writeback(struct page *page); > #define folio_start_writeback_keepwrite(folio) \ > __folio_start_writeback(folio, true) > > -static inline void set_page_writeback_keepwrite(struct page *page) > -{ > - folio_start_writeback_keepwrite(page_folio(page)); > -} > - > static inline bool test_set_page_writeback(struct page *page) > { > return set_page_writeback(page); > -- > 2.35.1
On Thu, Jan 26, 2023 at 08:23:47PM +0000, Matthew Wilcox (Oracle) wrote: > Remove several calls to compound_head() and the last caller of > set_page_writeback_keepwrite(), so remove the wrapper too. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Reviewed-by: Theodore Ts'o <tytso@mit.edu>
diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c index beaec6d81074..982791050892 100644 --- a/fs/ext4/page-io.c +++ b/fs/ext4/page-io.c @@ -409,11 +409,9 @@ static void io_submit_init_bio(struct ext4_io_submit *io, static void io_submit_add_bh(struct ext4_io_submit *io, struct inode *inode, - struct page *page, + struct folio *folio, struct buffer_head *bh) { - int ret; - if (io->io_bio && (bh->b_blocknr != io->io_next_block || !fscrypt_mergeable_bio_bh(io->io_bio, bh))) { submit_and_retry: @@ -421,10 +419,9 @@ static void io_submit_add_bh(struct ext4_io_submit *io, } if (io->io_bio == NULL) io_submit_init_bio(io, bh); - ret = bio_add_page(io->io_bio, page, bh->b_size, bh_offset(bh)); - if (ret != bh->b_size) + if (!bio_add_folio(io->io_bio, folio, bh->b_size, bh_offset(bh))) goto submit_and_retry; - wbc_account_cgroup_owner(io->io_wbc, page, bh->b_size); + wbc_account_cgroup_owner(io->io_wbc, &folio->page, bh->b_size); io->io_next_block++; } @@ -432,8 +429,9 @@ int ext4_bio_write_page(struct ext4_io_submit *io, struct page *page, int len) { - struct page *bounce_page = NULL; - struct inode *inode = page->mapping->host; + struct folio *folio = page_folio(page); + struct folio *io_folio = folio; + struct inode *inode = folio->mapping->host; unsigned block_start; struct buffer_head *bh, *head; int ret = 0; @@ -441,30 +439,30 @@ int ext4_bio_write_page(struct ext4_io_submit *io, struct writeback_control *wbc = io->io_wbc; bool keep_towrite = false; - BUG_ON(!PageLocked(page)); - BUG_ON(PageWriteback(page)); + BUG_ON(!folio_test_locked(folio)); + BUG_ON(folio_test_writeback(folio)); - ClearPageError(page); + folio_clear_error(folio); /* * Comments copied from block_write_full_page: * - * The page straddles i_size. It must be zeroed out on each and every + * The folio straddles i_size. It must be zeroed out on each and every * writepage invocation because it may be mmapped. "A file is mapped * in multiples of the page size. For a file that is not a multiple of * the page size, the remaining memory is zeroed when mapped, and * writes to that region are not written out to the file." */ - if (len < PAGE_SIZE) - zero_user_segment(page, len, PAGE_SIZE); + if (len < folio_size(folio)) + folio_zero_segment(folio, len, folio_size(folio)); /* * In the first loop we prepare and mark buffers to submit. We have to - * mark all buffers in the page before submitting so that - * end_page_writeback() cannot be called from ext4_end_bio() when IO + * mark all buffers in the folio before submitting so that + * folio_end_writeback() cannot be called from ext4_end_bio() when IO * on the first buffer finishes and we are still working on submitting * the second buffer. */ - bh = head = page_buffers(page); + bh = head = folio_buffers(folio); do { block_start = bh_offset(bh); if (block_start >= len) { @@ -479,14 +477,14 @@ int ext4_bio_write_page(struct ext4_io_submit *io, clear_buffer_dirty(bh); /* * Keeping dirty some buffer we cannot write? Make sure - * to redirty the page and keep TOWRITE tag so that - * racing WB_SYNC_ALL writeback does not skip the page. + * to redirty the folio and keep TOWRITE tag so that + * racing WB_SYNC_ALL writeback does not skip the folio. * This happens e.g. when doing writeout for * transaction commit. */ if (buffer_dirty(bh)) { - if (!PageDirty(page)) - redirty_page_for_writepage(wbc, page); + if (!folio_test_dirty(folio)) + folio_redirty_for_writepage(wbc, folio); keep_towrite = true; } continue; @@ -498,11 +496,11 @@ int ext4_bio_write_page(struct ext4_io_submit *io, nr_to_submit++; } while ((bh = bh->b_this_page) != head); - /* Nothing to submit? Just unlock the page... */ + /* Nothing to submit? Just unlock the folio... */ if (!nr_to_submit) goto unlock; - bh = head = page_buffers(page); + bh = head = folio_buffers(folio); /* * If any blocks are being written to an encrypted file, encrypt them @@ -514,6 +512,7 @@ int ext4_bio_write_page(struct ext4_io_submit *io, if (fscrypt_inode_uses_fs_layer_crypto(inode) && nr_to_submit) { gfp_t gfp_flags = GFP_NOFS; unsigned int enc_bytes = round_up(len, i_blocksize(inode)); + struct page *bounce_page; /* * Since bounce page allocation uses a mempool, we can only use @@ -540,7 +539,7 @@ int ext4_bio_write_page(struct ext4_io_submit *io, } printk_ratelimited(KERN_ERR "%s: ret = %d\n", __func__, ret); - redirty_page_for_writepage(wbc, page); + folio_redirty_for_writepage(wbc, folio); do { if (buffer_async_write(bh)) { clear_buffer_async_write(bh); @@ -550,21 +549,18 @@ int ext4_bio_write_page(struct ext4_io_submit *io, } while (bh != head); goto unlock; } + io_folio = page_folio(bounce_page); } - if (keep_towrite) - set_page_writeback_keepwrite(page); - else - set_page_writeback(page); + __folio_start_writeback(folio, keep_towrite); /* Now submit buffers to write */ do { if (!buffer_async_write(bh)) continue; - io_submit_add_bh(io, inode, - bounce_page ? bounce_page : page, bh); + io_submit_add_bh(io, inode, io_folio, bh); } while ((bh = bh->b_this_page) != head); unlock: - unlock_page(page); + folio_unlock(folio); return ret; } diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h index 0425f22a9c82..bba2a32031a2 100644 --- a/include/linux/page-flags.h +++ b/include/linux/page-flags.h @@ -766,11 +766,6 @@ bool set_page_writeback(struct page *page); #define folio_start_writeback_keepwrite(folio) \ __folio_start_writeback(folio, true) -static inline void set_page_writeback_keepwrite(struct page *page) -{ - folio_start_writeback_keepwrite(page_folio(page)); -} - static inline bool test_set_page_writeback(struct page *page) { return set_page_writeback(page);
Remove several calls to compound_head() and the last caller of set_page_writeback_keepwrite(), so remove the wrapper too. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/ext4/page-io.c | 58 ++++++++++++++++++-------------------- include/linux/page-flags.h | 5 ---- 2 files changed, 27 insertions(+), 36 deletions(-)