* Re: [PATCH v2 16/19] btrfs: use extent_io_tree to handle subpage extent buffer allocation
@ 2020-09-15 10:17 kernel test robot
0 siblings, 0 replies; 2+ messages in thread
From: kernel test robot @ 2020-09-15 10:17 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 4937 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20200915053532.63279-17-wqu@suse.com>
References: <20200915053532.63279-17-wqu@suse.com>
TO: Qu Wenruo <wqu@suse.com>
TO: linux-btrfs(a)vger.kernel.org
Hi Qu,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v5.9-rc5]
[also build test WARNING on next-20200915]
[cannot apply to kdave/for-next btrfs/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-add-read-only-support-for-subpage-sector-size/20200915-133811
base: 856deb866d16e29bd65952e0289066f6078af773
:::::: branch date: 5 hours ago
:::::: commit date: 5 hours ago
config: x86_64-randconfig-s022-20200914 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.2-191-g10164920-dirty
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
fs/btrfs/extent_io.c:2327:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
fs/btrfs/extent_io.c:2327:9: sparse: struct rcu_string [noderef] __rcu *
fs/btrfs/extent_io.c:2327:9: sparse: struct rcu_string *
>> fs/btrfs/extent_io.c:4960:13: sparse: sparse: context imbalance in 'detach_extent_buffer_subpage' - different lock contexts for basic block
fs/btrfs/extent_io.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/slab.h):
include/linux/page-flags.h:182:30: sparse: sparse: context imbalance in 'btrfs_release_extent_buffer_pages' - different lock contexts for basic block
# https://github.com/0day-ci/linux/commit/e501596ccc2e9c62fe48af688b4f89376d799864
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Qu-Wenruo/btrfs-add-read-only-support-for-subpage-sector-size/20200915-133811
git checkout e501596ccc2e9c62fe48af688b4f89376d799864
vim +/detach_extent_buffer_subpage +4960 fs/btrfs/extent_io.c
db7f3436c1c186 Josef Bacik 2013-08-07 4959
e501596ccc2e9c Qu Wenruo 2020-09-15 @4960 static void detach_extent_buffer_subpage(struct extent_buffer *eb)
e501596ccc2e9c Qu Wenruo 2020-09-15 4961 {
e501596ccc2e9c Qu Wenruo 2020-09-15 4962 struct btrfs_fs_info *fs_info = eb->fs_info;
e501596ccc2e9c Qu Wenruo 2020-09-15 4963 struct extent_io_tree *io_tree = info_to_btree_io_tree(fs_info);
e501596ccc2e9c Qu Wenruo 2020-09-15 4964 struct page *page = eb->pages[0];
e501596ccc2e9c Qu Wenruo 2020-09-15 4965 bool mapped = !test_bit(EXTENT_BUFFER_UNMAPPED, &eb->bflags);
e501596ccc2e9c Qu Wenruo 2020-09-15 4966 int ret;
e501596ccc2e9c Qu Wenruo 2020-09-15 4967
e501596ccc2e9c Qu Wenruo 2020-09-15 4968 if (!page)
e501596ccc2e9c Qu Wenruo 2020-09-15 4969 return;
e501596ccc2e9c Qu Wenruo 2020-09-15 4970
e501596ccc2e9c Qu Wenruo 2020-09-15 4971 if (mapped)
e501596ccc2e9c Qu Wenruo 2020-09-15 4972 spin_lock(&page->mapping->private_lock);
e501596ccc2e9c Qu Wenruo 2020-09-15 4973
e501596ccc2e9c Qu Wenruo 2020-09-15 4974 /*
e501596ccc2e9c Qu Wenruo 2020-09-15 4975 * Clear the EXTENT_NEW bit from io tree, to indicate that there is
e501596ccc2e9c Qu Wenruo 2020-09-15 4976 * no longer an extent buffer in the range.
e501596ccc2e9c Qu Wenruo 2020-09-15 4977 */
e501596ccc2e9c Qu Wenruo 2020-09-15 4978 __clear_extent_bit(io_tree, eb->start, eb->start + eb->len - 1,
e501596ccc2e9c Qu Wenruo 2020-09-15 4979 EXTENT_NEW, 0, 0, NULL, GFP_ATOMIC, NULL);
e501596ccc2e9c Qu Wenruo 2020-09-15 4980
e501596ccc2e9c Qu Wenruo 2020-09-15 4981 /* Test if we still have other extent buffer in the page range */
e501596ccc2e9c Qu Wenruo 2020-09-15 4982 ret = test_range_bit(io_tree, round_down(eb->start, PAGE_SIZE),
e501596ccc2e9c Qu Wenruo 2020-09-15 4983 round_down(eb->start, PAGE_SIZE) + PAGE_SIZE - 1,
e501596ccc2e9c Qu Wenruo 2020-09-15 4984 EXTENT_NEW, 0, NULL);
e501596ccc2e9c Qu Wenruo 2020-09-15 4985 if (!ret)
e501596ccc2e9c Qu Wenruo 2020-09-15 4986 detach_page_private(eb->pages[0]);
e501596ccc2e9c Qu Wenruo 2020-09-15 4987 if (mapped)
e501596ccc2e9c Qu Wenruo 2020-09-15 4988 spin_unlock(&page->mapping->private_lock);
e501596ccc2e9c Qu Wenruo 2020-09-15 4989 }
e501596ccc2e9c Qu Wenruo 2020-09-15 4990
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36628 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
* [PATCH v2 00/19] btrfs: add read-only support for subpage sector size @ 2020-09-15 5:35 Qu Wenruo 2020-09-15 5:35 ` [PATCH v2 16/19] btrfs: use extent_io_tree to handle subpage extent buffer allocation Qu Wenruo 0 siblings, 1 reply; 2+ messages in thread From: Qu Wenruo @ 2020-09-15 5:35 UTC (permalink / raw) To: linux-btrfs Patches can be fetched from github: https://github.com/adam900710/linux/tree/subpage Currently btrfs only allows to mount fs with sectorsize == PAGE_SIZE. That means, for 64K page size system, they can only use 64K sector size fs. This brings a big compatible problem for btrfs. This patch is going to slightly solve the problem by, allowing 64K system to mount 4K sectorsize fs in read-only mode. The main objective here, is to remove the blockage in the code base, and pave the road to full RW mount support. == What works == Existing regular page sized sector size support Subpage read-only Mount (with all self tests and ASSERT) Subpage metadata read (including all trees and inline extents, and csum checking) Subpage uncompressed data read (with csum checking) == What doesn't work == Read-write mount (see the subject) Compressed data read == Challenge we meet == The main problem is metadata, where we have several limitations: - We always read the full page of a metadata In subpage case, one full page can contain several tree blocks. - We use page::private to point to extent buffer This means we currently can only support one-page-to-one-extent-buffer mapping. For subpage size support, we need one-page-to-multiple-extent-buffer mapping. == Solutions == So here for the metadata part, we use the following methods to workaround the problem: - Completely rely on extent_io_tree for metadata status/locking Now for subpage metadata, page::private is never utilized. It always points to NULL. And we only utilize private page status, other status (locked/uptodate/dirty/...) are all ignored. Instead, page lock is replayed by EXTENT_LOCK of extent_io_tree. Page uptodate is replaced by EXTENT_UPTODATE of extent_io_tree. And if a range has extent buffer is represented by EXTENT_NEW. This provides the full potential for later RW support. - Do subpage read for metadata Now we do proper subpage read for both data and metadata. For metadata we never merge bio for adjacent tree blocks, but always submit one bio for one tree block. This allows us to do proper verification for each tree blocks. For data part, it's pretty simple, all existing infrastructure can be easily converted to support subpage read, without any subpage specific handing yet. == Patchset structure == The structure of the patchset: Patch 01~15: Preparation patches for data and metadata subpage read support. These patches can be merged without problem, and work for both regular and subpage case. This part can conflict with Nikolay's latest cleanup, but the conflicts should be pretty controllable. Patch 16~19: Patches for metadata subpage read support. The main part of the patchset. It converts metadata to purely extent_io_tree based solution for subpage read. In theory, page sized routine can also be converted to extent_io_tree. But that would be another topic in the future. The number of patches is the main reason I'm submitting them to the mail list. As there are too many preparation patches already. Qu Wenruo (19): btrfs: extent-io-tests: remove invalid tests btrfs: remove the unnecessary parameter @start and @len for check_data_csum() btrfs: calculate inline extent buffer page size based on page size btrfs: remove the open-code to read disk-key btrfs: make btrfs_fs_info::buffer_radix to take sector size devided values btrfs: don't allow tree block to cross page boundary for subpage support btrfs: update num_extent_pages() to support subpage sized extent buffer btrfs: handle sectorsize < PAGE_SIZE case for extent buffer accessors btrfs: make csum_tree_block() handle sectorsize smaller than page size btrfs: add assert_spin_locked() for attach_extent_buffer_page() btrfs: extract the extent buffer verification from btree_readpage_end_io_hook() btrfs: extent_io: only require sector size alignment for page read btrfs: make btrfs_readpage_end_io_hook() follow sector size btrfs: make btree inode io_tree has its special owner btrfs: don't set extent_io_tree bits for btree inode at endio time btrfs: use extent_io_tree to handle subpage extent buffer allocation btrfs: implement subpage metadata read and its endio function btrfs: implement btree_readpage() and try_release_extent_buffer() for subpage btrfs: allow RO mount of 4K sector size fs on 64K page system fs/btrfs/btrfs_inode.h | 12 + fs/btrfs/ctree.c | 13 +- fs/btrfs/ctree.h | 38 +++- fs/btrfs/disk-io.c | 217 ++++++++++++++---- fs/btrfs/extent-io-tree.h | 8 + fs/btrfs/extent_io.c | 376 +++++++++++++++++++++++++++---- fs/btrfs/extent_io.h | 19 +- fs/btrfs/inode.c | 40 +++- fs/btrfs/ordered-data.c | 8 + fs/btrfs/qgroup.c | 4 + fs/btrfs/struct-funcs.c | 18 +- fs/btrfs/super.c | 7 + fs/btrfs/tests/extent-io-tests.c | 26 +-- 13 files changed, 642 insertions(+), 144 deletions(-) -- 2.28.0 ^ permalink raw reply [flat|nested] 2+ messages in thread
* [PATCH v2 16/19] btrfs: use extent_io_tree to handle subpage extent buffer allocation 2020-09-15 5:35 [PATCH v2 00/19] btrfs: add read-only support for subpage sector size Qu Wenruo @ 2020-09-15 5:35 ` Qu Wenruo 0 siblings, 0 replies; 2+ messages in thread From: Qu Wenruo @ 2020-09-15 5:35 UTC (permalink / raw) To: linux-btrfs Currently btrfs uses page::private as an indicator of who owns the extent buffer, this method won't really work on subpage support, as one page can contain several tree blocks (up to 16 for 4K node size and 64K page size). Instead, here we utilize btree extent io tree to handle them. Now EXTENT_NEW means we have an extent buffer for the range. This will affects the following functions: - alloc_extent_buffer() Now for subpage we never use page->private to grab an existing eb. Instead, we rely on extra safenet in alloc_extent_buffer() to detect two callers on the same eb. - btrfs_release_extent_buffer_pages() Now for subpage, we clear the EXTENT_NEW bit first, then check if the remaining range in the page has EXTENT_NEW bit. If not, then clear the private bit for the page. - attach_extent_buffer_page() Now we set EXTENT_NEW bit for the new extent buffer to be attached, and set the page private, with NULL as page::private. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/btrfs_inode.h | 12 +++++++ fs/btrfs/disk-io.c | 7 ++++ fs/btrfs/extent_io.c | 79 ++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h index c47b6c6fea9f..cff818e0c406 100644 --- a/fs/btrfs/btrfs_inode.h +++ b/fs/btrfs/btrfs_inode.h @@ -217,6 +217,18 @@ static inline struct btrfs_inode *BTRFS_I(const struct inode *inode) return container_of(inode, struct btrfs_inode, vfs_inode); } +static inline struct btrfs_fs_info *page_to_fs_info(struct page *page) +{ + ASSERT(page->mapping); + return BTRFS_I(page->mapping->host)->root->fs_info; +} + +static inline struct extent_io_tree +*info_to_btree_io_tree(struct btrfs_fs_info *fs_info) +{ + return &BTRFS_I(fs_info->btree_inode)->io_tree; +} + static inline unsigned long btrfs_inode_hash(u64 objectid, const struct btrfs_root *root) { diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index b526adf20f3e..2ef35eb7a6e1 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -2120,6 +2120,13 @@ static void btrfs_init_btree_inode(struct btrfs_fs_info *fs_info) inode->i_mapping->a_ops = &btree_aops; RB_CLEAR_NODE(&BTRFS_I(inode)->rb_node); + /* + * This extent io tree is subpage metadata specific. + * + * It uses the following bits to represent new meaning: + * - EXTENT_NEW: Has extent buffer allocated + * - EXTENT_UPTODATE Has latest metadata read from disk + */ extent_io_tree_init(fs_info, &BTRFS_I(inode)->io_tree, IO_TREE_BTREE_IO, inode); BTRFS_I(inode)->io_tree.track_uptodate = false; diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 16fe9f4313a1..2af6786e6ab4 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -3116,6 +3116,20 @@ static void attach_extent_buffer_page(struct extent_buffer *eb, if (page->mapping) assert_spin_locked(&page->mapping->private_lock); + if (eb->fs_info->sectorsize < PAGE_SIZE && page->mapping) { + struct extent_io_tree *io_tree = + info_to_btree_io_tree(eb->fs_info); + + if (!PagePrivate(page)) + attach_page_private(page, NULL); + + /* EXTENT_NEW represents we have an extent buffer */ + set_extent_bit(io_tree, eb->start, eb->start + eb->len - 1, + EXTENT_NEW, NULL, NULL, GFP_ATOMIC); + eb->pages[0] = page; + return; + } + if (!PagePrivate(page)) attach_page_private(page, eb); else @@ -4943,6 +4957,37 @@ int extent_buffer_under_io(const struct extent_buffer *eb) test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags)); } +static void detach_extent_buffer_subpage(struct extent_buffer *eb) +{ + struct btrfs_fs_info *fs_info = eb->fs_info; + struct extent_io_tree *io_tree = info_to_btree_io_tree(fs_info); + struct page *page = eb->pages[0]; + bool mapped = !test_bit(EXTENT_BUFFER_UNMAPPED, &eb->bflags); + int ret; + + if (!page) + return; + + if (mapped) + spin_lock(&page->mapping->private_lock); + + /* + * Clear the EXTENT_NEW bit from io tree, to indicate that there is + * no longer an extent buffer in the range. + */ + __clear_extent_bit(io_tree, eb->start, eb->start + eb->len - 1, + EXTENT_NEW, 0, 0, NULL, GFP_ATOMIC, NULL); + + /* Test if we still have other extent buffer in the page range */ + ret = test_range_bit(io_tree, round_down(eb->start, PAGE_SIZE), + round_down(eb->start, PAGE_SIZE) + PAGE_SIZE - 1, + EXTENT_NEW, 0, NULL); + if (!ret) + detach_page_private(eb->pages[0]); + if (mapped) + spin_unlock(&page->mapping->private_lock); +} + /* * Release all pages attached to the extent buffer. */ @@ -4954,6 +4999,9 @@ static void btrfs_release_extent_buffer_pages(struct extent_buffer *eb) BUG_ON(extent_buffer_under_io(eb)); + if (eb->fs_info->sectorsize < PAGE_SIZE) + return detach_extent_buffer_subpage(eb); + num_pages = num_extent_pages(eb); for (i = 0; i < num_pages; i++) { struct page *page = eb->pages[i]; @@ -5248,6 +5296,7 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, struct extent_buffer *exists = NULL; struct page *p; struct address_space *mapping = fs_info->btree_inode->i_mapping; + bool subpage = (fs_info->sectorsize < PAGE_SIZE); int uptodate = 1; int ret; @@ -5280,7 +5329,7 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, } spin_lock(&mapping->private_lock); - if (PagePrivate(p)) { + if (PagePrivate(p) && !subpage) { /* * We could have already allocated an eb for this page * and attached one so lets see if we can get a ref on @@ -5321,8 +5370,21 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, * we could crash. */ } - if (uptodate) + if (uptodate && !subpage) set_bit(EXTENT_BUFFER_UPTODATE, &eb->bflags); + /* + * For subpage, we must check extent_io_tree to get if the eb is really + * UPTODATE, as the page bit is no longer reliable as we can do subpage + * read. + */ + if (subpage) { + struct extent_io_tree *io_tree = info_to_btree_io_tree(fs_info); + + ret = test_range_bit(io_tree, eb->start, eb->start + eb->len - 1, + EXTENT_UPTODATE, 1, NULL); + if (ret) + set_bit(EXTENT_BUFFER_UPTODATE, &eb->bflags); + } again: ret = radix_tree_preload(GFP_NOFS); if (ret) { @@ -5361,6 +5423,19 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, if (eb->pages[i]) unlock_page(eb->pages[i]); } + /* + * For subpage case, btrfs_release_extent_buffer() will clear the + * EXTENT_NEW bit if there is a page, and EXTENT_NEW bit represents + * we have an extent buffer in that range. + * + * Since we're here because we hit a race with another caller, who + * succeeded in inserting the eb, we shouldn't clear that EXTENT_NEW + * bit. So here we cleanup the page manually. + */ + if (subpage) { + put_page(eb->pages[0]); + eb->pages[i] = NULL; + } btrfs_release_extent_buffer(eb); return exists; -- 2.28.0 ^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-09-15 10:17 UTC | newest] Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-15 10:17 [PATCH v2 16/19] btrfs: use extent_io_tree to handle subpage extent buffer allocation kernel test robot -- strict thread matches above, loose matches on Subject: below -- 2020-09-15 5:35 [PATCH v2 00/19] btrfs: add read-only support for subpage sector size Qu Wenruo 2020-09-15 5:35 ` [PATCH v2 16/19] btrfs: use extent_io_tree to handle subpage extent buffer allocation Qu Wenruo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.