[PATCH v1 0/5] clean up block_commit_write

[PATCH v1 0/5] clean up block_commit_write

[Bean Huo]

*** BLURB HERE ***

Bean Huo (5):
  fs/buffer: clean up block_commit_write
  fs/buffer.c: convert block_commit_write to return void
  ext4: No need to check return value of block_commit_write()
  fs/ocfs2: No need to check return value of block_commit_write()
  udf: No need to check return value of block_commit_write()

 fs/buffer.c                 | 24 +++++++-----------------
 fs/ext4/move_extent.c       |  7 ++-----
 fs/ocfs2/file.c             |  7 +------
 fs/udf/file.c               |  6 +++---
 include/linux/buffer_head.h |  2 +-
 5 files changed, 14 insertions(+), 32 deletions(-)

-- 
2.34.1

[PATCH v1 1/5] fs/buffer: clean up block_commit_write

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

Originally inode is used to get blksize, after commit 45bce8f3e343
("fs/buffer.c: make block-size be per-page and protected by the page lock"),
__block_commit_write no longer uses this parameter inode, this patch is to
remove inode and clean up block_commit_write.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 fs/buffer.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index a7fc561758b1..b88bb7ec38be 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2116,8 +2116,7 @@ int __block_write_begin(struct page *page, loff_t pos, unsigned len,
 }
 EXPORT_SYMBOL(__block_write_begin);
 
-static int __block_commit_write(struct inode *inode, struct page *page,
-		unsigned from, unsigned to)
+int block_commit_write(struct page *page, unsigned int from, unsigned int to)
 {
 	unsigned block_start, block_end;
 	int partial = 0;
@@ -2154,6 +2153,7 @@ static int __block_commit_write(struct inode *inode, struct page *page,
 		SetPageUptodate(page);
 	return 0;
 }
+EXPORT_SYMBOL(block_commit_write);
 
 /*
  * block_write_begin takes care of the basic task of block allocation and
@@ -2188,7 +2188,6 @@ int block_write_end(struct file *file, struct address_space *mapping,
 			loff_t pos, unsigned len, unsigned copied,
 			struct page *page, void *fsdata)
 {
-	struct inode *inode = mapping->host;
 	unsigned start;
 
 	start = pos & (PAGE_SIZE - 1);
@@ -2214,7 +2213,7 @@ int block_write_end(struct file *file, struct address_space *mapping,
 	flush_dcache_page(page);
 
 	/* This could be a short (even 0-length) commit */
-	__block_commit_write(inode, page, start, start+copied);
+	block_commit_write(page, start, start+copied);
 
 	return copied;
 }
@@ -2535,14 +2534,6 @@ int cont_write_begin(struct file *file, struct address_space *mapping,
 }
 EXPORT_SYMBOL(cont_write_begin);
 
-int block_commit_write(struct page *page, unsigned from, unsigned to)
-{
-	struct inode *inode = page->mapping->host;
-	__block_commit_write(inode,page,from,to);
-	return 0;
-}
-EXPORT_SYMBOL(block_commit_write);
-
 /*
  * block_page_mkwrite() is not allowed to change the file size as it gets
  * called from a page fault handler when a page is first dirtied. Hence we must
-- 
2.34.1

Re: [PATCH v1 1/5] fs/buffer: clean up block_commit_write

[Jan Kara]

On Sun 18-06-23 23:32:46, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
> 
> Originally inode is used to get blksize, after commit 45bce8f3e343
> ("fs/buffer.c: make block-size be per-page and protected by the page lock"),
> __block_commit_write no longer uses this parameter inode, this patch is to
> remove inode and clean up block_commit_write.
> 
> Signed-off-by: Bean Huo <beanhuo@micron.com>

Nice! Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/buffer.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index a7fc561758b1..b88bb7ec38be 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2116,8 +2116,7 @@ int __block_write_begin(struct page *page, loff_t pos, unsigned len,
>  }
>  EXPORT_SYMBOL(__block_write_begin);
>  
> -static int __block_commit_write(struct inode *inode, struct page *page,
> -		unsigned from, unsigned to)
> +int block_commit_write(struct page *page, unsigned int from, unsigned int to)
>  {
>  	unsigned block_start, block_end;
>  	int partial = 0;
> @@ -2154,6 +2153,7 @@ static int __block_commit_write(struct inode *inode, struct page *page,
>  		SetPageUptodate(page);
>  	return 0;
>  }
> +EXPORT_SYMBOL(block_commit_write);
>  
>  /*
>   * block_write_begin takes care of the basic task of block allocation and
> @@ -2188,7 +2188,6 @@ int block_write_end(struct file *file, struct address_space *mapping,
>  			loff_t pos, unsigned len, unsigned copied,
>  			struct page *page, void *fsdata)
>  {
> -	struct inode *inode = mapping->host;
>  	unsigned start;
>  
>  	start = pos & (PAGE_SIZE - 1);
> @@ -2214,7 +2213,7 @@ int block_write_end(struct file *file, struct address_space *mapping,
>  	flush_dcache_page(page);
>  
>  	/* This could be a short (even 0-length) commit */
> -	__block_commit_write(inode, page, start, start+copied);
> +	block_commit_write(page, start, start+copied);
>  
>  	return copied;
>  }
> @@ -2535,14 +2534,6 @@ int cont_write_begin(struct file *file, struct address_space *mapping,
>  }
>  EXPORT_SYMBOL(cont_write_begin);
>  
> -int block_commit_write(struct page *page, unsigned from, unsigned to)
> -{
> -	struct inode *inode = page->mapping->host;
> -	__block_commit_write(inode,page,from,to);
> -	return 0;
> -}
> -EXPORT_SYMBOL(block_commit_write);
> -
>  /*
>   * block_page_mkwrite() is not allowed to change the file size as it gets
>   * called from a page fault handler when a page is first dirtied. Hence we must
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v1 2/5] fs/buffer.c: convert block_commit_write to return void

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

block_commit_write() always returns 0, this patch changes it to
return void.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 fs/buffer.c                 | 11 +++++------
 include/linux/buffer_head.h |  2 +-
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index b88bb7ec38be..fa09cf94f771 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2116,7 +2116,7 @@ int __block_write_begin(struct page *page, loff_t pos, unsigned len,
 }
 EXPORT_SYMBOL(__block_write_begin);
 
-int block_commit_write(struct page *page, unsigned int from, unsigned int to)
+void block_commit_write(struct page *page, unsigned int from, unsigned int to)
 {
 	unsigned block_start, block_end;
 	int partial = 0;
@@ -2151,7 +2151,6 @@ int block_commit_write(struct page *page, unsigned int from, unsigned int to)
 	 */
 	if (!partial)
 		SetPageUptodate(page);
