[kernel][vm] Remove hidden parents from VmObjectPaged
This removes the notion of hidden parents from VmObjectPaged and in
the process places control of the VmCowPages hierarchy manipulation in
the VmCowPages code, instead of being done from the VmObjectPaged.
Aside from the removal of the redundant VmObjectPaged hidden parents
this also performs an additional optimization of making the parent_
reference in VmObjectPaged a raw reference and not a RefPtr. This
can be done since now that the copy-on-write pages are held in
VmCowPages there is no reason to hold the VmObjectPaged around after
any other references, such as user handles and mappings, have gone
away. Not holding the parent via a refptr also ensures we can not have
any chained deletions and so the deferred deletion mechanism is only
needed now by VmCowPages.
Change-Id: I710eab49b583542f2a773f130319d5b4a8dfc7e8
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/437347
Testability-Review: Adrian Danis <adanis@google.com>
Testability-Review: Rasha Eqbal <rashaeqbal@google.com>
Commit-Queue: Adrian Danis <adanis@google.com>
Reviewed-by: Rasha Eqbal <rashaeqbal@google.com>
diff --git a/zircon/kernel/vm/include/vm/vm_cow_pages.h b/zircon/kernel/vm/include/vm/vm_cow_pages.h
index 967fc98..96766cd 100644
--- a/zircon/kernel/vm/include/vm/vm_cow_pages.h
+++ b/zircon/kernel/vm/include/vm/vm_cow_pages.h
@@ -33,19 +33,10 @@
class VmObjectPaged;
// Implements a copy-on-write hierarchy of pages in a VmPageList.
-// Currently this is treated as a private helper class of VmObjectPaged, with it being responsible
-// for correct usage. Once the hierarchy in VmObjectPaged is changed this class will become more
-// independent and responsible for its own correctness. The specific ways it currently relies on
-// VmObjectPaged are
-// 1. The backlink must be set via set_paged_backlink_locked and be non-null at all times.
-// 2. Setting up the hidden node hierarchy is performed by VmObjectPaged manually doing
-// ::CreateHidden and inserting the correct children/parents.
class VmCowPages final : public VmHierarchyBase,
public fbl::ContainableBaseClasses<
fbl::TaggedDoublyLinkedListable<VmCowPages*, internal::ChildListTag>> {
public:
- // All create functions currently are close mirrors of the VmObjectPaged create functions and
- // exist for VmObjectPaged to create appropriate nodes in the VmCowPages hierarchy.
static zx_status_t Create(fbl::RefPtr<VmHierarchyState> root_lock, uint32_t pmm_alloc_flags,
uint64_t size, fbl::RefPtr<VmCowPages>* cow_pages);
@@ -53,10 +44,10 @@
fbl::RefPtr<VmHierarchyState> root_lock, uint64_t size,
fbl::RefPtr<VmCowPages>* cow_pages);
- zx_status_t CreateHidden(fbl::RefPtr<VmCowPages>* hidden_cow);
-
- zx_status_t CreateCloneLocked(uint64_t offset, uint64_t size, fbl::RefPtr<VmCowPages>* child_cow)
- TA_REQ(lock_);
+ // Creates a copy-on-write clone with the desired parameters. This can fail due to various
+ // internal states not being correct.
+ zx_status_t CreateCloneLocked(CloneType type, uint64_t offset, uint64_t size,
+ fbl::RefPtr<VmCowPages>* child_cow) TA_REQ(lock_);
// Creates a child that looks back to this VmCowPages for all operations. Once a child slice is
// created this node should not ever be Resized.
@@ -219,14 +210,6 @@
// descendants within the range.
void RangeChangeUpdateLocked(uint64_t offset, uint64_t len, RangeChangeOp op) TA_REQ(lock_);
- // These helper functions exist for VmObjectPaged to manipulate the hierarchy. They are temporary
- // until this is cleaned up and the 1:1 equivalence of hierarchies is removed.
- void InsertHiddenParentLocked(fbl::RefPtr<VmCowPages> hidden_parent) TA_REQ(lock_);
- void RemoveChildLocked(VmCowPages* child) TA_REQ(lock_);
- void InitializeOriginalParentLocked(fbl::RefPtr<VmCowPages> parent, uint64_t offset)
- TA_REQ(lock_);
- void AddChildLocked(VmCowPages* o) TA_REQ(lock_);
-
private:
// private constructor (use Create())
VmCowPages(fbl::RefPtr<VmHierarchyState> root_lock, uint32_t options, uint32_t pmm_alloc_flags,
@@ -238,6 +221,8 @@
DISALLOW_COPY_ASSIGN_AND_MOVE(VmCowPages);
+ zx_status_t CreateHidden(fbl::RefPtr<VmCowPages>* hidden_cow);
+
bool is_hidden() const { return (options_ & kHidden); }
bool is_slice() const { return options_ & kSlice; }
@@ -362,6 +347,21 @@
// terms of functional correctness this never has to be called.
void UpdateOnAccessLocked(vm_page_t* page, uint64_t offset) TA_REQ(lock_);
+ // Inserts |hidden_parent| as a hidden parent of |this|. This vmo and |hidden_parent|
+ // must have the same lock.
+ void InsertHiddenParentLocked(fbl::RefPtr<VmCowPages> hidden_parent) TA_REQ(lock_);
+
+ // Initializes the original parent state of the vmo. |offset| is the offset of
+ // this vmo in |parent|.
+ //
+ // This function should be called at most once, even if the parent changes
+ // after initialization.
+ void InitializeOriginalParentLocked(fbl::RefPtr<VmCowPages> parent, uint64_t offset)
+ TA_REQ(lock_);
+
+ void RemoveChildLocked(VmCowPages* child) TA_REQ(lock_);
+ void AddChildLocked(VmCowPages* child) TA_REQ(lock_);
+
// Outside of initialization/destruction, hidden vmos always have two children. For
// clarity, whichever child is first in the list is the 'left' child, and whichever
// child is second is the 'right' child. Children of a paged vmo will always be paged
diff --git a/zircon/kernel/vm/include/vm/vm_object_paged.h b/zircon/kernel/vm/include/vm/vm_object_paged.h
index 69a423b..fbc1ef5 100644
--- a/zircon/kernel/vm/include/vm/vm_object_paged.h
+++ b/zircon/kernel/vm/include/vm/vm_object_paged.h
@@ -34,7 +34,6 @@
// |options_| is a bitmask of:
static constexpr uint32_t kResizable = (1u << 0);
static constexpr uint32_t kContiguous = (1u << 1);
- static constexpr uint32_t kHidden = (1u << 2);
static constexpr uint32_t kSlice = (1u << 3);
static zx_status_t Create(uint32_t pmm_alloc_flags, uint32_t options, uint64_t size,
@@ -73,18 +72,21 @@
Guard<Mutex> guard{&lock_};
return cow_pages_locked()->is_pager_backed_locked();
}
- bool is_hidden() const override { return (options_ & kHidden); }
ChildType child_type() const override {
if (is_slice()) {
return ChildType::kSlice;
}
Guard<Mutex> guard{&lock_};
- return (original_parent_user_id_ != 0) ? ChildType::kCowClone : ChildType::kNotChild;
+ return parent_ ? ChildType::kCowClone : ChildType::kNotChild;
}
bool is_slice() const { return options_ & kSlice; }
uint64_t parent_user_id() const override {
Guard<Mutex> guard{&lock_};
- return original_parent_user_id_;
+ if (parent_) {
+ AssertHeld(parent_->lock_ref());
+ return parent_->user_id_locked();
+ }
+ return 0;
}
void set_user_id(uint64_t user_id) override {
VmObject::set_user_id(user_id);
@@ -155,9 +157,6 @@
zx_status_t CreateClone(Resizability resizable, CloneType type, uint64_t offset, uint64_t size,
bool copy_name, fbl::RefPtr<VmObject>* child_vmo) override;
- // Inserts |hidden_parent| as a hidden parent of |this|. This vmo and |hidden_parent|
- // must have the same lock.
- void InsertHiddenParentLocked(fbl::RefPtr<VmObjectPaged>&& hidden_parent) TA_REQ(lock_);
uint32_t GetMappingCachePolicy() const override {
Guard<Mutex> guard{&lock_};
@@ -166,9 +165,6 @@
uint32_t GetMappingCachePolicyLocked() const TA_REQ(lock_) { return cache_policy_; }
zx_status_t SetMappingCachePolicy(const uint32_t cache_policy) override;
- void RemoveChild(VmObject* child, Guard<Mutex>&& guard) override TA_REQ(lock_);
- bool OnChildAddedLocked() override TA_REQ(lock_);
-
void DetachSource() override {
Guard<Mutex> guard{&lock_};
@@ -229,14 +225,6 @@
// private constructor (use Create())
VmObjectPaged(uint32_t options, fbl::RefPtr<VmHierarchyState> root_state);
- // Initializes the original parent state of the vmo. |offset| is the offset of
- // this vmo in |parent|.
- //
- // This function should be called at most once, even if the parent changes
- // after initialization.
- void InitializeOriginalParentLocked(fbl::RefPtr<VmObjectPaged> parent, uint64_t offset)
- TA_REQ(lock_);
-
static zx_status_t CreateCommon(uint32_t pmm_alloc_flags, uint32_t options, uint64_t size,
fbl::RefPtr<VmObjectPaged>* vmo);
@@ -282,11 +270,9 @@
const uint32_t options_;
uint32_t cache_policy_ TA_GUARDED(lock_) = ARCH_MMU_FLAG_CACHED;
- // parent pointer (may be null)
- fbl::RefPtr<VmObjectPaged> parent_ TA_GUARDED(lock_);
- // Record the user_id_ of the original parent, in case we make
- // a bidirectional clone and end up changing parent_.
- uint64_t original_parent_user_id_ TA_GUARDED(lock_) = 0;
+ // parent pointer (may be null). This is a raw pointer as we have no need to hold our parent alive
+ // once they want to go away.
+ VmObjectPaged* parent_ TA_GUARDED(lock_) = nullptr;
// Tracks the last cached page attribution count.
mutable CachedPageAttribution cached_page_attribution_ TA_GUARDED(lock_) = {};
diff --git a/zircon/kernel/vm/vm_cow_pages.cc b/zircon/kernel/vm/vm_cow_pages.cc
index 3db256b..90b0f19 100644
--- a/zircon/kernel/vm/vm_cow_pages.cc
+++ b/zircon/kernel/vm/vm_cow_pages.cc
@@ -179,17 +179,46 @@
VmCowPages::~VmCowPages() {
canary_.Assert();
- // The hierarchy of VmCowPages, although it mirrors VmObjectPaged right now, is maintained
- // slightly differently. The VmObjectPaged destructor will explicitly clean up any hidden
- // hierarchies before it drops the reference to us. This means we do not need to do any thing
- // with removing ourselves from our parent etc, and can literally just free the page lists.
- // All of this will change once VmObjectPaged has its hierarchy simplified, and the logic that
- // is currently there will be moved into here.
if (!is_hidden()) {
- DEBUG_ASSERT(!parent_);
+ // If we're not a hidden vmo, then we need to remove ourself from our parent. This needs
+ // to be done before emptying the page list so that a hidden parent can't merge into this
+ // vmo and repopulate the page list.
+ //
+ // To prevent races with a hidden parent merging itself into this vmo, it is necessary
+ // to hold the lock over the parent_ check and into the subsequent removal call.
+ Guard<Mutex> guard{&lock_};
+ if (parent_) {
+ parent_->RemoveChildLocked(this);
+ guard.Release();
+ // Avoid recursing destructors when we delete our parent by using the deferred deletion
+ // method. See common in parent else branch for why we can avoid this on a hidden parent.
+ if (!parent_->is_hidden()) {
+ hierarchy_state_ptr_->DoDeferredDelete(ktl::move(parent_));
+ }
+ }
} else {
+ // Most of the hidden vmo's state should have already been cleaned up when it merged
+ // itself into its child in ::RemoveChildLocked.
DEBUG_ASSERT(children_list_len_ == 0);
DEBUG_ASSERT(page_list_.HasNoPages());
+ // Even though we are hidden we might have a parent. Unlike in the other branch of this if we
+ // do not need to perform any deferred deletion. The reason for this is that the deferred
+ // deletion mechanism is intended to resolve the scenario where there is a chain of 'one ref'
+ // parent pointers that will chain delete. However, with hidden parents we *know* that a hidden
+ // parent has two children (and hence at least one other ref to it) and so we cannot be in a
+ // one ref chain. Even if N threads all tried to remove children from the hierarchy at once,
+ // this would ultimately get serialized through the lock and the hierarchy would go from
+ //
+ // [..]
+ // /
+ // A [..]
+ // / \ /
+ // B E TO B A
+ // / \ / / \.
+ // C D C D E
+ //
+ // And so each serialized deletion breaks of a discrete two VMO chain that can be safely
+ // finalized with one recursive step.
}
// Cleanup page lists and page sources.
@@ -330,16 +359,16 @@
children_list_.replace(*old, new_child);
}
-void VmCowPages::DropChildLocked(VmCowPages* c) {
+void VmCowPages::DropChildLocked(VmCowPages* child) {
canary_.Assert();
DEBUG_ASSERT(children_list_len_ > 0);
- children_list_.erase(*c);
+ children_list_.erase(*child);
--children_list_len_;
}
-void VmCowPages::AddChildLocked(VmCowPages* o) {
+void VmCowPages::AddChildLocked(VmCowPages* child) {
canary_.Assert();
- children_list_.push_front(o);
+ children_list_.push_front(child);
children_list_len_++;
}
@@ -429,22 +458,46 @@
slice->root_parent_offset_ = CheckedAdd(offset, slice->root_parent_offset_);
CheckedAdd(slice->root_parent_offset_, size);
- // Currently rely on VmObjectPaged code to setup the hierarchy and call
- // InitializeOriginalParentLocked etc. This will be changed once the VmObjectPaged hierarchy is
- // simplified.
+ AddChildLocked(slice.get());
+ slice->InitializeOriginalParentLocked(fbl::RefPtr(this), offset);
*cow_slice = slice;
return ZX_OK;
}
-zx_status_t VmCowPages::CreateCloneLocked(uint64_t offset, uint64_t size,
+zx_status_t VmCowPages::CreateCloneLocked(CloneType type, uint64_t offset, uint64_t size,
fbl::RefPtr<VmCowPages>* cow_child) {
LTRACEF("vmo %p offset %#" PRIx64 " size %#" PRIx64 "\n", this, offset, size);
canary_.Assert();
+ DEBUG_ASSERT(IS_PAGE_ALIGNED(offset));
+ DEBUG_ASSERT(IS_PAGE_ALIGNED(size));
+
// All validation *must* be performed here prior to construction the VmCowPages, as the
// destructor for VmCowPages may acquire the lock, which we are already holding.
+
+ switch (type) {
+ case CloneType::Snapshot: {
+ if (!IsCowClonableLocked()) {
+ return ZX_ERR_NOT_SUPPORTED;
+ }
+
+ // If this is non-zero, that means that there are pages which hardware can
+ // touch, so the vmo can't be safely cloned.
+ // TODO: consider immediately forking these pages.
+ if (pinned_page_count_locked()) {
+ return ZX_ERR_BAD_STATE;
+ }
+ break;
+ }
+ case CloneType::PrivatePagerCopy:
+ if (!is_pager_backed_locked()) {
+ return ZX_ERR_NOT_SUPPORTED;
+ }
+ break;
+ }
+
uint64_t new_root_parent_offset;
bool overflow;
overflow = add_overflow(offset, root_parent_offset_, &new_root_parent_offset);
@@ -457,6 +510,18 @@
return ZX_ERR_INVALID_ARGS;
}
+ // Create the hidden cow pages first. If we later fail and need to destroy them it is fine, as
+ // hidden destruction does not require taking the lock which we are currently holding.
+ fbl::RefPtr<VmCowPages> hidden_parent;
+ if (type == CloneType::Snapshot) {
+ // The initial size is 0. It will be initialized as part of the atomic
+ // insertion into the child tree.
+ zx_status_t status = CreateHidden(&hidden_parent);
+ if (status != ZX_OK) {
+ return status;
+ }
+ }
+
fbl::AllocChecker ac;
auto cow_pages = fbl::AdoptRef<VmCowPages>(
new (&ac) VmCowPages(hierarchy_state_ptr_, 0, pmm_alloc_flags_, size, nullptr));
@@ -477,6 +542,23 @@
cow_pages->parent_limit_ = ktl::min(size, size_ - offset);
}
+ VmCowPages* clone_parent;
+ if (type == CloneType::Snapshot) {
+ clone_parent = hidden_parent.get();
+
+ InsertHiddenParentLocked(ktl::move(hidden_parent));
+
+ // Invalidate everything the clone will be able to see. They're COW pages now,
+ // so any existing mappings can no longer directly write to the pages.
+ RangeChangeUpdateLocked(offset, size, RangeChangeOp::RemoveWrite);
+ } else {
+ clone_parent = this;
+ }
+
+ cow_pages->InitializeOriginalParentLocked(fbl::RefPtr(clone_parent), offset);
+ AssertHeld(clone_parent->lock_ref());
+ clone_parent->AddChildLocked(cow_pages.get());
+
*cow_child = ktl::move(cow_pages);
return ZX_OK;
}
@@ -496,7 +578,6 @@
canary_.Assert();
AssertHeld(removed->lock_);
- removed->parent_.reset();
if (!is_hidden()) {
DropChildLocked(removed);
@@ -2124,7 +2205,7 @@
// to use the split bits to release pages in the parent. It also means that ancestor pages in
// the specified range might end up being released based on their current split bits, instead of
// through subsequent calls to this function. Therefore parent and all ancestors need to have
- // the partial_cow_release_ flag set to prevent fast merge issues in ::RemoveChild.
+ // the partial_cow_release_ flag set to prevent fast merge issues in ::RemoveChildLocked.
auto cur = this;
AssertHeld(cur->lock_);
uint64_t cur_start = start;
@@ -2167,7 +2248,7 @@
}
if (skip_split_bits) {
// If we were able to update this vmo's parent limit, that made the pages
- // uniaccessible. We clear the split bits to allow ::RemoveChild to efficiently
+ // uniaccessible. We clear the split bits to allow ::RemoveChildLocked to efficiently
// merge vmos without having to worry about pages above parent_limit_.
page->object.cow_left_split = 0;
page->object.cow_right_split = 0;
diff --git a/zircon/kernel/vm/vm_object_paged.cc b/zircon/kernel/vm/vm_object_paged.cc
index fdf8790..d25f70d 100644
--- a/zircon/kernel/vm/vm_object_paged.cc
+++ b/zircon/kernel/vm/vm_object_paged.cc
@@ -48,16 +48,6 @@
LTRACEF("%p\n", this);
}
-void VmObjectPaged::InitializeOriginalParentLocked(fbl::RefPtr<VmObjectPaged> parent,
- uint64_t offset) {
- DEBUG_ASSERT(parent_ == nullptr);
- DEBUG_ASSERT(original_parent_user_id_ == 0);
-
- AssertHeld(parent->lock_);
- original_parent_user_id_ = parent->user_id_locked();
- parent_ = ktl::move(parent);
-}
-
VmObjectPaged::~VmObjectPaged() {
canary_.Assert();
@@ -82,46 +72,38 @@
}
}
+ AssertHeld(hierarchy_state_ptr_->lock_ref());
+ hierarchy_state_ptr_->IncrementHierarchyGenerationCountLocked();
+
cow_pages_locked()->set_paged_backlink_locked(nullptr);
- if (!is_hidden()) {
- // If we're not a hidden vmo, then we need to remove ourself from our parent. This needs
- // to be done before emptying the page list so that a hidden parent can't merge into this
- // vmo and repopulate the page list.
- //
- // To prevent races with a hidden parent merging itself into this vmo, it is necessary
- // to hold the lock over the parent_ check and into the subsequent removal call.
+ // Re-home all our children with any parent that we have.
+ while (!children_list_.is_empty()) {
+ VmObject* c = &children_list_.front();
+ children_list_.pop_front();
+ VmObjectPaged* child = reinterpret_cast<VmObjectPaged*>(c);
+ child->parent_ = parent_;
if (parent_) {
- LTRACEF("removing ourself from our parent %p\n", parent_.get());
- parent_->RemoveChild(this, guard.take());
- // Avoid recursing destructors when we delete our parent by using the deferred deletion
- // method. See common in parent else branch for why we can avoid this on a hidden parent.
- if (!parent_->is_hidden()) {
- hierarchy_state_ptr_->DoDeferredDelete(ktl::move(parent_));
- }
+ // Ignore the return since 'this' is a child so we know we are not transitioning from 0->1
+ // children.
+ bool __UNUSED notify = parent_->AddChildLocked(child);
+ DEBUG_ASSERT(!notify);
}
- } else {
- // Most of the hidden vmo's state should have already been cleaned up when it merged
- // itself into its child in ::RemoveChild.
- DEBUG_ASSERT(children_list_len_ == 0);
- // Even though we are hidden we might have a parent. Unlike in the other branch of this if we
- // do not need to perform any deferred deletion. The reason for this is that the deferred
- // deletion mechanism is intended to resolve the scenario where there is a chain of 'one ref'
- // parent pointers that will chain delete. However, with hidden parents we *know* that a hidden
- // parent has two children (and hence at least one other ref to it) and so we cannot be in a
- // one ref chain. Even if N threads all tried to remove children from the hierarchy at once,
- // this would ultimately get serialized through the lock and the hierarchy would go from
- //
- // [..]
- // /
- // A [..]
- // / \ /
- // B E TO B A
- // / \ / / \.
- // C D C D E
- //
- // And so each serialized deletion breaks of a discrete two VMO chain that can be safely
- // finalized with one recursive step.
+ }
+
+ if (parent_) {
+ // As parent_ is a raw pointer we must ensure that if we call a method on it that it lives long
+ // enough. To do so we attempt to upgrade it to a refptr, which could fail if it's already
+ // slated for deletion.
+ fbl::RefPtr<VmObjectPaged> parent = fbl::MakeRefPtrUpgradeFromRaw(parent_, guard);
+ if (parent) {
+ // Holding refptr, can safely pass in the guard to RemoveChild.
+ parent->RemoveChild(this, guard.take());
+ } else {
+ // parent is up for deletion and so there's no need to use RemoveChild since there is no
+ // user dispatcher to notify anyway and so just drop ourselves to keep the hierarchy correct.
+ parent_->DropChildLocked(this);
+ }
}
}
@@ -415,24 +397,6 @@
return ZX_OK;
}
-void VmObjectPaged::InsertHiddenParentLocked(fbl::RefPtr<VmObjectPaged>&& hidden_parent) {
- AssertHeld(hidden_parent->lock_);
- // Insert the new VmObject |hidden_parent| between between |this| and |parent_|.
- cow_pages_locked()->InsertHiddenParentLocked(hidden_parent->cow_pages_);
- if (parent_) {
- AssertHeld(parent_->lock_ref());
- hidden_parent->InitializeOriginalParentLocked(parent_, 0);
- parent_->ReplaceChildLocked(this, hidden_parent.get());
- }
- hidden_parent->AddChildLocked(this);
- parent_ = hidden_parent;
-
- // We use the user_id to walk the tree looking for the right child observer. This
- // is set after adding the hidden parent into the tree since that's not really
- // a 'real' child.
- hidden_parent->user_id_ = user_id_;
-}
-
zx_status_t VmObjectPaged::CreateChildSlice(uint64_t offset, uint64_t size, bool copy_name,
fbl::RefPtr<VmObject>* child_vmo) {
LTRACEF("vmo %p offset %#" PRIx64 " size %#" PRIx64 "\n", this, offset, size);
@@ -493,20 +457,14 @@
if (status != ZX_OK) {
return status;
}
- // Whilst we have the lock and we know failure cannot happen, link up the cow pages. Will place
- // in global list at the end.
+ // Now that everything has succeeded, link up the cow pages and our parents/children.
+ // Both child notification and inserting into the globals list has to happen outside the lock.
AssertHeld(cow_pages->lock_ref());
cow_pages->set_paged_backlink_locked(vmo.get());
vmo->cow_pages_ = ktl::move(cow_pages);
- // Initialize the parents for both parallel hierarchies.
- vmo->InitializeOriginalParentLocked(fbl::RefPtr(this), offset);
- vmo->cow_pages_locked()->InitializeOriginalParentLocked(cow_pages_, offset);
-
- // add the new vmo as a child before we do anything, since its
- // dtor expects to find it in its parent's child list
+ vmo->parent_ = this;
notify_one_child = AddChildLocked(vmo.get());
- cow_pages_locked()->AddChildLocked(vmo->cow_pages_.get());
if (copy_name) {
vmo->name_ = name_;
@@ -556,36 +514,6 @@
return ZX_ERR_NO_MEMORY;
}
- // Hidden parent needs to be declared before the guard as after it is initialized and added to
- // the global list we can still fail and need to destruct it, this destruction must happen without
- // the lock being held.
- fbl::RefPtr<VmObjectPaged> hidden_parent;
- // Optimistically create the hidden parent early as we want to do it outside the lock, but we
- // need to hold the lock to validate invariants.
- if (type == CloneType::Snapshot) {
- // The initial size is 0. It will be initialized as part of the atomic
- // insertion into the child tree.
- hidden_parent =
- fbl::AdoptRef<VmObjectPaged>(new (&ac) VmObjectPaged(kHidden, hierarchy_state_ptr_));
- if (!ac.check()) {
- return ZX_ERR_NO_MEMORY;
- }
- // Can immediately link up some cow pages and add to the global list.
- {
- fbl::RefPtr<VmCowPages> hidden_cow_pages;
- Guard<Mutex> guard{&lock_};
- AssertHeld(hidden_parent->lock_ref());
- status = cow_pages_locked()->CreateHidden(&hidden_cow_pages);
- if (status != ZX_OK) {
- return status;
- }
- AssertHeld(hidden_cow_pages->lock_ref());
- hidden_cow_pages->set_paged_backlink_locked(hidden_parent.get());
- hidden_parent->cow_pages_ = ktl::move(hidden_cow_pages);
- }
- hidden_parent->AddToGlobalList();
- }
-
bool notify_one_child;
{
// Declare these prior to the guard so that any failure paths destroy these without holding
@@ -593,40 +521,12 @@
fbl::RefPtr<VmCowPages> clone_cow_pages;
Guard<Mutex> guard{&lock_};
AssertHeld(vmo->lock_);
- switch (type) {
- case CloneType::Snapshot: {
- // To create an eager copy-on-write clone, the kernel creates an artifical parent vmo
- // called a 'hidden vmo'. The content of the original vmo is moved into the hidden
- // vmo, and the original vmo becomes a child of the hidden vmo. Then a second child
- // is created, which is the userspace visible clone.
- //
- // Hidden vmos are an implementation detail that are not exposed to userspace.
-
- if (!cow_pages_locked()->IsCowClonableLocked()) {
- return ZX_ERR_NOT_SUPPORTED;
- }
-
- // If this is non-zero, that means that there are pages which hardware can
- // touch, so the vmo can't be safely cloned.
- // TODO: consider immediately forking these pages.
- if (cow_pages_locked()->pinned_page_count_locked()) {
- return ZX_ERR_BAD_STATE;
- }
- break;
- }
- case CloneType::PrivatePagerCopy:
- if (!cow_pages_locked()->is_pager_backed_locked()) {
- return ZX_ERR_NOT_SUPPORTED;
- }
- break;
- }
-
// check that we're not uncached in some way
if (cache_policy_ != ARCH_MMU_FLAG_CACHED) {
return ZX_ERR_BAD_STATE;
}
- status = cow_pages_locked()->CreateCloneLocked(offset, size, &clone_cow_pages);
+ status = cow_pages_locked()->CreateCloneLocked(type, offset, size, &clone_cow_pages);
if (status != ZX_OK) {
return status;
}
@@ -637,30 +537,12 @@
clone_cow_pages->set_paged_backlink_locked(vmo.get());
vmo->cow_pages_ = ktl::move(clone_cow_pages);
- VmObjectPaged* clone_parent;
- if (type == CloneType::Snapshot) {
- clone_parent = hidden_parent.get();
-
- InsertHiddenParentLocked(ktl::move(hidden_parent));
-
- // Invalidate everything the clone will be able to see. They're COW pages now,
- // so any existing mappings can no longer directly write to the pages.
- // This should be being done by VmCowPages, but as we are temporarily responsible for
- // construction of the hierarchy it's easier for us to do it for the moment.
- cow_pages_locked()->RangeChangeUpdateLocked(offset, size, RangeChangeOp::RemoveWrite);
- } else {
- clone_parent = this;
- }
- AssertHeld(clone_parent->lock_);
-
- // Initialize the parents for both parallel hierarchies.
- vmo->InitializeOriginalParentLocked(fbl::RefPtr(clone_parent), offset);
- vmo->cow_pages_locked()->InitializeOriginalParentLocked(clone_parent->cow_pages_, offset);
+ // Install the parent.
+ vmo->parent_ = this;
// add the new vmo as a child before we do anything, since its
// dtor expects to find it in its parent's child list
- notify_one_child = clone_parent->AddChildLocked(vmo.get());
- clone_parent->cow_pages_locked()->AddChildLocked(vmo->cow_pages_.get());
+ notify_one_child = AddChildLocked(vmo.get());
if (copy_name) {
vmo->name_ = name_;
@@ -680,134 +562,20 @@
return ZX_OK;
}
-bool VmObjectPaged::OnChildAddedLocked() {
- if (!is_hidden()) {
- return VmObject::OnChildAddedLocked();
- }
-
- if (user_id_ == ZX_KOID_INVALID) {
- // The original vmo is added as a child of the hidden vmo before setting
- // the user id to prevent counting as its own child.
- return false;
- }
-
- // After initialization, hidden vmos always have two children - the vmo on which
- // zx_vmo_create_child was invoked and the vmo which that syscall created.
- DEBUG_ASSERT(children_list_len_ == 2);
-
- // Reaching into the children confuses analysis
- for (auto& c : children_list_) {
- DEBUG_ASSERT(c.is_paged());
- VmObjectPaged& child = static_cast<VmObjectPaged&>(c);
- AssertHeld(child.lock_);
- if (child.user_id_ == user_id_) {
- return child.OnChildAddedLocked();
- }
- }
-
- // One of the children should always have a matching user_id.
- panic("no child with matching user_id: %" PRIx64 "\n", user_id_);
-}
-
-void VmObjectPaged::RemoveChild(VmObject* removed, Guard<Mutex>&& adopt) {
- DEBUG_ASSERT(adopt.wraps_lock(lock_ref().lock()));
-
- // This is scoped before guard to ensure the guard is dropped first, see comment where child_ref
- // is assigned for more details.
- fbl::RefPtr<VmObject> child_ref;
-
- Guard<Mutex> guard{AdoptLock, ktl::move(adopt)};
-
- IncrementHierarchyGenerationCountLocked();
-
- // Remove the child in our parallel hierarchy, resulting in any necessary merging with the
- // hidden parent to happen.
- VmObjectPaged* paged_removed = static_cast<VmObjectPaged*>(removed);
- AssertHeld(paged_removed->lock_ref());
- cow_pages_locked()->RemoveChildLocked(paged_removed->cow_pages_.get());
-
- if (!is_hidden()) {
- VmObject::RemoveChild(removed, guard.take());
- return;
- }
-
- // Hidden vmos always have 0 or 2 children, but we can't be here with 0 children.
- DEBUG_ASSERT(children_list_len_ == 2);
- // A hidden vmo must be fully initialized to have 2 children.
- DEBUG_ASSERT(user_id_ != ZX_KOID_INVALID);
-
- DropChildLocked(removed);
-
- VmObject* child = &children_list_.front();
- DEBUG_ASSERT(child);
-
- // Attempt to upgrade our raw pointer to a ref ptr. This upgrade can fail in the scenario that
- // the childs refcount has dropped to zero and is also attempting to delete itself. If this
- // happens, as we hold the vmo lock we know our child cannot complete its destructor, and so we
- // can still modify pieces of it until we drop the lock. It is now possible that after we upgrade
- // we become the sole holder of a refptr, and the refptr *must* be destroyed after we release the
- // VMO lock to prevent a deadlock.
- child_ref = fbl::MakeRefPtrUpgradeFromRaw(child, guard);
-
- // Our children must be paged.
- DEBUG_ASSERT(child->is_paged());
- VmObjectPaged* typed_child = static_cast<VmObjectPaged*>(child);
- AssertHeld(typed_child->lock_);
-
- // The child which removed itself and led to the invocation should have a reference
- // to us, in addition to child.parent_ which we are about to clear.
- DEBUG_ASSERT(ref_count_debug() >= 2);
-
- // Drop the child from our list, but don't recurse back into this function. Then
- // remove ourselves from the clone tree.
- DropChildLocked(typed_child);
- if (parent_) {
- AssertHeld(parent_->lock_ref());
- parent_->ReplaceChildLocked(this, typed_child);
- }
- typed_child->parent_ = ktl::move(parent_);
-
- // To use child here we need to ensure that it will live long enough. Up until here even if child
- // was waiting to be destroyed, we knew it would stay alive as long as we held the lock. Since we
- // give away the guard in the call to OnUserChildRemoved, we can only perform the call if we can
- // separately guarantee the child stays alive by having a refptr to it.
- // In the scenario where the refptr does not exist, that means the upgrade failed and there is no
- // user object to signal anyway.
- if (child_ref) {
- // We need to proxy the closure down to the original user-visible vmo. To find
- // that, we can walk down the clone tree following the user_id_.
- VmObjectPaged* descendant = typed_child;
- AssertHeld(descendant->lock_);
- while (descendant && descendant->user_id_ == user_id_) {
- if (!descendant->is_hidden()) {
- descendant->OnUserChildRemoved(guard.take());
- return;
- }
- VmObjectPaged* left = static_cast<VmObjectPaged*>(&descendant->children_list_.front());
- VmObjectPaged* right = static_cast<VmObjectPaged*>(&descendant->children_list_.back());
- AssertHeld(left->lock_ref());
- AssertHeld(right->lock_ref());
- if (left->user_id_locked() == user_id_) {
- descendant = left;
- } else if (right->user_id_locked() == user_id_) {
- descendant = right;
- } else {
- descendant = nullptr;
- }
- }
- }
-}
-
void VmObjectPaged::DumpLocked(uint depth, bool verbose) const {
canary_.Assert();
- uint64_t parent_id = original_parent_user_id_;
+ uint64_t parent_id = 0;
+ if (parent_) {
+ AssertHeld(parent_->lock_ref());
+ parent_id = parent_->user_id_locked();
+ }
for (uint i = 0; i < depth; ++i) {
printf(" ");
}
printf("vmo %p/k%" PRIu64 " ref %d parent %p/k%" PRIu64 "\n", this, user_id_, ref_count_debug(),
- parent_.get(), parent_id);
+ parent_, parent_id);
char name[ZX_MAX_NAME_LEN];
get_name(name, sizeof(name));
@@ -822,10 +590,6 @@
}
size_t VmObjectPaged::AttributedPagesInRangeLocked(uint64_t offset, uint64_t len) const {
- if (is_hidden()) {
- return 0;
- }
-
uint64_t new_len;
if (!TrimRange(offset, len, size_locked(), &new_len)) {
return 0;
diff --git a/zircon/kernel/vm/vm_unittest.cc b/zircon/kernel/vm/vm_unittest.cc
index 902b0c0..0618844 100644
--- a/zircon/kernel/vm/vm_unittest.cc
+++ b/zircon/kernel/vm/vm_unittest.cc
@@ -2541,6 +2541,70 @@
END_TEST;
}
+// Test that a VmObjectPaged that is only referenced by its children gets removed by effectively
+// merging into its parent and re-homing all the children. This should also drop any VmCowPages
+// being held open.
+static bool vmo_parent_merge_test() {
+ BEGIN_TEST;
+
+ fbl::RefPtr<VmObjectPaged> vmo;
+ zx_status_t status = VmObjectPaged::Create(PMM_ALLOC_FLAG_ANY, 0, PAGE_SIZE, &vmo);
+ ASSERT_EQ(ZX_OK, status);
+
+ // Set a user ID for testing.
+ vmo->set_user_id(42);
+
+ fbl::RefPtr<VmObject> child;
+ status = vmo->CreateClone(Resizability::NonResizable, CloneType::Snapshot, 0, PAGE_SIZE, false,
+ &child);
+ ASSERT_EQ(ZX_OK, status);
+
+ child->set_user_id(43);
+
+ EXPECT_EQ(0u, vmo->parent_user_id());
+ EXPECT_EQ(42u, vmo->user_id());
+ EXPECT_EQ(43u, child->user_id());
+ EXPECT_EQ(42u, child->parent_user_id());
+
+ // Dropping the parent should re-home the child to an empty parent.
+ vmo.reset();
+ EXPECT_EQ(43u, child->user_id());
+ EXPECT_EQ(0u, child->parent_user_id());
+
+ child.reset();
+
+ // Recreate a more interesting 3 level hierarchy with vmo->child->(child2,child3)
+
+ status = VmObjectPaged::Create(PMM_ALLOC_FLAG_ANY, 0, PAGE_SIZE, &vmo);
+ ASSERT_EQ(ZX_OK, status);
+ vmo->set_user_id(42);
+ status = vmo->CreateClone(Resizability::NonResizable, CloneType::Snapshot, 0, PAGE_SIZE, false,
+ &child);
+ ASSERT_EQ(ZX_OK, status);
+ child->set_user_id(43);
+ fbl::RefPtr<VmObject> child2;
+ status = child->CreateClone(Resizability::NonResizable, CloneType::Snapshot, 0, PAGE_SIZE, false,
+ &child2);
+ ASSERT_EQ(ZX_OK, status);
+ child2->set_user_id(44);
+ fbl::RefPtr<VmObject> child3;
+ status = child->CreateClone(Resizability::NonResizable, CloneType::Snapshot, 0, PAGE_SIZE, false,
+ &child3);
+ ASSERT_EQ(ZX_OK, status);
+ child3->set_user_id(45);
+ EXPECT_EQ(0u, vmo->parent_user_id());
+ EXPECT_EQ(42u, child->parent_user_id());
+ EXPECT_EQ(43u, child2->parent_user_id());
+ EXPECT_EQ(43u, child3->parent_user_id());
+
+ // Drop the intermediate child, child2+3 should get re-homed to vmo
+ child.reset();
+ EXPECT_EQ(42u, child2->parent_user_id());
+ EXPECT_EQ(42u, child3->parent_user_id());
+
+ END_TEST;
+}
+
// TODO(fxbug.dev/31326): The ARM code's error codes are always ZX_ERR_INTERNAL, so
// special case that.
#if ARCH_ARM64
@@ -3832,6 +3896,7 @@
VM_UNITTEST(vmo_attribution_pager_test)
VM_UNITTEST(vmo_attribution_evict_test)
VM_UNITTEST(vmo_attribution_dedup_test)
+VM_UNITTEST(vmo_parent_merge_test)
VM_UNITTEST(arch_noncontiguous_map)
VM_UNITTEST(vm_kernel_region_test)
VM_UNITTEST(region_list_get_alloc_spot_test)
