On 2021/1/19 上午6:46, David Sterba wrote:
On Sat, Jan 16, 2021 at 03:15:18PM +0800, Qu Wenruo wrote:
For btrfs subpage support, we need a structure to record extra info for
the status of each sectors of a page.

This patch will introduce the skeleton structure for future btrfs
subpage support.
All subpage related code would go to subpage.[ch] to avoid populating
the existing code base.

Reviewed-by: Josef Bacik <jo...@toxicpanda.com>
Signed-off-by: Qu Wenruo <w...@suse.com>
---
  fs/btrfs/Makefile  |  3 ++-
  fs/btrfs/subpage.c | 39 +++++++++++++++++++++++++++++++++++++++
  fs/btrfs/subpage.h | 31 +++++++++++++++++++++++++++++++
  3 files changed, 72 insertions(+), 1 deletion(-)
  create mode 100644 fs/btrfs/subpage.c
  create mode 100644 fs/btrfs/subpage.h

diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index 9f1b1a88e317..942562e11456 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -11,7 +11,8 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o 
root-tree.o dir-item.o \
           compression.o delayed-ref.o relocation.o delayed-inode.o scrub.o \
           reada.o backref.o ulist.o qgroup.o send.o dev-replace.o raid56.o \
           uuid-tree.o props.o free-space-tree.o tree-checker.o space-info.o \
-          block-rsv.o delalloc-space.o block-group.o discard.o reflink.o
+          block-rsv.o delalloc-space.o block-group.o discard.o reflink.o \
+          subpage.o

  btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o
  btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
diff --git a/fs/btrfs/subpage.c b/fs/btrfs/subpage.c
new file mode 100644
index 000000000000..c6ab32db3995
--- /dev/null
+++ b/fs/btrfs/subpage.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "subpage.h"
+
+int btrfs_attach_subpage(struct btrfs_fs_info *fs_info, struct page *page)
+{
+       struct btrfs_subpage *subpage;
+
+       /*
+        * We have cases like a dummy extent buffer page, which is not
+        * mappped and doesn't need to be locked.
+        */
+       if (page->mapping)
+               ASSERT(PageLocked(page));
+       /* Either not subpage, or the page already has private attached */
+       if (fs_info->sectorsize == PAGE_SIZE || PagePrivate(page))
+               return 0;
+
+       subpage = kzalloc(sizeof(*subpage), GFP_NOFS);
+       if (!subpage)
+               return -ENOMEM;
+
+       spin_lock_init(&subpage->lock);
+       attach_page_private(page, subpage);
+       return 0;
+}
+
+void btrfs_detach_subpage(struct btrfs_fs_info *fs_info, struct page *page)
+{
+       struct btrfs_subpage *subpage;
+
+       /* Either not subpage, or already detached */
+       if (fs_info->sectorsize == PAGE_SIZE || !PagePrivate(page))
+               return;
+
+       subpage = (struct btrfs_subpage *)detach_page_private(page);
+       ASSERT(subpage);
+       kfree(subpage);
+}
diff --git a/fs/btrfs/subpage.h b/fs/btrfs/subpage.h
new file mode 100644
index 000000000000..96f3b226913e
--- /dev/null
+++ b/fs/btrfs/subpage.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef BTRFS_SUBPAGE_H
+#define BTRFS_SUBPAGE_H
+
+#include <linux/spinlock.h>
+#include "ctree.h"

So subpage.h would pull the whole ctree.h, that's not very nice. If
anything, the .c could include ctree.h because there are lots of the
common structure and function definitions, but not the .h. This creates
unnecessary include dependencies.

Any pointer type you'd need in structures could be forward declared.

Unfortunately, the main needed pointer is fs_info, and we're accessing
it pretty frequently (mostly for sector/node size).

I don't believe forward declaration would help in this case.

Thanks,
Qu

+
+/*
+ * Since the maximum page size btrfs is going to support is 64K while the
+ * minimum sectorsize is 4K, this means a u16 bitmap is enough.
+ *
+ * The regular bitmap requires 32 bits as minimal bitmap size, so we can't use
+ * existing bitmap_* helpers here.
+ */
+#define BTRFS_SUBPAGE_BITMAP_SIZE      16
+
+/*
+ * Structure to trace status of each sector inside a page.
+ *
+ * Will be attached to page::private for both data and metadata inodes.
+ */
+struct btrfs_subpage {
+       /* Common members for both data and metadata pages */
+       spinlock_t lock;
+};
+
+int btrfs_attach_subpage(struct btrfs_fs_info *fs_info, struct page *page);
+void btrfs_detach_subpage(struct btrfs_fs_info *fs_info, struct page *page);
+
+#endif /* BTRFS_SUBPAGE_H */
--
2.30.0

Reply via email to