| Message ID | 20230309090526.332550-4-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/20] btrfs: mark extent_buffer_under_io static | expand |
On 09.03.23 10:07, Christoph Hellwig wrote: > verify_parent_transid is only called by btrfs_buffer_uptodate, which > confusingly inverts the return value. Merge the two functions and > reflow the parent_transid so that error handling is in a branch. This would be a good chance to make btrfs_buffer_uptodate() a bool function. Otherwise, Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
On Thu, Mar 09, 2023 at 11:17:47AM +0000, Johannes Thumshirn wrote: > On 09.03.23 10:07, Christoph Hellwig wrote: > > verify_parent_transid is only called by btrfs_buffer_uptodate, which > > confusingly inverts the return value. Merge the two functions and > > reflow the parent_transid so that error handling is in a branch. > > This would be a good chance to make btrfs_buffer_uptodate() a bool > function. It still can return 0/1/-EAGAIN.
On 2023/3/9 17:05, Christoph Hellwig wrote: > verify_parent_transid is only called by btrfs_buffer_uptodate, which > confusingly inverts the return value. Merge the two functions and > reflow the parent_transid so that error handling is in a branch. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/disk-io.c | 46 +++++++++++++++------------------------------- > 1 file changed, 15 insertions(+), 31 deletions(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 7d766eaef4aee7..d03b431b07781c 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -110,32 +110,33 @@ static void csum_tree_block(struct extent_buffer *buf, u8 *result) > * detect blocks that either didn't get written at all or got written > * in the wrong place. > */ > -static int verify_parent_transid(struct extent_io_tree *io_tree, > - struct extent_buffer *eb, u64 parent_transid, > - int atomic) > +int btrfs_buffer_uptodate(struct extent_buffer *eb, u64 parent_transid, > + int atomic) > { > + struct inode *btree_inode = eb->pages[0]->mapping->host; > + struct extent_io_tree *io_tree = &BTRFS_I(btree_inode)->io_tree; > struct extent_state *cached_state = NULL; > - int ret; > + int ret = 1; > > - if (!parent_transid || btrfs_header_generation(eb) == parent_transid) > + if (!extent_buffer_uptodate(eb)) > return 0; > > + if (!parent_transid || btrfs_header_generation(eb) == parent_transid) > + return 1; > + > if (atomic) > return -EAGAIN; > > lock_extent(io_tree, eb->start, eb->start + eb->len - 1, &cached_state); > - if (extent_buffer_uptodate(eb) && > - btrfs_header_generation(eb) == parent_transid) { > - ret = 0; > - goto out; > - } > - btrfs_err_rl(eb->fs_info, > + if (!extent_buffer_uptodate(eb) || > + btrfs_header_generation(eb) != parent_transid) { > + btrfs_err_rl(eb->fs_info, > "parent transid verify failed on logical %llu mirror %u wanted %llu found %llu", > eb->start, eb->read_mirror, > parent_transid, btrfs_header_generation(eb)); > - ret = 1; > - clear_extent_buffer_uptodate(eb); > -out: > + clear_extent_buffer_uptodate(eb); > + ret = 0; > + } > unlock_extent(io_tree, eb->start, eb->start + eb->len - 1, > &cached_state); > return ret; > @@ -4638,23 +4639,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info) > btrfs_close_devices(fs_info->fs_devices); > } > > -int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid, > - int atomic) > -{ > - int ret; > - struct inode *btree_inode = buf->pages[0]->mapping->host; > - > - ret = extent_buffer_uptodate(buf); > - if (!ret) > - return ret; > - > - ret = verify_parent_transid(&BTRFS_I(btree_inode)->io_tree, buf, > - parent_transid, atomic); > - if (ret == -EAGAIN) > - return ret; > - return !ret; > -} > - > void btrfs_mark_buffer_dirty(struct extent_buffer *buf) > { > struct btrfs_fs_info *fs_info = buf->fs_info;
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 7d766eaef4aee7..d03b431b07781c 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -110,32 +110,33 @@ static void csum_tree_block(struct extent_buffer *buf, u8 *result) * detect blocks that either didn't get written at all or got written * in the wrong place. */ -static int verify_parent_transid(struct extent_io_tree *io_tree, - struct extent_buffer *eb, u64 parent_transid, - int atomic) +int btrfs_buffer_uptodate(struct extent_buffer *eb, u64 parent_transid, + int atomic) { + struct inode *btree_inode = eb->pages[0]->mapping->host; + struct extent_io_tree *io_tree = &BTRFS_I(btree_inode)->io_tree; struct extent_state *cached_state = NULL; - int ret; + int ret = 1; - if (!parent_transid || btrfs_header_generation(eb) == parent_transid) + if (!extent_buffer_uptodate(eb)) return 0; + if (!parent_transid || btrfs_header_generation(eb) == parent_transid) + return 1; + if (atomic) return -EAGAIN; lock_extent(io_tree, eb->start, eb->start + eb->len - 1, &cached_state); - if (extent_buffer_uptodate(eb) && - btrfs_header_generation(eb) == parent_transid) { - ret = 0; - goto out; - } - btrfs_err_rl(eb->fs_info, + if (!extent_buffer_uptodate(eb) || + btrfs_header_generation(eb) != parent_transid) { + btrfs_err_rl(eb->fs_info, "parent transid verify failed on logical %llu mirror %u wanted %llu found %llu", eb->start, eb->read_mirror, parent_transid, btrfs_header_generation(eb)); - ret = 1; - clear_extent_buffer_uptodate(eb); -out: + clear_extent_buffer_uptodate(eb); + ret = 0; + } unlock_extent(io_tree, eb->start, eb->start + eb->len - 1, &cached_state); return ret; @@ -4638,23 +4639,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info) btrfs_close_devices(fs_info->fs_devices); } -int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid, - int atomic) -{ - int ret; - struct inode *btree_inode = buf->pages[0]->mapping->host; - - ret = extent_buffer_uptodate(buf); - if (!ret) - return ret; - - ret = verify_parent_transid(&BTRFS_I(btree_inode)->io_tree, buf, - parent_transid, atomic); - if (ret == -EAGAIN) - return ret; - return !ret; -} - void btrfs_mark_buffer_dirty(struct extent_buffer *buf) { struct btrfs_fs_info *fs_info = buf->fs_info;
verify_parent_transid is only called by btrfs_buffer_uptodate, which confusingly inverts the return value. Merge the two functions and reflow the parent_transid so that error handling is in a branch. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/disk-io.c | 46 +++++++++++++++------------------------------- 1 file changed, 15 insertions(+), 31 deletions(-)