[PATCH 0/2] btrfs: minor cleanup for subpage helpers

[PATCH 0/2] btrfs: minor cleanup for subpage helpers

[Qu Wenruo]

The first patch removes dead "unlikely(!fs_info)" checks, as commit
b2136cc288fc ("btrfs: tests: allocate dummy fs_info and root in
test_find_delalloc()") has properly allocated a dummy fs_info for
extent-io-tests.

The second patch is inspired by Boris' new cow fixup, where we can
determine if a folio belongs to data or metadata.
As the only special case is when the folio has no mapping. Meanwhile for
data folios they are always from page cache, thus they are always
mapped.
So an unmapped folio must belongs to metadata, for dummy ebs.

This allows us to enhance btrfs_is_subpage() to cover both data and
metadata, now btrfs_meta_folio_*() helpers are just a simple wrapper for
the correponding btrfs_folio_*() helpers.

Qu Wenruo (2):
  btrfs: remove NULL fs_info checks in subpage helpers
  btrfs: unify data and metadata subpage handling

 fs/btrfs/subpage.c | 50 +++++++++-------------------------------------
 fs/btrfs/subpage.h | 45 +++++++++++++++++++++++++++++------------
 2 files changed, 41 insertions(+), 54 deletions(-)

-- 
2.54.0

[PATCH 1/2] btrfs: remove NULL fs_info checks in subpage helpers

[Qu Wenruo]

We have several "unlikely(!fs_info)" checks inside subpage helpers, this
behavior was to avoid NULL pointer dereference for extent-io-tests,
where we have a dummy inode for testing but without an fs_info/root for
that testing inode.

However commit b2136cc288fc ("btrfs: tests: allocate dummy fs_info and
root in test_find_delalloc()") changed the test to properly allocate an
fs_info and root, so even for that selftest, we will have a proper
fs_info for btrfs_folio_*() helpers.

So there is no need to do that "unlikely(!fs_info)" check, as it will
always return false now.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/subpage.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/subpage.c b/fs/btrfs/subpage.c
index ebf18efe1ea3..72402ea156d3 100644
--- a/fs/btrfs/subpage.c
+++ b/fs/btrfs/subpage.c
@@ -246,7 +246,7 @@ void btrfs_folio_end_lock(const struct btrfs_fs_info *fs_info,
 
 	ASSERT(folio_test_locked(folio));
 
-	if (unlikely(!fs_info) || !btrfs_is_subpage(fs_info, folio)) {
+	if (!btrfs_is_subpage(fs_info, folio)) {
 		folio_unlock(folio);
 		return;
 	}
@@ -688,18 +688,12 @@ IMPLEMENT_BTRFS_SUBPAGE_TEST_OP(dirty);
 IMPLEMENT_BTRFS_SUBPAGE_TEST_OP(writeback);
 IMPLEMENT_BTRFS_SUBPAGE_TEST_OP(fixup);
 
-/*
- * Note that, in selftests (extent-io-tests), we can have empty fs_info passed
- * in.  We only test sectorsize == PAGE_SIZE cases so far, thus we can fall
- * back to regular sectorsize branch.
- */
 #define IMPLEMENT_BTRFS_PAGE_OPS(name, folio_set_func,			\
 				 folio_clear_func, folio_test_func)	\
 void btrfs_folio_set_##name(const struct btrfs_fs_info *fs_info,	\
 			    struct folio *folio, u64 start, u32 len)	\
 {									\
-	if (unlikely(!fs_info) ||					\
-	    !btrfs_is_subpage(fs_info, folio)) {			\
+	if (!btrfs_is_subpage(fs_info, folio)) {			\
 		folio_set_func(folio);					\
 		return;							\
 	}								\
@@ -708,8 +702,7 @@ void btrfs_folio_set_##name(const struct btrfs_fs_info *fs_info,	\
 void btrfs_folio_clear_##name(const struct btrfs_fs_info *fs_info,	\
 			      struct folio *folio, u64 start, u32 len)	\
 {									\
-	if (unlikely(!fs_info) ||					\
-	    !btrfs_is_subpage(fs_info, folio)) {			\
+	if (!btrfs_is_subpage(fs_info, folio)) {			\
 		folio_clear_func(folio);				\
 		return;							\
 	}								\
@@ -718,16 +711,14 @@ void btrfs_folio_clear_##name(const struct btrfs_fs_info *fs_info,	\
 bool btrfs_folio_test_##name(const struct btrfs_fs_info *fs_info,	\
 			     struct folio *folio, u64 start, u32 len)	\
 {									\
-	if (unlikely(!fs_info) ||					\
-	    !btrfs_is_subpage(fs_info, folio))				\
+	if (!btrfs_is_subpage(fs_info, folio))				\
 		return folio_test_func(folio);				\
 	return btrfs_subpage_test_##name(fs_info, folio, start, len);	\
 }									\
 void btrfs_folio_clamp_set_##name(const struct btrfs_fs_info *fs_info,	\
 				  struct folio *folio, u64 start, u32 len) \
 {									\
-	if (unlikely(!fs_info) ||					\
-	    !btrfs_is_subpage(fs_info, folio)) {			\
+	if (!btrfs_is_subpage(fs_info, folio)) {			\
 		folio_set_func(folio);					\
 		return;							\
 	}								\
@@ -737,8 +728,7 @@ void btrfs_folio_clamp_set_##name(const struct btrfs_fs_info *fs_info,	\
 void btrfs_folio_clamp_clear_##name(const struct btrfs_fs_info *fs_info, \
 				    struct folio *folio, u64 start, u32 len) \
 {									\
-	if (unlikely(!fs_info) ||					\
-	    !btrfs_is_subpage(fs_info, folio)) {			\
+	if (!btrfs_is_subpage(fs_info, folio)) {			\
 		folio_clear_func(folio);				\
 		return;							\
 	}								\
@@ -748,8 +738,7 @@ void btrfs_folio_clamp_clear_##name(const struct btrfs_fs_info *fs_info, \
 bool btrfs_folio_clamp_test_##name(const struct btrfs_fs_info *fs_info,	\
 				   struct folio *folio, u64 start, u32 len) \
 {									\
-	if (unlikely(!fs_info) ||					\
-	    !btrfs_is_subpage(fs_info, folio))				\
+	if (!btrfs_is_subpage(fs_info, folio))				\
 		return folio_test_func(folio);				\
 	btrfs_subpage_clamp_range(folio, &start, &len);			\
 	return btrfs_subpage_test_##name(fs_info, folio, start, len);	\
@@ -882,7 +871,7 @@ void btrfs_folio_set_lock(const struct btrfs_fs_info *fs_info,
 	int ret;
 
 	ASSERT(folio_test_locked(folio));
-	if (unlikely(!fs_info) || !btrfs_is_subpage(fs_info, folio))
+	if (!btrfs_is_subpage(fs_info, folio))
 		return;
 
 	bfs = folio_get_private(folio);
-- 
2.54.0

[PATCH 2/2] btrfs: unify data and metadata subpage handling

[Qu Wenruo]

Currently we have split data and metadata subpage handling, this is due
to the fact that data and metadata have very different conditions to
determine if a folio needs subpage handling.

But the condition different is not that huge, it's just checking
different values.
For data it's checking if the sectorsize is smaller than folio size, for
metadata it doesn't even need to bother folio size yet, as we have not
yet support large folios for metadata.

The idea is to merge btrfs_meta_is_subpage() into btrfs_is_subpage(),
so that btrfs_is_subpage() is the only entrance to check if a folio
needs subpage handling.

With that said, all the existing btrfs_meta_folio_*() helpers can be
updated to a very simple wrapper to the corresponding btrfs_folio_*()
helper.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/subpage.c | 23 +----------------------
 fs/btrfs/subpage.h | 45 ++++++++++++++++++++++++++++++++-------------
 2 files changed, 33 insertions(+), 35 deletions(-)

diff --git a/fs/btrfs/subpage.c b/fs/btrfs/subpage.c
index 72402ea156d3..7e760934cab8 100644
--- a/fs/btrfs/subpage.c
+++ b/fs/btrfs/subpage.c
@@ -742,29 +742,8 @@ bool btrfs_folio_clamp_test_##name(const struct btrfs_fs_info *fs_info,	\
 		return folio_test_func(folio);				\
 	btrfs_subpage_clamp_range(folio, &start, &len);			\
 	return btrfs_subpage_test_##name(fs_info, folio, start, len);	\
-}									\
-void btrfs_meta_folio_set_##name(struct folio *folio, const struct extent_buffer *eb) \
-{									\
-	if (!btrfs_meta_is_subpage(eb->fs_info)) {			\
-		folio_set_func(folio);					\
-		return;							\
-	}								\
-	btrfs_subpage_set_##name(eb->fs_info, folio, eb->start, eb->len); \
-}									\
-void btrfs_meta_folio_clear_##name(struct folio *folio, const struct extent_buffer *eb) \
-{									\
-	if (!btrfs_meta_is_subpage(eb->fs_info)) {			\
-		folio_clear_func(folio);				\
-		return;							\
-	}								\
-	btrfs_subpage_clear_##name(eb->fs_info, folio, eb->start, eb->len); \
-}									\
-bool btrfs_meta_folio_test_##name(struct folio *folio, const struct extent_buffer *eb) \
-{									\
-	if (!btrfs_meta_is_subpage(eb->fs_info))			\
-		return folio_test_func(folio);				\
-	return btrfs_subpage_test_##name(eb->fs_info, folio, eb->start, eb->len); \
 }
+
 IMPLEMENT_BTRFS_PAGE_OPS(uptodate, folio_mark_uptodate, folio_clear_uptodate,
 			 folio_test_uptodate);
 IMPLEMENT_BTRFS_PAGE_OPS(dirty, btrfs_folio_mark_dirty_reserved,
diff --git a/fs/btrfs/subpage.h b/fs/btrfs/subpage.h
index 9b106a73d682..7e25d8d1d3bb 100644
--- a/fs/btrfs/subpage.h
+++ b/fs/btrfs/subpage.h
@@ -95,11 +95,23 @@ static inline bool btrfs_meta_is_subpage(const struct btrfs_fs_info *fs_info)
 {
 	return fs_info->nodesize < PAGE_SIZE;
 }
-static inline bool btrfs_is_subpage(const struct btrfs_fs_info *fs_info,
-				    struct folio *folio)
+
+static inline bool btrfs_folio_is_data(const struct folio *folio)
 {
-	if (folio->mapping && folio->mapping->host)
-		ASSERT(is_data_inode(BTRFS_I(folio->mapping->host)));
+	const struct address_space *mapping = folio_mapping(folio);
+
+	/* Only metadata folio can have no mapping for dummy ebs. */
+	if (!mapping || !mapping->host)
+		return false;
+	return is_data_inode(BTRFS_I(mapping->host));
+}
+
+/* This helper can be called on both data and metadata folios. */
+static inline bool btrfs_is_subpage(const struct btrfs_fs_info *fs_info,
+				    const struct folio *folio)
+{
+	if (!btrfs_folio_is_data(folio))
+		return btrfs_meta_is_subpage(fs_info);
 	return fs_info->sectorsize < folio_size(folio);
 }
 
@@ -140,12 +152,7 @@ void btrfs_folio_end_lock_bitmap(const struct btrfs_fs_info *fs_info,
  * need to be inside the page. Those functions will truncate the range
  * automatically.
  *
- * Both btrfs_folio_*() and btrfs_folio_clamp_*() are for data folios.
- *
- * For metadata, one should use btrfs_meta_folio_*() helpers instead, and there
- * is no clamp version for metadata helpers, as we either go subpage
- * (nodesize < PAGE_SIZE) or go regular folio helpers (nodesize >= PAGE_SIZE,
- * and our folio is never larger than nodesize).
+ * All helpers can be called on both data and metadata folios.
  */
 #define DECLARE_BTRFS_SUBPAGE_OPS(name)					\
 void btrfs_subpage_set_##name(const struct btrfs_fs_info *fs_info,	\
@@ -166,9 +173,21 @@ void btrfs_folio_clamp_clear_##name(const struct btrfs_fs_info *fs_info,	\
 		struct folio *folio, u64 start, u32 len);			\
 bool btrfs_folio_clamp_test_##name(const struct btrfs_fs_info *fs_info,	\
 		struct folio *folio, u64 start, u32 len);		\
-void btrfs_meta_folio_set_##name(struct folio *folio, const struct extent_buffer *eb); \
-void btrfs_meta_folio_clear_##name(struct folio *folio, const struct extent_buffer *eb); \
-bool btrfs_meta_folio_test_##name(struct folio *folio, const struct extent_buffer *eb);
+static inline void btrfs_meta_folio_set_##name(struct folio *folio,		\
+					       const struct extent_buffer *eb)	\
+{										\
+	btrfs_folio_set_##name(eb->fs_info, folio, eb->start, eb->len);		\
+}										\
+static inline void btrfs_meta_folio_clear_##name(struct folio *folio,		\
+						 const struct extent_buffer *eb) \
+{										\
+	btrfs_folio_clear_##name(eb->fs_info, folio, eb->start, eb->len);	\
+}										\
+static inline bool btrfs_meta_folio_test_##name(struct folio *folio,		\
+				  const struct extent_buffer *eb)		\
+{										\
+	return btrfs_folio_test_##name(eb->fs_info, folio, eb->start, eb->len);	\
+}
 
 DECLARE_BTRFS_SUBPAGE_OPS(uptodate);
 DECLARE_BTRFS_SUBPAGE_OPS(dirty);
-- 
2.54.0