-	return 0;
 }
 EXPORT_SYMBOL(block_commit_write);
 
@@ -2577,11 +2576,11 @@ int block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
 		end = PAGE_SIZE;
 
 	ret = __block_write_begin(page, 0, end, get_block);
-	if (!ret)
-		ret = block_commit_write(page, 0, end);
-
-	if (unlikely(ret < 0))
+	if (unlikely(ret))
 		goto out_unlock;
+
+	block_commit_write(page, 0, end);
+
 	set_page_dirty(page);
 	wait_for_stable_page(page);
 	return 0;
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 1520793c72da..873653d2f1aa 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -284,7 +284,7 @@ int cont_write_begin(struct file *, struct address_space *, loff_t,
 			unsigned, struct page **, void **,
 			get_block_t *, loff_t *);
 int generic_cont_expand_simple(struct inode *inode, loff_t size);
-int block_commit_write(struct page *page, unsigned from, unsigned to);
+void block_commit_write(struct page *page, unsigned int from, unsigned int to);
 int block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
 				get_block_t get_block);
 /* Convert errno to return value from ->page_mkwrite() call */
-- 
2.34.1

Re: [PATCH v1 2/5] fs/buffer.c: convert block_commit_write to return void

[kernel test robot]

Hi Bean,

kernel test robot noticed the following build errors:

