| Message ID | e86267202871b02aad2359dc42aab05e96102aaa.1689061099.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: preparation patches for the incoming metadata folio conversion | expand |
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 6a7abcbe6bec..fef5a7b6c60a 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -4178,23 +4178,16 @@ static void assert_eb_page_uptodate(const struct extent_buffer *eb, > void write_extent_buffer_chunk_tree_uuid(const struct extent_buffer *eb, > const void *srcv) > { > - char *kaddr; > - > - assert_eb_page_uptodate(eb, eb->pages[0]); > - kaddr = page_address(eb->pages[0]) + > - get_eb_offset_in_page(eb, offsetof(struct btrfs_header, > - chunk_tree_uuid)); > - memcpy(kaddr, srcv, BTRFS_FSID_SIZE); > + write_extent_buffer(eb, srcv, > + offsetof(struct btrfs_header, chunk_tree_uuid), > + BTRFS_FSID_SIZE); > } > > void write_extent_buffer_fsid(const struct extent_buffer *eb, const void *srcv) > { > - char *kaddr; > > - assert_eb_page_uptodate(eb, eb->pages[0]); > - kaddr = page_address(eb->pages[0]) + > - get_eb_offset_in_page(eb, offsetof(struct btrfs_header, fsid)); > - memcpy(kaddr, srcv, BTRFS_FSID_SIZE); > + write_extent_buffer(eb, srcv, offsetof(struct btrfs_header, fsid), > + BTRFS_FSID_SIZE); > } > > void write_extent_buffer(const struct extent_buffer *eb, const void *srcv, write_extent_buffer_chunk_tree_uuid() has only one caller in kernel now; perhaps inline the function into its only callsite? On the other hand, it has several more in -progs, so maybe the name is useful and it could be moved it into extent_io.h since it's such a thin wrapper around write_extent_buffer()? write_extent_buffer_fsid() has three in kernel and five in -progs, maybe also into the .h?
On 2023/7/12 01:02, Sweet Tea Dorminy wrote: > >> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c >> index 6a7abcbe6bec..fef5a7b6c60a 100644 >> --- a/fs/btrfs/extent_io.c >> +++ b/fs/btrfs/extent_io.c >> @@ -4178,23 +4178,16 @@ static void assert_eb_page_uptodate(const >> struct extent_buffer *eb, >> void write_extent_buffer_chunk_tree_uuid(const struct extent_buffer >> *eb, >> const void *srcv) >> { >> - char *kaddr; >> - >> - assert_eb_page_uptodate(eb, eb->pages[0]); >> - kaddr = page_address(eb->pages[0]) + >> - get_eb_offset_in_page(eb, offsetof(struct btrfs_header, >> - chunk_tree_uuid)); >> - memcpy(kaddr, srcv, BTRFS_FSID_SIZE); >> + write_extent_buffer(eb, srcv, >> + offsetof(struct btrfs_header, chunk_tree_uuid), >> + BTRFS_FSID_SIZE); >> } >> void write_extent_buffer_fsid(const struct extent_buffer *eb, const >> void *srcv) >> { >> - char *kaddr; >> - assert_eb_page_uptodate(eb, eb->pages[0]); >> - kaddr = page_address(eb->pages[0]) + >> - get_eb_offset_in_page(eb, offsetof(struct btrfs_header, fsid)); >> - memcpy(kaddr, srcv, BTRFS_FSID_SIZE); >> + write_extent_buffer(eb, srcv, offsetof(struct btrfs_header, fsid), >> + BTRFS_FSID_SIZE); >> } >> void write_extent_buffer(const struct extent_buffer *eb, const void >> *srcv, > > write_extent_buffer_chunk_tree_uuid() has only one caller in kernel now; > perhaps inline the function into its only callsite? On the other hand, > it has several more in -progs, so maybe the name is useful and it could > be moved it into extent_io.h since it's such a thin wrapper around > write_extent_buffer()? > > write_extent_buffer_fsid() has three in kernel and five in -progs, maybe > also into the .h? This sound very reasonable, would go this direction in the next update. Thanks, Qu
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 6a7abcbe6bec..fef5a7b6c60a 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -4178,23 +4178,16 @@ static void assert_eb_page_uptodate(const struct extent_buffer *eb, void write_extent_buffer_chunk_tree_uuid(const struct extent_buffer *eb, const void *srcv) { - char *kaddr; - - assert_eb_page_uptodate(eb, eb->pages[0]); - kaddr = page_address(eb->pages[0]) + - get_eb_offset_in_page(eb, offsetof(struct btrfs_header, - chunk_tree_uuid)); - memcpy(kaddr, srcv, BTRFS_FSID_SIZE); + write_extent_buffer(eb, srcv, + offsetof(struct btrfs_header, chunk_tree_uuid), + BTRFS_FSID_SIZE); } void write_extent_buffer_fsid(const struct extent_buffer *eb, const void *srcv) { - char *kaddr; - assert_eb_page_uptodate(eb, eb->pages[0]); - kaddr = page_address(eb->pages[0]) + - get_eb_offset_in_page(eb, offsetof(struct btrfs_header, fsid)); - memcpy(kaddr, srcv, BTRFS_FSID_SIZE); + write_extent_buffer(eb, srcv, offsetof(struct btrfs_header, fsid), + BTRFS_FSID_SIZE); } void write_extent_buffer(const struct extent_buffer *eb, const void *srcv,
For helpers write_extent_buffer_chunk_tree_uuid() and write_extent_buffer_fsid(), they can be implemented by write_extent_buffer(). And those two helpers are not that hot, they only get called during initialization of a new tree block. There is not much need for those slightly optimized versions. This would make later page/folio switch much easier, as all change only need to happen in write_extent_buffer(). Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/extent_io.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-)