Linux-BTRFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/8][v4] Cleanup how we handle root refs, part 2
@ 2020-02-14 21:11 Josef Bacik
  2020-02-14 21:11 ` [PATCH 1/8] btrfs: make the extent buffer leak check per fs info Josef Bacik
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Josef Bacik @ 2020-02-14 21:11 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

v3->v4:
- Rebased onto the latest misc-next, there were some subtle conflicts and
  weirdness with the automatic merge, so resending.

v2->v3:
- Rebased onto the latest misc-next, so the snapshot aware defrag related
  patches got dropped.

v1->v2:
- Fixed a error missed put in an error condition in relink_extent_backref
- Added "btrfs: make the init of static elements in fs_info" so that we could
  clean up fs_info init and make the leak detectors work for the self tests.

-- original email --
In testing with the recent fsstress I stumbled upon a deadlock in how we deal
with disappearing subvolumes.  We sort of half-ass a srcu lock to protect us,
but it's used inconsistently so doesn't really provide us with actual
protection, mostly it just makes us feel good.

In order to do away with this srcu thing we need to have proper ref counting for
our roots.  We currently refcount them, but only to handle the actual kfree, it
doesn't really control the lifetime of the root.  And again, this is not done in
any sort of consistent manner so it doesn't actually protect us.

This is the first set of patches, and yes I realize there are a lot of them.
Most of them are just "hold a ref on the root" in all of the call sites that
called btrfs_read_fs_root*() variations.  Now that we're going to actually hold
references to roots we need to make sure we put the reference when we're done
with them, so these patches go through each callsite and make sure we drop the
references appropriately.

Then there's a variety of cleanups and consolidations to make things clearer and
make it so we only have 1 place to get roots.

Finally there's the root leak detection patch.  I used this with a bunch of
testing to make sure I was never leaking roots with these patches.  I've been
testing these for several weeks cleaning up all the corners, so they should be
in relatively good shape.  Most of the patches are small so straightforward to
review.

This is just part 1, this is the prep work we need to make the root lifetime a
little saner, and will allow us to drop the subvol srcu, as well as the inode
rbtree.  It doesn't really fundamentally change how roots are handled other than
making the refcounting actually protect us from freeing the root while we're
using it.  That work will come later.  Thanks,

Josef



^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 0/8][v2] Cleanup how we handle root refs, part 2
@ 2020-01-17 13:52 " Josef Bacik
  2020-01-17 13:52 ` [PATCH 6/8] btrfs: don't take an extra root ref at allocation time Josef Bacik
  0 siblings, 1 reply; 15+ messages in thread
From: Josef Bacik @ 2020-01-17 13:52 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

v1->v2:
- Rebased onto latest misc-next.
- Fixed a problem where we were dropping the ino_cache_inode in the final put of
  the root, which is braindead since it was holding a ref on that root.

--------------- Original email -------------------
This series depends on part 1 and is the final bit of cleaning up how we handle
root refs.  Historically the ref counting has been just about kfree'ing the root
itself and not actually cleaning up the root and free'ing it.  This makes it
sort of awkward to handle the lifetime of a root, as we will just free a bunch
of things related to the root, but then not free the actual root until we drop
the last reference.

This patch series brings the actual cleanup of the root inside the ref
accounting for the root.  In addition to that we also now hold refs on the root
for all of the various users of the root in order to make the lifetime more
coherent.  Previously if you looked up a root we just held it in memory until
unmount and then we had to do two put's to clear it out.  Now we hold refs to
the root when we open an inode in the root, so we could get rid of this extra
ref holding.  Now we hold refs for discrete operations, like putting it in the
radix tree, adding to the dead roots list, and of course opening an inode.

This final piece allows us to remove the subvol_srcu, which was the reason for
all of this work.  I fixed the original deadlock, but it's use was sporadic and
inconsistent, which is a recipe for trouble.

Unfortunately I was not able to kill the per-root inode rb tree yet.  We use it
in relocation to drop the extent cache for extents we are relocating, and
there's not a very clear way to accomplish that without the inode rbtree right
now.  However this work means that once I've fixed that problem we can delete
the inode rbtree completely.  Thanks,

Josef



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, back to index

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14 21:11 [PATCH 0/8][v4] Cleanup how we handle root refs, part 2 Josef Bacik
2020-02-14 21:11 ` [PATCH 1/8] btrfs: make the extent buffer leak check per fs info Josef Bacik
2020-02-19 13:59   ` Nikolay Borisov
2020-02-20 15:49     ` Josef Bacik
2020-02-14 21:11 ` [PATCH 2/8] btrfs: move ino_cache_inode dropping Josef Bacik
2020-02-14 21:11 ` [PATCH 3/8] btrfs: move the root freeing stuff into btrfs_put_root Josef Bacik
2020-02-19 15:10   ` Nikolay Borisov
2020-02-20 15:48     ` Josef Bacik
2020-02-14 21:11 ` [PATCH 4/8] btrfs: make inodes hold a ref on their roots Josef Bacik
2020-02-14 21:11 ` [PATCH 5/8] btrfs: hold a ref on the root on the dead roots list Josef Bacik
2020-02-14 21:11 ` [PATCH 6/8] btrfs: don't take an extra root ref at allocation time Josef Bacik
2020-02-14 21:11 ` [PATCH 7/8] btrfs: make btrfs_cleanup_fs_roots use the fs_roots_radix_lock Josef Bacik
2020-02-14 21:11 ` [PATCH 8/8] btrfs: kill the subvol_srcu Josef Bacik
2020-03-13 21:19 ` [PATCH 0/8][v4] Cleanup how we handle root refs, part 2 David Sterba
  -- strict thread matches above, loose matches on Subject: below --
2020-01-17 13:52 [PATCH 0/8][v2] " Josef Bacik
2020-01-17 13:52 ` [PATCH 6/8] btrfs: don't take an extra root ref at allocation time Josef Bacik

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org
	public-inbox-index linux-btrfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git