diff mbox series

[16/16] btrfs: split page locking out of __process_pages_contig

Message ID 20230523081322.331337-17-hch@lst.de (mailing list archive)
State New, archived
Headers show
Series [01/16] btrfs: fix range_end calculation in extent_write_locked_range | expand

Commit Message

Christoph Hellwig May 23, 2023, 8:13 a.m. UTC
There is a lot of complexity in __process_pages_contig to deal with the
PAGE_LOCK case that can return an error unlike all the other actions.

Open code the page iteration for page locking in lock_delalloc_pages and
remove all the now unused code from __process_pages_contig.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/extent_io.c | 149 +++++++++++++++++--------------------------
 fs/btrfs/extent_io.h |   1 -
 2 files changed, 59 insertions(+), 91 deletions(-)

Comments

kernel test robot May 23, 2023, 11:27 p.m. UTC | #1
Hi Christoph,

kernel test robot noticed the following build warnings:

[auto build test WARNING on kdave/for-next]
[also build test WARNING on next-20230523]
[cannot apply to linus/master v6.4-rc3]
[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/Christoph-Hellwig/btrfs-factor-out-a-btrfs_verify_page-helper/20230523-215210
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
patch link:    https://lore.kernel.org/r/20230523081322.331337-17-hch%40lst.de
patch subject: [PATCH 16/16] btrfs: split page locking out of __process_pages_contig
config: i386-randconfig-i062-20230523
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/2d67fc662bca29378612eb06226bd9206c353173
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Christoph-Hellwig/btrfs-factor-out-a-btrfs_verify_page-helper/20230523-215210
        git checkout 2d67fc662bca29378612eb06226bd9206c353173
        # 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=i386 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/nvmem/ fs/btrfs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305240604.hqLtY7lK-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/btrfs/extent_io.c:230:16: warning: variable 'pages_processed' set but not used [-Wunused-but-set-variable]
           unsigned long pages_processed = 0;
                         ^
   1 warning generated.


vim +/pages_processed +230 fs/btrfs/extent_io.c

ed8f13bf4a2ccb Qu Wenruo             2021-05-31  221  
2d67fc662bca29 Christoph Hellwig     2023-05-23  222  static void __process_pages_contig(struct address_space *mapping,
2d67fc662bca29 Christoph Hellwig     2023-05-23  223  				   struct page *locked_page, u64 start, u64 end,
2d67fc662bca29 Christoph Hellwig     2023-05-23  224  				   unsigned long page_ops)
ed8f13bf4a2ccb Qu Wenruo             2021-05-31  225  {
e38992be1f6cf3 Qu Wenruo             2021-05-31  226  	struct btrfs_fs_info *fs_info = btrfs_sb(mapping->host->i_sb);
ed8f13bf4a2ccb Qu Wenruo             2021-05-31  227  	pgoff_t start_index = start >> PAGE_SHIFT;
ed8f13bf4a2ccb Qu Wenruo             2021-05-31  228  	pgoff_t end_index = end >> PAGE_SHIFT;
ed8f13bf4a2ccb Qu Wenruo             2021-05-31  229  	pgoff_t index = start_index;
ed8f13bf4a2ccb Qu Wenruo             2021-05-31 @230  	unsigned long pages_processed = 0;
04c6b79ae4f0bc Vishal Moola (Oracle  2022-08-23  231) 	struct folio_batch fbatch;
ed8f13bf4a2ccb Qu Wenruo             2021-05-31  232  	int i;
ed8f13bf4a2ccb Qu Wenruo             2021-05-31  233  
04c6b79ae4f0bc Vishal Moola (Oracle  2022-08-23  234) 	folio_batch_init(&fbatch);
04c6b79ae4f0bc Vishal Moola (Oracle  2022-08-23  235) 	while (index <= end_index) {
04c6b79ae4f0bc Vishal Moola (Oracle  2022-08-23  236) 		int found_folios;
04c6b79ae4f0bc Vishal Moola (Oracle  2022-08-23  237) 
04c6b79ae4f0bc Vishal Moola (Oracle  2022-08-23  238) 		found_folios = filemap_get_folios_contig(mapping, &index,
04c6b79ae4f0bc Vishal Moola (Oracle  2022-08-23  239) 				end_index, &fbatch);
04c6b79ae4f0bc Vishal Moola (Oracle  2022-08-23  240) 		for (i = 0; i < found_folios; i++) {
04c6b79ae4f0bc Vishal Moola (Oracle  2022-08-23  241) 			struct folio *folio = fbatch.folios[i];
2d67fc662bca29 Christoph Hellwig     2023-05-23  242  
2d67fc662bca29 Christoph Hellwig     2023-05-23  243  			process_one_page(fs_info, &folio->page, locked_page,
2d67fc662bca29 Christoph Hellwig     2023-05-23  244  					 page_ops, start, end);
04c6b79ae4f0bc Vishal Moola (Oracle  2022-08-23  245) 			pages_processed += folio_nr_pages(folio);
ed8f13bf4a2ccb Qu Wenruo             2021-05-31  246  		}
04c6b79ae4f0bc Vishal Moola (Oracle  2022-08-23  247) 		folio_batch_release(&fbatch);
ed8f13bf4a2ccb Qu Wenruo             2021-05-31  248  		cond_resched();
ed8f13bf4a2ccb Qu Wenruo             2021-05-31  249  	}
ed8f13bf4a2ccb Qu Wenruo             2021-05-31  250  }
da2c7009f6cafe Liu Bo                2017-02-10  251
diff mbox series

Patch

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 77f0e405280736..15021d25155f97 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -197,18 +197,9 @@  void extent_range_redirty_for_io(struct inode *inode, u64 start, u64 end)
 	}
 }
 