[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on vfs-idmapping/for-next linus/master v6.4-rc6]
[cannot apply to akpm-mm/mm-everything next-20230616]
[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/Bean-Huo/fs-buffer-clean-up-block_commit_write/20230619-053759
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20230618213250.694110-3-beanhuo%40iokpp.de
patch subject: [PATCH v1 2/5] fs/buffer.c: convert block_commit_write to return void
config: i386-defconfig (https://download.01.org/0day-ci/archive/20230619/202306190729.oETrBjGU-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230619/202306190729.oETrBjGU-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306190729.oETrBjGU-lkp@intel.com/

All errors (new ones prefixed by >>):

   fs/ext4/move_extent.c: In function 'move_extent_per_page':
>> fs/ext4/move_extent.c:399:22: error: void value not ignored as it ought to be
     399 |                 *err = block_commit_write(&folio[0]->page, from, from + replaced_size);
         |                      ^


vim +399 fs/ext4/move_extent.c

bb5574880574fe Dmitry Monakhov       2012-09-26  234  
748de6736c1e48 Akira Fujita          2009-06-17  235  /**
748de6736c1e48 Akira Fujita          2009-06-17  236   * move_extent_per_page - Move extent data per page
748de6736c1e48 Akira Fujita          2009-06-17  237   *
748de6736c1e48 Akira Fujita          2009-06-17  238   * @o_filp:			file structure of original file
748de6736c1e48 Akira Fujita          2009-06-17  239   * @donor_inode:		donor inode
748de6736c1e48 Akira Fujita          2009-06-17  240   * @orig_page_offset:		page index on original file
65dd8327eb055a Xiaoguang Wang        2014-10-11  241   * @donor_page_offset:		page index on donor file
748de6736c1e48 Akira Fujita          2009-06-17  242   * @data_offset_in_page:	block index where data swapping starts
748de6736c1e48 Akira Fujita          2009-06-17  243   * @block_len_in_page:		the number of blocks to be swapped
556615dcbf38b0 Lukas Czerner         2014-04-20  244   * @unwritten:			orig extent is unwritten or not
f868a48d06f888 Akira Fujita          2009-11-23  245   * @err:			pointer to save return value
748de6736c1e48 Akira Fujita          2009-06-17  246   *
748de6736c1e48 Akira Fujita          2009-06-17  247   * Save the data in original inode blocks and replace original inode extents
65dd8327eb055a Xiaoguang Wang        2014-10-11  248   * with donor inode extents by calling ext4_swap_extents().
f868a48d06f888 Akira Fujita          2009-11-23  249   * Finally, write out the saved data in new original inode blocks. Return
f868a48d06f888 Akira Fujita          2009-11-23  250   * replaced block count.
748de6736c1e48 Akira Fujita          2009-06-17  251   */
748de6736c1e48 Akira Fujita          2009-06-17  252  static int
44fc48f7048ab9 Akira Fujita          2009-09-05  253  move_extent_per_page(struct file *o_filp, struct inode *donor_inode,
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  254  		     pgoff_t orig_page_offset, pgoff_t donor_page_offset,
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  255  		     int data_offset_in_page,
556615dcbf38b0 Lukas Czerner         2014-04-20  256  		     int block_len_in_page, int unwritten, int *err)
748de6736c1e48 Akira Fujita          2009-06-17  257  {
496ad9aa8ef448 Al Viro               2013-01-23  258  	struct inode *orig_inode = file_inode(o_filp);
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  259) 	struct folio *folio[2] = {NULL, NULL};
748de6736c1e48 Akira Fujita          2009-06-17  260  	handle_t *handle;
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  261  	ext4_lblk_t orig_blk_offset, donor_blk_offset;
748de6736c1e48 Akira Fujita          2009-06-17  262  	unsigned long blocksize = orig_inode->i_sb->s_blocksize;
f868a48d06f888 Akira Fujita          2009-11-23  263  	unsigned int tmp_data_size, data_size, replaced_size;
bcff24887d00bc Eryu Guan             2016-02-12  264  	int i, err2, jblocks, retries = 0;
f868a48d06f888 Akira Fujita          2009-11-23  265  	int replaced_count = 0;
bb5574880574fe Dmitry Monakhov       2012-09-26  266  	int from = data_offset_in_page << orig_inode->i_blkbits;
09cbfeaf1a5a67 Kirill A. Shutemov    2016-04-01  267  	int blocks_per_page = PAGE_SIZE >> orig_inode->i_blkbits;
88c6b61ff1cfb4 Dmitry Monakhov       2014-11-05  268  	struct super_block *sb = orig_inode->i_sb;
bcff24887d00bc Eryu Guan             2016-02-12  269  	struct buffer_head *bh = NULL;
748de6736c1e48 Akira Fujita          2009-06-17  270  
748de6736c1e48 Akira Fujita          2009-06-17  271  	/*
748de6736c1e48 Akira Fujita          2009-06-17  272  	 * It needs twice the amount of ordinary journal buffers because
748de6736c1e48 Akira Fujita          2009-06-17  273  	 * inode and donor_inode may change each different metadata blocks.
748de6736c1e48 Akira Fujita          2009-06-17  274  	 */
bb5574880574fe Dmitry Monakhov       2012-09-26  275  again:
bb5574880574fe Dmitry Monakhov       2012-09-26  276  	*err = 0;
748de6736c1e48 Akira Fujita          2009-06-17  277  	jblocks = ext4_writepage_trans_blocks(orig_inode) * 2;
9924a92a8c2175 Theodore Ts'o         2013-02-08  278  	handle = ext4_journal_start(orig_inode, EXT4_HT_MOVE_EXTENTS, jblocks);
748de6736c1e48 Akira Fujita          2009-06-17  279  	if (IS_ERR(handle)) {
f868a48d06f888 Akira Fujita          2009-11-23  280  		*err = PTR_ERR(handle);
f868a48d06f888 Akira Fujita          2009-11-23  281  		return 0;
748de6736c1e48 Akira Fujita          2009-06-17  282  	}
748de6736c1e48 Akira Fujita          2009-06-17  283  
748de6736c1e48 Akira Fujita          2009-06-17  284  	orig_blk_offset = orig_page_offset * blocks_per_page +
748de6736c1e48 Akira Fujita          2009-06-17  285  		data_offset_in_page;
748de6736c1e48 Akira Fujita          2009-06-17  286  
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  287  	donor_blk_offset = donor_page_offset * blocks_per_page +
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  288  		data_offset_in_page;
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  289  
f868a48d06f888 Akira Fujita          2009-11-23  290  	/* Calculate data_size */
748de6736c1e48 Akira Fujita          2009-06-17  291  	if ((orig_blk_offset + block_len_in_page - 1) ==
748de6736c1e48 Akira Fujita          2009-06-17  292  	    ((orig_inode->i_size - 1) >> orig_inode->i_blkbits)) {
748de6736c1e48 Akira Fujita          2009-06-17  293  		/* Replace the last block */
f868a48d06f888 Akira Fujita          2009-11-23  294  		tmp_data_size = orig_inode->i_size & (blocksize - 1);
748de6736c1e48 Akira Fujita          2009-06-17  295  		/*
f868a48d06f888 Akira Fujita          2009-11-23  296  		 * If data_size equal zero, it shows data_size is multiples of
748de6736c1e48 Akira Fujita          2009-06-17  297  		 * blocksize. So we set appropriate value.
748de6736c1e48 Akira Fujita          2009-06-17  298  		 */
f868a48d06f888 Akira Fujita          2009-11-23  299  		if (tmp_data_size == 0)
f868a48d06f888 Akira Fujita          2009-11-23  300  			tmp_data_size = blocksize;
748de6736c1e48 Akira Fujita          2009-06-17  301  
f868a48d06f888 Akira Fujita          2009-11-23  302  		data_size = tmp_data_size +
748de6736c1e48 Akira Fujita          2009-06-17  303  			((block_len_in_page - 1) << orig_inode->i_blkbits);
f868a48d06f888 Akira Fujita          2009-11-23  304  	} else
f868a48d06f888 Akira Fujita          2009-11-23  305  		data_size = block_len_in_page << orig_inode->i_blkbits;
f868a48d06f888 Akira Fujita          2009-11-23  306  
f868a48d06f888 Akira Fujita          2009-11-23  307  	replaced_size = data_size;
748de6736c1e48 Akira Fujita          2009-06-17  308  
e8dfc854eef20a Vishal Moola (Oracle  2022-12-07  309) 	*err = mext_folio_double_lock(orig_inode, donor_inode, orig_page_offset,
e8dfc854eef20a Vishal Moola (Oracle  2022-12-07  310) 				     donor_page_offset, folio);
f868a48d06f888 Akira Fujita          2009-11-23  311  	if (unlikely(*err < 0))
bb5574880574fe Dmitry Monakhov       2012-09-26  312  		goto stop_journal;
8c854473917354 Dmitry Monakhov       2012-09-26  313  	/*
556615dcbf38b0 Lukas Czerner         2014-04-20  314  	 * If orig extent was unwritten it can become initialized
8c854473917354 Dmitry Monakhov       2012-09-26  315  	 * at any time after i_data_sem was dropped, in order to
8c854473917354 Dmitry Monakhov       2012-09-26  316  	 * serialize with delalloc we have recheck extent while we
8c854473917354 Dmitry Monakhov       2012-09-26  317  	 * hold page's lock, if it is still the case data copy is not
8c854473917354 Dmitry Monakhov       2012-09-26  318  	 * necessary, just swap data blocks between orig and donor.
8c854473917354 Dmitry Monakhov       2012-09-26  319  	 */
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  320) 
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  321) 	VM_BUG_ON_FOLIO(folio_test_large(folio[0]), folio[0]);
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  322) 	VM_BUG_ON_FOLIO(folio_test_large(folio[1]), folio[1]);
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  323) 	VM_BUG_ON_FOLIO(folio_nr_pages(folio[0]) != folio_nr_pages(folio[1]), folio[1]);
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  324) 
556615dcbf38b0 Lukas Czerner         2014-04-20  325  	if (unwritten) {
393d1d1d769338 Dr. Tilmann Bubeck    2013-04-08  326  		ext4_double_down_write_data_sem(orig_inode, donor_inode);
8c854473917354 Dmitry Monakhov       2012-09-26  327  		/* If any of extents in range became initialized we have to
8c854473917354 Dmitry Monakhov       2012-09-26  328  		 * fallback to data copying */
556615dcbf38b0 Lukas Czerner         2014-04-20  329  		unwritten = mext_check_coverage(orig_inode, orig_blk_offset,
8c854473917354 Dmitry Monakhov       2012-09-26  330  						block_len_in_page, 1, err);
8c854473917354 Dmitry Monakhov       2012-09-26  331  		if (*err)
8c854473917354 Dmitry Monakhov       2012-09-26  332  			goto drop_data_sem;
8c854473917354 Dmitry Monakhov       2012-09-26  333  
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  334  		unwritten &= mext_check_coverage(donor_inode, donor_blk_offset,
8c854473917354 Dmitry Monakhov       2012-09-26  335  						 block_len_in_page, 1, err);
8c854473917354 Dmitry Monakhov       2012-09-26  336  		if (*err)
8c854473917354 Dmitry Monakhov       2012-09-26  337  			goto drop_data_sem;
748de6736c1e48 Akira Fujita          2009-06-17  338  
556615dcbf38b0 Lukas Czerner         2014-04-20  339  		if (!unwritten) {
393d1d1d769338 Dr. Tilmann Bubeck    2013-04-08  340  			ext4_double_up_write_data_sem(orig_inode, donor_inode);
8c854473917354 Dmitry Monakhov       2012-09-26  341  			goto data_copy;
8c854473917354 Dmitry Monakhov       2012-09-26  342  		}
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  343) 		if ((folio_has_private(folio[0]) &&
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  344) 		     !filemap_release_folio(folio[0], 0)) ||
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  345) 		    (folio_has_private(folio[1]) &&
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  346) 		     !filemap_release_folio(folio[1], 0))) {
8c854473917354 Dmitry Monakhov       2012-09-26  347  			*err = -EBUSY;
8c854473917354 Dmitry Monakhov       2012-09-26  348  			goto drop_data_sem;
8c854473917354 Dmitry Monakhov       2012-09-26  349  		}
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  350  		replaced_count = ext4_swap_extents(handle, orig_inode,
8c854473917354 Dmitry Monakhov       2012-09-26  351  						   donor_inode, orig_blk_offset,
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  352  						   donor_blk_offset,
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  353  						   block_len_in_page, 1, err);
8c854473917354 Dmitry Monakhov       2012-09-26  354  	drop_data_sem:
393d1d1d769338 Dr. Tilmann Bubeck    2013-04-08  355  		ext4_double_up_write_data_sem(orig_inode, donor_inode);
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  356) 		goto unlock_folios;
8c854473917354 Dmitry Monakhov       2012-09-26  357  	}
8c854473917354 Dmitry Monakhov       2012-09-26  358  data_copy:
3060b6ef05603c Matthew Wilcox        2023-03-24  359  	*err = mext_page_mkuptodate(folio[0], from, from + replaced_size);
bb5574880574fe Dmitry Monakhov       2012-09-26  360  	if (*err)
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  361) 		goto unlock_folios;
748de6736c1e48 Akira Fujita          2009-06-17  362  
bb5574880574fe Dmitry Monakhov       2012-09-26  363  	/* At this point all buffers in range are uptodate, old mapping layout
bb5574880574fe Dmitry Monakhov       2012-09-26  364  	 * is no longer required, try to drop it now. */
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  365) 	if ((folio_has_private(folio[0]) &&
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  366) 		!filemap_release_folio(folio[0], 0)) ||
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  367) 	    (folio_has_private(folio[1]) &&
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  368) 		!filemap_release_folio(folio[1], 0))) {
bb5574880574fe Dmitry Monakhov       2012-09-26  369  		*err = -EBUSY;
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  370) 		goto unlock_folios;
bb5574880574fe Dmitry Monakhov       2012-09-26  371  	}
6e2631463f3a2c Dmitry Monakhov       2014-07-27  372  	ext4_double_down_write_data_sem(orig_inode, donor_inode);
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  373  	replaced_count = ext4_swap_extents(handle, orig_inode, donor_inode,
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  374  					       orig_blk_offset, donor_blk_offset,
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  375  					   block_len_in_page, 1, err);
6e2631463f3a2c Dmitry Monakhov       2014-07-27  376  	ext4_double_up_write_data_sem(orig_inode, donor_inode);
bb5574880574fe Dmitry Monakhov       2012-09-26  377  	if (*err) {
f868a48d06f888 Akira Fujita          2009-11-23  378  		if (replaced_count) {
f868a48d06f888 Akira Fujita          2009-11-23  379  			block_len_in_page = replaced_count;
f868a48d06f888 Akira Fujita          2009-11-23  380  			replaced_size =
f868a48d06f888 Akira Fujita          2009-11-23  381  				block_len_in_page << orig_inode->i_blkbits;
ac48b0a1d06888 Akira Fujita          2009-11-24  382  		} else
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  383) 			goto unlock_folios;
748de6736c1e48 Akira Fujita          2009-06-17  384  	}
bb5574880574fe Dmitry Monakhov       2012-09-26  385  	/* Perform all necessary steps similar write_begin()/write_end()
bb5574880574fe Dmitry Monakhov       2012-09-26  386  	 * but keeping in mind that i_size will not change */
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  387) 	if (!folio_buffers(folio[0]))
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  388) 		create_empty_buffers(&folio[0]->page, 1 << orig_inode->i_blkbits, 0);
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  389) 	bh = folio_buffers(folio[0]);
bcff24887d00bc Eryu Guan             2016-02-12  390  	for (i = 0; i < data_offset_in_page; i++)
bcff24887d00bc Eryu Guan             2016-02-12  391  		bh = bh->b_this_page;
bcff24887d00bc Eryu Guan             2016-02-12  392  	for (i = 0; i < block_len_in_page; i++) {
bcff24887d00bc Eryu Guan             2016-02-12  393  		*err = ext4_get_block(orig_inode, orig_blk_offset + i, bh, 0);
bcff24887d00bc Eryu Guan             2016-02-12  394  		if (*err < 0)
bcff24887d00bc Eryu Guan             2016-02-12  395  			break;
6ffe77bad545f4 Eryu Guan             2016-02-21  396  		bh = bh->b_this_page;
bcff24887d00bc Eryu Guan             2016-02-12  397  	}
bb5574880574fe Dmitry Monakhov       2012-09-26  398  	if (!*err)
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17 @399) 		*err = block_commit_write(&folio[0]->page, from, from + replaced_size);
748de6736c1e48 Akira Fujita          2009-06-17  400  
bb5574880574fe Dmitry Monakhov       2012-09-26  401  	if (unlikely(*err < 0))
bb5574880574fe Dmitry Monakhov       2012-09-26  402  		goto repair_branches;
bb5574880574fe Dmitry Monakhov       2012-09-26  403  
bb5574880574fe Dmitry Monakhov       2012-09-26  404  	/* Even in case of data=writeback it is reasonable to pin
bb5574880574fe Dmitry Monakhov       2012-09-26  405  	 * inode to transaction, to prevent unexpected data loss */
73131fbb003b36 Ross Zwisler          2019-06-20  406  	*err = ext4_jbd2_inode_add_write(handle, orig_inode,
73131fbb003b36 Ross Zwisler          2019-06-20  407  			(loff_t)orig_page_offset << PAGE_SHIFT, replaced_size);
bb5574880574fe Dmitry Monakhov       2012-09-26  408  
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  409) unlock_folios:
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  410) 	folio_unlock(folio[0]);
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  411) 	folio_put(folio[0]);
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  412) 	folio_unlock(folio[1]);
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  413) 	folio_put(folio[1]);
bb5574880574fe Dmitry Monakhov       2012-09-26  414  stop_journal:
748de6736c1e48 Akira Fujita          2009-06-17  415  	ext4_journal_stop(handle);
88c6b61ff1cfb4 Dmitry Monakhov       2014-11-05  416  	if (*err == -ENOSPC &&
88c6b61ff1cfb4 Dmitry Monakhov       2014-11-05  417  	    ext4_should_retry_alloc(sb, &retries))
88c6b61ff1cfb4 Dmitry Monakhov       2014-11-05  418  		goto again;
bb5574880574fe Dmitry Monakhov       2012-09-26  419  	/* Buffer was busy because probably is pinned to journal transaction,
bb5574880574fe Dmitry Monakhov       2012-09-26  420  	 * force transaction commit may help to free it. */
88c6b61ff1cfb4 Dmitry Monakhov       2014-11-05  421  	if (*err == -EBUSY && retries++ < 4 && EXT4_SB(sb)->s_journal &&
88c6b61ff1cfb4 Dmitry Monakhov       2014-11-05  422  	    jbd2_journal_force_commit_nested(EXT4_SB(sb)->s_journal))
bb5574880574fe Dmitry Monakhov       2012-09-26  423  		goto again;
f868a48d06f888 Akira Fujita          2009-11-23  424  	return replaced_count;
bb5574880574fe Dmitry Monakhov       2012-09-26  425  
bb5574880574fe Dmitry Monakhov       2012-09-26  426  repair_branches:
bb5574880574fe Dmitry Monakhov       2012-09-26  427  	/*
bb5574880574fe Dmitry Monakhov       2012-09-26  428  	 * This should never ever happen!
bb5574880574fe Dmitry Monakhov       2012-09-26  429  	 * Extents are swapped already, but we are not able to copy data.
bb5574880574fe Dmitry Monakhov       2012-09-26  430  	 * Try to swap extents to it's original places
bb5574880574fe Dmitry Monakhov       2012-09-26  431  	 */
393d1d1d769338 Dr. Tilmann Bubeck    2013-04-08  432  	ext4_double_down_write_data_sem(orig_inode, donor_inode);
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  433  	replaced_count = ext4_swap_extents(handle, donor_inode, orig_inode,
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  434  					       orig_blk_offset, donor_blk_offset,
fcf6b1b729bcd2 Dmitry Monakhov       2014-08-30  435  					   block_len_in_page, 0, &err2);
393d1d1d769338 Dr. Tilmann Bubeck    2013-04-08  436  	ext4_double_up_write_data_sem(orig_inode, donor_inode);
bb5574880574fe Dmitry Monakhov       2012-09-26  437  	if (replaced_count != block_len_in_page) {
54d3adbc29f0c7 Theodore Ts'o         2020-03-28  438  		ext4_error_inode_block(orig_inode, (sector_t)(orig_blk_offset),
54d3adbc29f0c7 Theodore Ts'o         2020-03-28  439  				       EIO, "Unable to copy data block,"
bb5574880574fe Dmitry Monakhov       2012-09-26  440  				       " data will be lost.");
bb5574880574fe Dmitry Monakhov       2012-09-26  441  		*err = -EIO;
bb5574880574fe Dmitry Monakhov       2012-09-26  442  	}
bb5574880574fe Dmitry Monakhov       2012-09-26  443  	replaced_count = 0;
6dd8fe86fa8472 Vishal Moola (Oracle  2022-11-17  444) 	goto unlock_folios;
748de6736c1e48 Akira Fujita          2009-06-17  445  }
748de6736c1e48 Akira Fujita          2009-06-17  446  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 2/5] fs/buffer.c: convert block_commit_write to return void

