[sysmem] Store more inspect information about every heap
Add inspect information about every heap, along with an ID. This makes
it possible to specify what heap a VMO is allocated from. Also add more
properties giving the VMO size and a koid for the channel that's keeping
it alive.
Change-Id: I40cf50ee739a68a2b02662a44c90c95544800840
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/475898
Reviewed-by: Dustin Green <dustingreen@google.com>
Commit-Queue: John Bauman <jbauman@google.com>
diff --git a/src/devices/sysmem/drivers/sysmem/buffer_collection.cc b/src/devices/sysmem/drivers/sysmem/buffer_collection.cc
index 39e3f49..105c1b6 100644
--- a/src/devices/sysmem/drivers/sysmem/buffer_collection.cc
+++ b/src/devices/sysmem/drivers/sysmem/buffer_collection.cc
@@ -80,6 +80,16 @@
(void)binding_.Close();
}
+void BufferCollection::Bind(zx::channel server_request) {
+ zx_info_handle_basic_t info;
+ zx_status_t status =
+ server_request.get_info(ZX_INFO_HANDLE_BASIC, &info, sizeof(info), nullptr, nullptr);
+ if (status == ZX_OK) {
+ node_.CreateUint("channel_koid", info.koid, &properties_);
+ }
+ FidlServer::Bind(std::move(server_request));
+}
+
zx_status_t BufferCollection::SetEventSink(zx_handle_t buffer_collection_events_client_param) {
zx::channel buffer_collection_events_client(buffer_collection_events_client_param);
if (is_done_) {
diff --git a/src/devices/sysmem/drivers/sysmem/buffer_collection.h b/src/devices/sysmem/drivers/sysmem/buffer_collection.h
index a95bdbf..a46054d 100644
--- a/src/devices/sysmem/drivers/sysmem/buffer_collection.h
+++ b/src/devices/sysmem/drivers/sysmem/buffer_collection.h
@@ -29,6 +29,8 @@
public:
~BufferCollection();
+ void Bind(zx::channel server_request);
+
//
// fuchsia.sysmem.BufferCollection interface methods
//
@@ -165,6 +167,7 @@
inspect::Node node_;
inspect::UintProperty debug_id_property_;
inspect::StringProperty debug_name_property_;
+ inspect::ValueList properties_;
};
} // namespace sysmem_driver
diff --git a/src/devices/sysmem/drivers/sysmem/buffer_collection_token.cc b/src/devices/sysmem/drivers/sysmem/buffer_collection_token.cc
index fd3b2b6..2cf531c 100644
--- a/src/devices/sysmem/drivers/sysmem/buffer_collection_token.cc
+++ b/src/devices/sysmem/drivers/sysmem/buffer_collection_token.cc
@@ -39,6 +39,12 @@
}
void BufferCollectionToken::Bind(zx::channel channel) {
+ zx_info_handle_basic_t info;
+ zx_status_t status =
+ channel.get_info(ZX_INFO_HANDLE_BASIC, &info, sizeof(info), nullptr, nullptr);
+ if (status == ZX_OK) {
+ node_.CreateUint("channel_koid", info.koid, &properties_);
+ }
auto res = fidl::BindServer(
parent_device_->dispatcher(), std::move(channel), this,
fidl::OnUnboundFn<BufferCollectionToken>(
diff --git a/src/devices/sysmem/drivers/sysmem/buffer_collection_token.h b/src/devices/sysmem/drivers/sysmem/buffer_collection_token.h
index 6a55cac..f4a5d90 100644
--- a/src/devices/sysmem/drivers/sysmem/buffer_collection_token.h
+++ b/src/devices/sysmem/drivers/sysmem/buffer_collection_token.h
@@ -107,6 +107,7 @@
inspect::Node node_;
inspect::UintProperty debug_id_property_;
inspect::StringProperty debug_name_property_;
+ inspect::ValueList properties_;
};
} // namespace sysmem_driver
diff --git a/src/devices/sysmem/drivers/sysmem/contiguous_pooled_memory_allocator.cc b/src/devices/sysmem/drivers/sysmem/contiguous_pooled_memory_allocator.cc
index 28e0554..a94940c 100644
--- a/src/devices/sysmem/drivers/sysmem/contiguous_pooled_memory_allocator.cc
+++ b/src/devices/sysmem/drivers/sysmem/contiguous_pooled_memory_allocator.cc
@@ -54,7 +54,8 @@
// Ensure NUL-terminated.
child_name_[sizeof(child_name_) - 1] = 0;
node_ = parent_node->CreateChild(allocation_name);
- size_property_ = node_.CreateUint("size", size);
+ node_.CreateUint("size", size, &properties_);
+ node_.CreateUint("id", id(), &properties_);
high_water_mark_property_ = node_.CreateUint("high_water_mark", 0);
free_at_high_water_mark_property_ = node_.CreateUint("free_at_high_water_mark", size);
used_size_property_ = node_.CreateUint("used_size", 0);
diff --git a/src/devices/sysmem/drivers/sysmem/contiguous_pooled_memory_allocator.h b/src/devices/sysmem/drivers/sysmem/contiguous_pooled_memory_allocator.h
index 258e4e87..265d484 100644
--- a/src/devices/sysmem/drivers/sysmem/contiguous_pooled_memory_allocator.h
+++ b/src/devices/sysmem/drivers/sysmem/contiguous_pooled_memory_allocator.h
@@ -93,6 +93,7 @@
uint64_t max_free_size_at_high_water_mark_{};
inspect::Node node_;
+ inspect::ValueList properties_;
inspect::UintProperty size_property_;
inspect::UintProperty high_water_mark_property_;
inspect::UintProperty used_size_property_;
diff --git a/src/devices/sysmem/drivers/sysmem/device.cc b/src/devices/sysmem/drivers/sysmem/device.cc
index 667fd0b..fcd4da94a 100644
--- a/src/devices/sysmem/drivers/sysmem/device.cc
+++ b/src/devices/sysmem/drivers/sysmem/device.cc
@@ -26,6 +26,7 @@
#include <ddk/device.h>
#include <ddk/platform-defs.h>
+#include <fbl/string_printf.h>
#include "allocator.h"
#include "buffer_collection_token.h"
@@ -83,12 +84,15 @@
class SystemRamMemoryAllocator : public MemoryAllocator {
public:
- SystemRamMemoryAllocator()
+ SystemRamMemoryAllocator(Owner* parent_device)
: MemoryAllocator(BuildHeapPropertiesWithCoherencyDomainSupport(
true /*cpu*/, true /*ram*/, true /*inaccessible*/,
// Zircon guarantees created VMO are filled with 0; sysmem doesn't
// need to clear it once again.
- false /*need_clear*/)) {}
+ false /*need_clear*/)) {
+ node_ = parent_device->heap_node()->CreateChild("SysmemRamMemoryAllocator");
+ node_.CreateUint("id", id(), &properties_);
+ }
zx_status_t Allocate(uint64_t size, std::optional<std::string> name,
zx::vmo* parent_vmo) override {
@@ -115,6 +119,10 @@
// allocator since the VMOs independently track what pages they're using. So this allocator can
// always claim is_empty() true.
bool is_empty() override { return true; }
+
+ private:
+ inspect::Node node_;
+ inspect::ValueList properties_;
};
class ContiguousSystemRamMemoryAllocator : public MemoryAllocator {
@@ -125,7 +133,10 @@
// Zircon guarantees contagious VMO created are filled with 0;
// sysmem doesn't need to clear it once again.
false /*need_clear*/)),
- parent_device_(parent_device) {}
+ parent_device_(parent_device) {
+ node_ = parent_device_->heap_node()->CreateChild("ContiguousSystemRamMemoryAllocator");
+ node_.CreateUint("id", id(), &properties_);
+ }
zx_status_t Allocate(uint64_t size, std::optional<std::string> name,
zx::vmo* parent_vmo) override {
@@ -176,6 +187,8 @@
private:
Owner* const parent_device_;
+ inspect::Node node_;
+ inspect::ValueList properties_;
};
class ExternalMemoryAllocator : public MemoryAllocator {
@@ -187,7 +200,11 @@
: MemoryAllocator(std::move(properties)),
owner_(owner),
heap_(std::move(heap)),
- wait_for_close_(std::move(wait_for_close)) {}
+ wait_for_close_(std::move(wait_for_close)) {
+ node_ = owner->heap_node()->CreateChild(
+ fbl::StringPrintf("ExternalMemoryAllocator-%ld", id()).c_str());
+ node_.CreateUint("id", id(), &properties_);
+ }
~ExternalMemoryAllocator() { ZX_DEBUG_ASSERT(is_empty()); }
@@ -258,6 +275,9 @@
// From parent vmo handle to ID.
std::map<zx_handle_t, uint64_t> allocations_;
+
+ inspect::Node node_;
+ inspect::ValueList properties_;
};
fuchsia_sysmem_DriverConnector_ops_t driver_connector_ops = {
@@ -402,7 +422,7 @@
contiguous_memory_size = AlignUp(contiguous_memory_size, static_cast<int64_t>(ZX_PAGE_SIZE));
allocators_[llcpp::fuchsia::sysmem2::HeapType::SYSTEM_RAM] =
- std::make_unique<SystemRamMemoryAllocator>();
+ std::make_unique<SystemRamMemoryAllocator>(this);
status = pdev_.GetBti(0, &bti_);
if (status != ZX_OK) {
diff --git a/src/devices/sysmem/drivers/sysmem/device.h b/src/devices/sysmem/drivers/sysmem/device.h
index b7aa33a..a904aaf 100644
--- a/src/devices/sysmem/drivers/sysmem/device.h
+++ b/src/devices/sysmem/drivers/sysmem/device.h
@@ -79,6 +79,7 @@
[[nodiscard]] zx_status_t CreatePhysicalVmo(uint64_t base, uint64_t size,
zx::vmo* vmo_out) override;
void CheckForUnbind() override;
+ inspect::Node* heap_node() override { return &heaps_; }
[[nodiscard]] zx_status_t Connect(zx_handle_t allocator_request);
diff --git a/src/devices/sysmem/drivers/sysmem/logical_buffer_collection.cc b/src/devices/sysmem/drivers/sysmem/logical_buffer_collection.cc
index d682660..16b3fbe 100644
--- a/src/devices/sysmem/drivers/sysmem/logical_buffer_collection.cc
+++ b/src/devices/sysmem/drivers/sysmem/logical_buffer_collection.cc
@@ -1982,6 +1982,10 @@
}
}
+ node_.CreateUint("allocator_id", allocator->id(), &vmo_properties_);
+ node_.CreateUint("size_bytes", min_size_bytes, &vmo_properties_);
+ node_.CreateUint("heap", static_cast<uint64_t>(buffer_settings.heap()), &vmo_properties_);
+
// Now that min_size_bytes accounts for any ImageFormatConstraints, we can just allocate
// min_size_bytes buffers.
//
diff --git a/src/devices/sysmem/drivers/sysmem/memory_allocator.cc b/src/devices/sysmem/drivers/sysmem/memory_allocator.cc
index 21302ec..ddb5297 100644
--- a/src/devices/sysmem/drivers/sysmem/memory_allocator.cc
+++ b/src/devices/sysmem/drivers/sysmem/memory_allocator.cc
@@ -6,10 +6,15 @@
#include <zircon/assert.h>
+#include <atomic>
+
namespace sysmem_driver {
MemoryAllocator::MemoryAllocator(llcpp::fuchsia::sysmem2::HeapProperties properties)
- : heap_properties_(std::move(properties)) {}
+ : heap_properties_(std::move(properties)) {
+ static std::atomic_uint64_t id;
+ id_ = id++;
+}
MemoryAllocator::~MemoryAllocator() {
for (auto& it : destroy_callbacks_) {
diff --git a/src/devices/sysmem/drivers/sysmem/memory_allocator.h b/src/devices/sysmem/drivers/sysmem/memory_allocator.h
index ed9d2cf..676e884 100644
--- a/src/devices/sysmem/drivers/sysmem/memory_allocator.h
+++ b/src/devices/sysmem/drivers/sysmem/memory_allocator.h
@@ -8,6 +8,7 @@
#include <fuchsia/sysmem/llcpp/fidl.h>
#include <fuchsia/sysmem2/llcpp/fidl.h>
#include <lib/fit/function.h>
+#include <lib/inspect/cpp/inspect.h>
#include <lib/zx/bti.h>
#include <lib/zx/vmo.h>
@@ -21,6 +22,7 @@
// enables a fake in tests where we don't have a real zx::bti etc.
class Owner {
public:
+ virtual inspect::Node* heap_node() = 0;
virtual const zx::bti& bti() = 0;
virtual zx_status_t CreatePhysicalVmo(uint64_t base, uint64_t size, zx::vmo* vmo_out) = 0;
// Should be called after every delete that makes the allocator empty.
@@ -83,10 +85,14 @@
// Allocators must be empty before they're deleted.
virtual bool is_empty() = 0;
+ uint64_t id() const { return id_; }
+
public:
std::map<intptr_t, fit::callback<void()>> destroy_callbacks_;
private:
+ // This is a unique ID for the allocator on this system.
+ uint64_t id_{};
llcpp::fuchsia::sysmem2::HeapProperties heap_properties_;
};
diff --git a/src/devices/sysmem/drivers/sysmem/test/contiguous_pooled_memory_allocator_test.cc b/src/devices/sysmem/drivers/sysmem/test/contiguous_pooled_memory_allocator_test.cc
index 6220958..2111e47 100644
--- a/src/devices/sysmem/drivers/sysmem/test/contiguous_pooled_memory_allocator_test.cc
+++ b/src/devices/sysmem/drivers/sysmem/test/contiguous_pooled_memory_allocator_test.cc
@@ -19,7 +19,12 @@
class FakeOwner : public MemoryAllocator::Owner {
public:
- FakeOwner() { EXPECT_OK(fake_bti_create(bti_.reset_and_get_address())); }
+ explicit FakeOwner(inspect::Node* heap_node)
+ : heap_node_(heap_node)
+
+ {
+ EXPECT_OK(fake_bti_create(bti_.reset_and_get_address()));
+ }
~FakeOwner() {}
@@ -27,8 +32,10 @@
zx_status_t CreatePhysicalVmo(uint64_t base, uint64_t size, zx::vmo* vmo_out) override {
return zx::vmo::create(size, 0u, vmo_out);
}
+ inspect::Node* heap_node() override { return heap_node_; }
private:
+ inspect::Node* heap_node_;
zx::bti bti_;
};
@@ -47,8 +54,8 @@
static constexpr uint32_t kVmoCount = 1024;
static constexpr char kVmoName[] = "test-pool";
- FakeOwner fake_owner_;
inspect::Inspector inspector_;
+ FakeOwner fake_owner_{&inspector_.GetRoot()};
ContiguousPooledMemoryAllocator allocator_;
};
