2023-06-26 06:04:12

by Bean Huo

[permalink] [raw]
Subject: [RESEND PATCH v3 0/2] clean up block_commit_write

change log:
v1--v2:
1. reordered patches

v2--v3:
1. rebased patches to git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next

Bean Huo (2):
fs/buffer: clean up block_commit_write
fs: convert block_commit_write to return void

fs/buffer.c | 20 ++++++++------------
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, 15 insertions(+), 27 deletions(-)

--
2.34.1



2023-06-26 06:05:27

by Bean Huo

[permalink] [raw]
Subject: [RESEND PATCH v3 2/2] fs: convert block_commit_write to return void

From: Bean Huo <[email protected]>

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

Signed-off-by: Bean Huo <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
Acked-by: Theodore Ts'o <[email protected]>
---
fs/buffer.c | 14 ++++++--------
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, 13 insertions(+), 23 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 50821dfb02f7..1568d0c9942d 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2180,7 +2180,7 @@ int __block_write_begin(struct page *page, loff_t pos, unsigned len,
}
EXPORT_SYMBOL(__block_write_begin);

-static int __block_commit_write(struct folio *folio, size_t from, size_t to)
+static void __block_commit_write(struct folio *folio, size_t from, size_t to)
{
size_t block_start, block_end;
bool partial = false;
@@ -2215,7 +2215,6 @@ static int __block_commit_write(struct folio *folio, size_t from, size_t to)
*/
if (!partial)
folio_mark_uptodate(folio);
- return 0;
}

/*
@@ -2597,11 +2596,10 @@ 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)
+void block_commit_write(struct page *page, unsigned from, unsigned to)
{
struct folio *folio = page_folio(page);
__block_commit_write(folio, from, to);
- return 0;
}
EXPORT_SYMBOL(block_commit_write);

@@ -2647,11 +2645,11 @@ int block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
end = size - folio_pos(folio);

ret = __block_write_begin_int(folio, 0, end, get_block, NULL);
- if (!ret)
- ret = __block_commit_write(folio, 0, end);
+ if (unlikely(ret))
+ goto out_unlock;
+
+ __block_commit_write(folio, 0, end);

- if (unlikely(ret < 0))
- goto out_unlock;
folio_mark_dirty(folio);
folio_wait_stable(folio);
return 0;
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 */
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 91a194596552..9e417cd4fd16 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);
}

/*
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 243840dc83ad..0292d75e60cc 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -63,13 +63,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);
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 6cb3e9af78c9..a7377877ff4e 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -288,7 +288,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


2023-06-26 12:25:50

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 2/2] fs: convert block_commit_write to return void

On Mon, Jun 26, 2023 at 07:55:18AM +0200, Bean Huo wrote:
> From: Bean Huo <[email protected]>
>
> block_commit_write() always returns 0, this patch changes it to
> return void.
>
> Signed-off-by: Bean Huo <[email protected]>
> Reviewed-by: Jan Kara <[email protected]>
> Acked-by: Theodore Ts'o <[email protected]>

Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>


2023-06-26 12:29:18

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 0/2] clean up block_commit_write

On Mon, Jun 26, 2023 at 07:55:16AM +0200, Bean Huo wrote:
> change log:
> v1--v2:
> 1. reordered patches
>
> v2--v3:
> 1. rebased patches to git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next

It's be nice to have a bit of an explanation for the whole series here,
but I think the two patches work standalone.

If you'd like to extend this work, you could convert the callers of
block_commit_write() to use a folio instead of a page and then unify
block_commit_write() and __block_commit_write() as you did in the earlier
version of your patchset. It shouldn't be too hard, both callers in
ext4 and the caller in iomap are already done. That just leaves the
three callers in ocfs2 and the one caller in udf.