[Jan Kara]

On Sun 18-06-23 23:32:47, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
> 
> block_commit_write() always returns 0, this patch changes it to
> return void.
> 
> Signed-off-by: Bean Huo <beanhuo@micron.com>

Looks good to me but you'll need to reorder this patch at the end of the
patch series to avoid breaking compilation in the middle of the series.
Otherwise feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/buffer.c                 | 11 +++++------
>  include/linux/buffer_head.h |  2 +-
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index b88bb7ec38be..fa09cf94f771 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2116,7 +2116,7 @@ int __block_write_begin(struct page *page, loff_t pos, unsigned len,
>  }
>  EXPORT_SYMBOL(__block_write_begin);
>  
> -int block_commit_write(struct page *page, unsigned int from, unsigned int to)
> +void block_commit_write(struct page *page, unsigned int from, unsigned int to)
>  {
>  	unsigned block_start, block_end;
>  	int partial = 0;
> @@ -2151,7 +2151,6 @@ int block_commit_write(struct page *page, unsigned int from, unsigned int to)
>  	 */
>  	if (!partial)
>  		SetPageUptodate(page);
> -	return 0;
>  }
>  EXPORT_SYMBOL(block_commit_write);
>  
> @@ -2577,11 +2576,11 @@ int block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
>  		end = PAGE_SIZE;
>  
>  	ret = __block_write_begin(page, 0, end, get_block);
> -	if (!ret)
> -		ret = block_commit_write(page, 0, end);
> -
> -	if (unlikely(ret < 0))
> +	if (unlikely(ret))
>  		goto out_unlock;
> +
> +	block_commit_write(page, 0, end);
> +
>  	set_page_dirty(page);
>  	wait_for_stable_page(page);
>  	return 0;
> diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
> index 1520793c72da..873653d2f1aa 100644
> --- a/include/linux/buffer_head.h
> +++ b/include/linux/buffer_head.h
> @@ -284,7 +284,7 @@ int cont_write_begin(struct file *, struct address_space *, loff_t,
>  			unsigned, struct page **, void **,
>  			get_block_t *, loff_t *);
>  int generic_cont_expand_simple(struct inode *inode, loff_t size);
> -int block_commit_write(struct page *page, unsigned from, unsigned to);
> +void block_commit_write(struct page *page, unsigned int from, unsigned int to);
>  int block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
>  				get_block_t get_block);
>  /* Convert errno to return value from ->page_mkwrite() call */
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v1 2/5] fs/buffer.c: convert block_commit_write to return void