-/*
- * Process one page for __process_pages_contig().
- *
- * Return >0 if we hit @page == @locked_page.
- * Return 0 if we updated the page status.
- * Return -EGAIN if the we need to try again.
- * (For PAGE_LOCK case but got dirty page or page not belong to mapping)
- */
-static int process_one_page(struct btrfs_fs_info *fs_info,
-			    struct address_space *mapping,
-			    struct page *page, struct page *locked_page,
-			    unsigned long page_ops, u64 start, u64 end)
+static void process_one_page(struct btrfs_fs_info *fs_info,
+			     struct page *page, struct page *locked_page,
+			     unsigned long page_ops, u64 start, u64 end)
 {
 	u32 len;
 
@@ -224,29 +215,13 @@  static int process_one_page(struct btrfs_fs_info *fs_info,
 	if (page_ops & PAGE_END_WRITEBACK)
 		btrfs_page_clamp_clear_writeback(fs_info, page, start, len);
 
-	if (page == locked_page)
-		return 1;
-
-	if (page_ops & PAGE_LOCK) {
-		int ret;
-
-		ret = btrfs_page_start_writer_lock(fs_info, page, start, len);
-		if (ret)
-			return ret;
-		if (!PageDirty(page) || page->mapping != mapping) {
-			btrfs_page_end_writer_lock(fs_info, page, start, len);
-			return -EAGAIN;
-		}
-	}
-	if (page_ops & PAGE_UNLOCK)
+	if (page != locked_page && (page_ops & PAGE_UNLOCK))
 		btrfs_page_end_writer_lock(fs_info, page, start, len);
-	return 0;
 }
 
-static int __process_pages_contig(struct address_space *mapping,
-				  struct page *locked_page,
-				  u64 start, u64 end, unsigned long page_ops,
-				  u64 *processed_end)
+static void __process_pages_contig(struct address_space *mapping,
+				   struct page *locked_page, u64 start, u64 end,
+				   unsigned long page_ops)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(mapping->host->i_sb);
 	pgoff_t start_index = start >> PAGE_SHIFT;
@@ -254,64 +229,24 @@  static int __process_pages_contig(struct address_space *mapping,
 	pgoff_t index = start_index;
 	unsigned long pages_processed = 0;
 	struct folio_batch fbatch;
-	int err = 0;
 	int i;
 
-	if (page_ops & PAGE_LOCK) {
-		ASSERT(page_ops == PAGE_LOCK);
-		ASSERT(processed_end && *processed_end == start);
-	}
-
 	folio_batch_init(&fbatch);
 	while (index <= end_index) {
 		int found_folios;
 
 		found_folios = filemap_get_folios_contig(mapping, &index,
 				end_index, &fbatch);
-
-		if (found_folios == 0) {
-			/*
-			 * Only if we're going to lock these pages, we can find
-			 * nothing at @index.
-			 */
-			ASSERT(page_ops & PAGE_LOCK);
-			err = -EAGAIN;
-			goto out;
-		}
-
 		for (i = 0; i < found_folios; i++) {
-			int process_ret;
 			struct folio *folio = fbatch.folios[i];
-			process_ret = process_one_page(fs_info, mapping,
-					&folio->page, locked_page, page_ops,
-					start, end);
-			if (process_ret < 0) {
-				err = -EAGAIN;
-				folio_batch_release(&fbatch);
-				goto out;
-			}
+
+			process_one_page(fs_info, &folio->page, locked_page,
+					 page_ops, start, end);
 			pages_processed += folio_nr_pages(folio);
 		}
 		folio_batch_release(&fbatch);
 		cond_resched();
 	}
-out:
-	if (err && processed_end) {
-		/*
-		 * Update @processed_end. I know this is awful since it has
-		 * two different return value patterns (inclusive vs exclusive).
-		 *
-		 * But the exclusive pattern is necessary if @start is 0, or we
-		 * underflow and check against processed_end won't work as
-		 * expected.
-		 */
-		if (pages_processed)
-			*processed_end = min(end,
-			((u64)(start_index + pages_processed) << PAGE_SHIFT) - 1);
-		else
-			*processed_end = start;
-	}
-	return err;
 }
 
 static noinline void __unlock_for_delalloc(struct inode *inode,
@@ -326,29 +261,63 @@  static noinline void __unlock_for_delalloc(struct inode *inode,
 		return;
 
 	__process_pages_contig(inode->i_mapping, locked_page, start, end,
-			       PAGE_UNLOCK, NULL);
+			       PAGE_UNLOCK);
 }
 
 static noinline int lock_delalloc_pages(struct inode *inode,
 					struct page *locked_page,
-					u64 delalloc_start,
-					u64 delalloc_end)
+					u64 start,
+					u64 end)
 {
-	unsigned long index = delalloc_start >> PAGE_SHIFT;
-	unsigned long end_index = delalloc_end >> PAGE_SHIFT;
-	u64 processed_end = delalloc_start;
-	int ret;
+	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
+	struct address_space *mapping = inode->i_mapping;
+	pgoff_t start_index = start >> PAGE_SHIFT;
+	pgoff_t end_index = end >> PAGE_SHIFT;
+	pgoff_t index = start_index;
+	u64 processed_end = start;
+	struct folio_batch fbatch;
 
-	ASSERT(locked_page);
 	if (index == locked_page->index && index == end_index)
 		return 0;
 
-	ret = __process_pages_contig(inode->i_mapping, locked_page, delalloc_start,
-				     delalloc_end, PAGE_LOCK, &processed_end);
-	if (ret == -EAGAIN && processed_end > delalloc_start)
-		__unlock_for_delalloc(inode, locked_page, delalloc_start,
-				      processed_end);
-	return ret;
+	folio_batch_init(&fbatch);
+	while (index <= end_index) {
+		unsigned int found_folios, i;
+
+		found_folios = filemap_get_folios_contig(mapping, &index,
+				end_index, &fbatch);
+		if (found_folios == 0)
+			goto out;
+
+		for (i = 0; i < found_folios; i++) {
+			struct page *page = &fbatch.folios[i]->page;
+			u32 len = end + 1 - start;
+
+			if (page == locked_page)
+				continue;
+
+			if (btrfs_page_start_writer_lock(fs_info, page, start,
+							 len))
+				goto out;
+
+			if (!PageDirty(page) || page->mapping != mapping) {
+				btrfs_page_end_writer_lock(fs_info, page, start,
+							   len);
+				goto out;
+			}
+
+			processed_end = page_offset(page) + PAGE_SIZE - 1;
+		}
+		folio_batch_release(&fbatch);
+		cond_resched();
+	}
+
+	return 0;
+out:
+	folio_batch_release(&fbatch);
+	if (processed_end > start)
+		__unlock_for_delalloc(inode, locked_page, start, processed_end);
+	return -EAGAIN;
 }
 
 /*
@@ -467,7 +436,7 @@  void extent_clear_unlock_delalloc(struct btrfs_inode *inode, u64 start, u64 end,
 	clear_extent_bit(&inode->io_tree, start, end, clear_bits, NULL);
 
 	__process_pages_contig(inode->vfs_inode.i_mapping, locked_page,
-			       start, end, page_ops, NULL);
+			       start, end, page_ops);
 }
 
 static int btrfs_verify_page(struct page *page, u64 start)
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index daef9374c2095f..423853be57ed87 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -39,7 +39,6 @@  enum {
 	ENUM_BIT(PAGE_START_WRITEBACK),
 	ENUM_BIT(PAGE_END_WRITEBACK),
 	ENUM_BIT(PAGE_SET_ORDERED),
-	ENUM_BIT(PAGE_LOCK),
 };
 
 /*