[Bean Huo]

On Mon, 2023-06-19 at 11:56 +0200, Jan Kara wrote:
> Looks good to me but you'll need to reorder this patch at the end of
> the
> patch series to avoid breaking compilation in the middle of the
> series.
> Otherwise feel free to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>
> 
>                                                                 Honza
thanks Jan, I will reorder it in the v2.

Kind regards,
Bean

[PATCH v1 3/5] ext4: No need to check return value of block_commit_write()

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

Remove unnecessary check on the return value of block_commit_write().

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 fs/ext4/move_extent.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index b5af2fc03b2f..f4b4861a74ee 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -392,14 +392,11 @@ move_extent_per_page(struct file *o_filp, struct inode *donor_inode,
 	for (i = 0; i < block_len_in_page; i++) {
 		*err = ext4_get_block(orig_inode, orig_blk_offset + i, bh, 0);
 		if (*err < 0)
-			break;
+			goto repair_branches;
 		bh = bh->b_this_page;
 	}
-	if (!*err)
-		*err = block_commit_write(&folio[0]->page, from, from + replaced_size);
 
-	if (unlikely(*err < 0))
-		goto repair_branches;
+	block_commit_write(&folio[0]->page, from, from + replaced_size);
 
 	/* Even in case of data=writeback it is reasonable to pin
 	 * inode to transaction, to prevent unexpected data loss */
-- 
2.34.1

Re: [PATCH v1 3/5] ext4: No need to check return value of block_commit_write()

[Jan Kara]

On Sun 18-06-23 23:32:48, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
> 
> Remove unnecessary check on the return value of block_commit_write().
> 
> Signed-off-by: Bean Huo <beanhuo@micron.com>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/move_extent.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index b5af2fc03b2f..f4b4861a74ee 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -392,14 +392,11 @@ move_extent_per_page(struct file *o_filp, struct inode *donor_inode,
>  	for (i = 0; i < block_len_in_page; i++) {
>  		*err = ext4_get_block(orig_inode, orig_blk_offset + i, bh, 0);
>  		if (*err < 0)
> -			break;
> +			goto repair_branches;
>  		bh = bh->b_this_page;
>  	}
> -	if (!*err)
> -		*err = block_commit_write(&folio[0]->page, from, from + replaced_size);
>  
> -	if (unlikely(*err < 0))
> -		goto repair_branches;
> +	block_commit_write(&folio[0]->page, from, from + replaced_size);
>  
>  	/* Even in case of data=writeback it is reasonable to pin
>  	 * inode to transaction, to prevent unexpected data loss */
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v1 4/5] fs/ocfs2: No need to check return value of block_commit_write()

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

Remove unnecessary check on the return value of block_commit_write().

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 fs/ocfs2/file.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index efb09de4343d..39d8dbb26bb3 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -808,12 +808,7 @@ static int ocfs2_write_zero_page(struct inode *inode, u64 abs_from,
 
 
 		/* must not update i_size! */
-		ret = block_commit_write(page, block_start + 1,
-					 block_start + 1);
-		if (ret < 0)
-			mlog_errno(ret);
-		else
-			ret = 0;
+		block_commit_write(page, block_start + 1, block_start + 1);
 	}
 
 	/*
-- 
2.34.1

Re: [PATCH v1 4/5] fs/ocfs2: No need to check return value of block_commit_write()

[Jan Kara]

On Sun 18-06-23 23:32:49, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
> 
> Remove unnecessary check on the return value of block_commit_write().
> 
> Signed-off-by: Bean Huo <beanhuo@micron.com>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ocfs2/file.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index efb09de4343d..39d8dbb26bb3 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -808,12 +808,7 @@ static int ocfs2_write_zero_page(struct inode *inode, u64 abs_from,
>  
>  
>  		/* must not update i_size! */
> -		ret = block_commit_write(page, block_start + 1,
> -					 block_start + 1);
> -		if (ret < 0)
> -			mlog_errno(ret);
> -		else
> -			ret = 0;
> +		block_commit_write(page, block_start + 1, block_start + 1);
>  	}
>  
>  	/*
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v1 5/5] udf: No need to check return value of block_commit_write()

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

Remove unnecessary check on the return value of block_commit_write().

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 fs/udf/file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/udf/file.c b/fs/udf/file.c
index 8238f742377b..b1a062922a24 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -67,13 +67,13 @@ static vm_fault_t udf_page_mkwrite(struct vm_fault *vmf)
 	else
 		end = PAGE_SIZE;
 	err = __block_write_begin(page, 0, end, udf_get_block);
-	if (!err)
-		err = block_commit_write(page, 0, end);
-	if (err < 0) {
+	if (err) {
 		unlock_page(page);
 		ret = block_page_mkwrite_return(err);
 		goto out_unlock;
 	}
+
+	block_commit_write(page, 0, end);
 out_dirty:
 	set_page_dirty(page);
 	wait_for_stable_page(page);
-- 
2.34.1

Re: [PATCH v1 5/5] udf: No need to check return value of block_commit_write()

[Jan Kara]

On Sun 18-06-23 23:32:50, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
> 
> Remove unnecessary check on the return value of block_commit_write().
> 
> Signed-off-by: Bean Huo <beanhuo@micron.com>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/udf/file.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/udf/file.c b/fs/udf/file.c
> index 8238f742377b..b1a062922a24 100644
> --- a/fs/udf/file.c
> +++ b/fs/udf/file.c
> @@ -67,13 +67,13 @@ static vm_fault_t udf_page_mkwrite(struct vm_fault *vmf)
>  	else
>  		end = PAGE_SIZE;
>  	err = __block_write_begin(page, 0, end, udf_get_block);
> -	if (!err)
> -		err = block_commit_write(page, 0, end);
> -	if (err < 0) {
> +	if (err) {
>  		unlock_page(page);
>  		ret = block_page_mkwrite_return(err);
>  		goto out_unlock;
>  	}
> +
> +	block_commit_write(page, 0, end);
>  out_dirty:
>  	set_page_dirty(page);
>  	wait_for_stable_page(page);
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR