gfxstream: guest: nuke Android HealthMonitor

The HealthMonitor is hang detection at the API Vulkan
encoder level.

It's actually not used in CF/AAOS/Emualtor.  The
plan for AOSP is actually to increase the amount
of Android CTS that are run against gfxstream to
increase health/stability, for example (b/347288539).

So if we consistently pass CTS on main with gfxstream
(as is the plan), HealthMonitoring would be somewhat
redundant.

Also, AndroidHealthMonitor is somewhat duplicated with
libaemu's HealthMonitor as well.

Also, nuke EncoderAutoLock while we're at it.  It also
is unused code.

Reviewed-by: Aaron Ruby <aruby@blackberry.com>
Acked-by: Yonggang Luo <luoyonggang@gmail.com>
Acked-by: Adam Jackson <ajax@redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27246>
This commit is contained in:
Gurchetan Singh
2024-08-05 14:23:31 -07:00
committed by Marge Bot
parent f42b3be1b5
commit 2f161c31e3
10 changed files with 57 additions and 143 deletions

View File

@@ -21,7 +21,7 @@ encoder_decl_preamble = """
class VkEncoder {
public:
VkEncoder(gfxstream::guest::IOStream* stream, gfxstream::guest::HealthMonitor<>* healthMonitor = nullptr);
VkEncoder(gfxstream::guest::IOStream* stream);
~VkEncoder();
#include "VkEncoder.h.inl"
@@ -31,7 +31,6 @@ encoder_decl_postamble = """
private:
class Impl;
std::unique_ptr<Impl> mImpl;
gfxstream::guest::HealthMonitor<>* mHealthMonitor;
};
"""
@@ -39,8 +38,6 @@ encoder_impl_preamble ="""
using namespace gfxstream::vk;
using gfxstream::guest::AutoLock;
using gfxstream::guest::Lock;
using gfxstream::guest::BumpPool;
#include "VkEncoder.cpp.inl"
@@ -363,7 +360,6 @@ def emit_parameter_encode_write_packet_info(typeInfo, api, cgen):
cgen.stmt("uint32_t packetSize_%s = 4 + 4 + (queueSubmitWithCommandsEnabled ? 4 : 0) + count" % (api.name))
else:
cgen.stmt("uint32_t packetSize_%s = 4 + 4 + count" % (api.name))
cgen.stmt("healthMonitorAnnotation_packetSize = std::make_optional(packetSize_%s)" % (api.name))
if not doDispatchSerialize:
cgen.stmt("if (queueSubmitWithCommandsEnabled) packetSize_%s -= 8" % api.name)
@@ -375,7 +371,6 @@ def emit_parameter_encode_write_packet_info(typeInfo, api, cgen):
if doSeqno:
cgen.stmt("uint32_t seqno; if (queueSubmitWithCommandsEnabled) seqno = ResourceTracker::nextSeqno()")
cgen.stmt("healthMonitorAnnotation_seqno = std::make_optional(seqno)")
cgen.stmt("memcpy(streamPtr, &opcode_%s, sizeof(uint32_t)); streamPtr += sizeof(uint32_t)" % api.name)
cgen.stmt("memcpy(streamPtr, &packetSize_%s, sizeof(uint32_t)); streamPtr += sizeof(uint32_t)" % api.name)
@@ -398,12 +393,6 @@ def emit_parameter_encode_do_parameter_write(typeInfo, api, cgen):
dispatchDone = True
cgen.beginIf("watchdog")
cgen.stmt("size_t watchdogBufSize = std::min<size_t>(static_cast<size_t>(packetSize_%s), kWatchdogBufferMax)" % (api.name))
cgen.stmt("healthMonitorAnnotation_packetContents.resize(watchdogBufSize)")
cgen.stmt("memcpy(&healthMonitorAnnotation_packetContents[0], packetBeginPtr, watchdogBufSize)")
cgen.endIf()
def emit_parameter_encode_read(typeInfo, api, cgen):
encodingParams = EncodingParameters(api)
@@ -495,31 +484,6 @@ def emit_debug_log(typeInfo, api, cgen):
cgen.stmt("ENCODER_DEBUG_LOG(\"%s(%s)\", %s)" % (api.name, logFormatStr, logVargsStr))
def emit_health_watchdog(api, cgen):
cgen.stmt("std::optional<uint32_t> healthMonitorAnnotation_seqno = std::nullopt")
cgen.stmt("std::optional<uint32_t> healthMonitorAnnotation_packetSize = std::nullopt")
cgen.stmt("std::vector<uint8_t> healthMonitorAnnotation_packetContents")
cgen.line("""
auto watchdog = WATCHDOG_BUILDER(mHealthMonitor, \"%s in VkEncoder\")
.setOnHangCallback([&]() {
auto annotations = std::make_unique<EventHangMetadata::HangAnnotations>();
if (healthMonitorAnnotation_seqno) {
annotations->insert({{"seqno", std::to_string(healthMonitorAnnotation_seqno.value())}});
}
if (healthMonitorAnnotation_packetSize) {
annotations->insert({{"packetSize", std::to_string(healthMonitorAnnotation_packetSize.value())}});
}
if (!healthMonitorAnnotation_packetContents.empty()) {
annotations->insert(
{{"packetContents", getPacketContents(
&healthMonitorAnnotation_packetContents[0], healthMonitorAnnotation_packetContents.size())}});
}
return std::move(annotations);
})
.build();
"""% (api.name)
)
def emit_default_encoding(typeInfo, api, cgen):
emit_debug_log(typeInfo, api, cgen)
emit_lock(cgen)
@@ -710,7 +674,6 @@ class VulkanEncoder(VulkanWrapperGenerator):
self.module.appendHeader(self.cgenHeader.swapCode())
def emit_function_impl(cgen):
emit_health_watchdog(api, cgen)
if api.name in custom_encodes.keys():
custom_encodes[api.name](self.typeInfo, api, cgen)
else:

View File

@@ -310,7 +310,6 @@ class CerealGenerator(OutputGenerator):
self.hostCommonExtraVulkanHeaders = '#include "vk_android_native_buffer_gfxstream.h"'
encoderInclude = f"""
#include "{self.guestBaseLibDirPrefix}/AndroidHealthMonitor.h"
#include "goldfish_vk_private_defs.h"
#include <memory>
@@ -330,7 +329,6 @@ class IOStream;
#include "{self.guestBaseLibDirPrefix}/AlignedBuf.h"
#include "{self.guestBaseLibDirPrefix}/BumpPool.h"
#include "{self.guestBaseLibDirPrefix}/synchronization/AndroidLock.h"
#include <cutils/properties.h>

View File

@@ -34,39 +34,33 @@
static const size_t kReadSize = 512 * 1024;
static const size_t kWriteOffset = kReadSize;
AddressSpaceStream::AddressSpaceStream(
address_space_handle_t handle,
uint32_t version,
struct asg_context context,
uint64_t ringOffset,
uint64_t writeBufferOffset,
struct address_space_ops ops,
HealthMonitor<>* healthMonitor) :
IOStream(context.ring_config->flush_interval),
m_ops(ops),
m_tmpBuf(0),
m_tmpBufSize(0),
m_tmpBufXferSize(0),
m_usingTmpBuf(0),
m_readBuf(0),
m_read(0),
m_readLeft(0),
m_handle(handle),
m_version(version),
m_context(context),
m_ringOffset(ringOffset),
m_writeBufferOffset(writeBufferOffset),
m_writeBufferSize(context.ring_config->buffer_size),
m_writeBufferMask(m_writeBufferSize - 1),
m_buf((unsigned char*)context.buffer),
m_writeStart(m_buf),
m_writeStep(context.ring_config->flush_interval),
m_notifs(0),
m_written(0),
m_backoffIters(0),
m_backoffFactor(1),
m_ringStorageSize(sizeof(struct asg_ring_storage) + m_writeBufferSize),
m_healthMonitor(healthMonitor) {
AddressSpaceStream::AddressSpaceStream(address_space_handle_t handle, uint32_t version,
struct asg_context context, uint64_t ringOffset,
uint64_t writeBufferOffset, struct address_space_ops ops)
: IOStream(context.ring_config->flush_interval),
m_ops(ops),
m_tmpBuf(0),
m_tmpBufSize(0),
m_tmpBufXferSize(0),
m_usingTmpBuf(0),
m_readBuf(0),
m_read(0),
m_readLeft(0),
m_handle(handle),
m_version(version),
m_context(context),
m_ringOffset(ringOffset),
m_writeBufferOffset(writeBufferOffset),
m_writeBufferSize(context.ring_config->buffer_size),
m_writeBufferMask(m_writeBufferSize - 1),
m_buf((unsigned char*)context.buffer),
m_writeStart(m_buf),
m_writeStep(context.ring_config->flush_interval),
m_notifs(0),
m_written(0),
m_backoffIters(0),
m_backoffFactor(1),
m_ringStorageSize(sizeof(struct asg_ring_storage) + m_writeBufferSize) {
// We'll use this in the future, but at the moment,
// it's a potential compile Werror.
(void)m_ringStorageSize;
@@ -95,8 +89,7 @@ size_t AddressSpaceStream::idealAllocSize(size_t len) {
return m_writeStep;
}
void *AddressSpaceStream::allocBuffer(size_t minSize) {
auto watchdog = WATCHDOG_BUILDER(m_healthMonitor, "ASG watchdog").build();
void* AddressSpaceStream::allocBuffer(size_t minSize) {
AEMU_SCOPED_TRACE("allocBuffer");
ensureType3Finished();
@@ -242,9 +235,7 @@ const unsigned char *AddressSpaceStream::read(void *buf, size_t *inout_len) {
return (const unsigned char*)dst;
}
int AddressSpaceStream::writeFully(const void *buf, size_t size)
{
auto watchdog = WATCHDOG_BUILDER(m_healthMonitor, "ASG watchdog").build();
int AddressSpaceStream::writeFully(const void* buf, size_t size) {
AEMU_SCOPED_TRACE("writeFully");
ensureType3Finished();
ensureType1Finished();
@@ -308,9 +299,7 @@ int AddressSpaceStream::writeFully(const void *buf, size_t size)
return 0;
}
int AddressSpaceStream::writeFullyAsync(const void *buf, size_t size)
{
auto watchdog = WATCHDOG_BUILDER(m_healthMonitor, "ASG watchdog").build();
int AddressSpaceStream::writeFullyAsync(const void* buf, size_t size) {
AEMU_SCOPED_TRACE("writeFullyAsync");
ensureType3Finished();
ensureType1Finished();
@@ -432,7 +421,6 @@ ssize_t AddressSpaceStream::speculativeRead(unsigned char* readBuffer, size_t tr
}
void AddressSpaceStream::notifyAvailable() {
auto watchdog = WATCHDOG_BUILDER(m_healthMonitor, "ASG watchdog").build();
AEMU_SCOPED_TRACE("PING");
struct address_space_ping request;
request.metadata = ASG_NOTIFY_AVAILABLE;
@@ -475,7 +463,6 @@ void AddressSpaceStream::ensureConsumerFinishing() {
}
void AddressSpaceStream::ensureType1Finished() {
auto watchdog = WATCHDOG_BUILDER(m_healthMonitor, "ASG watchdog").build();
AEMU_SCOPED_TRACE("ensureType1Finished");
uint32_t currAvailRead =
@@ -492,7 +479,6 @@ void AddressSpaceStream::ensureType1Finished() {
}
void AddressSpaceStream::ensureType3Finished() {
auto watchdog = WATCHDOG_BUILDER(m_healthMonitor, "ASG watchdog").build();
AEMU_SCOPED_TRACE("ensureType3Finished");
uint32_t availReadLarge =
ring_buffer_available_read(
@@ -516,8 +502,6 @@ void AddressSpaceStream::ensureType3Finished() {
}
int AddressSpaceStream::type1Write(uint32_t bufferOffset, size_t size) {
auto watchdog = WATCHDOG_BUILDER(m_healthMonitor, "ASG watchdog").build();
AEMU_SCOPED_TRACE("type1Write");
ensureType3Finished();

View File

@@ -16,8 +16,7 @@
#include "goldfish_address_space.h"
AddressSpaceStream* createGoldfishAddressSpaceStream(size_t ignored_bufSize,
HealthMonitor<>* healthMonitor) {
AddressSpaceStream* createGoldfishAddressSpaceStream(size_t ignored_bufSize) {
// Ignore incoming ignored_bufSize
(void)ignored_bufSize;
@@ -122,10 +121,8 @@ AddressSpaceStream* createGoldfishAddressSpaceStream(size_t ignored_bufSize,
.ping = goldfish_address_space_ping,
};
AddressSpaceStream* res =
new AddressSpaceStream(
child_device_handle, version, context,
ringOffset, bufferOffset, ops, healthMonitor);
AddressSpaceStream* res = new AddressSpaceStream(child_device_handle, version, context,
ringOffset, bufferOffset, ops);
return res;
}
}

View File

@@ -14,6 +14,8 @@
#include "VirtioGpuAddressSpaceStream.h"
#include <cutils/log.h>
#include "util.h"
static bool GetRingParamsFromCapset(enum VirtGpuCapset capset, const VirtGpuCaps& caps,
@@ -74,8 +76,7 @@ bool virtgpu_address_space_ping(address_space_handle_t, struct address_space_pin
return true;
}
AddressSpaceStream* createVirtioGpuAddressSpaceStream(enum VirtGpuCapset capset,
HealthMonitor<>* healthMonitor) {
AddressSpaceStream* createVirtioGpuAddressSpaceStream(enum VirtGpuCapset capset) {
VirtGpuResourcePtr pipe, blob;
VirtGpuResourceMappingPtr pipeMapping, blobMapping;
struct VirtGpuExecBuffer exec = {};
@@ -141,7 +142,7 @@ AddressSpaceStream* createVirtioGpuAddressSpaceStream(enum VirtGpuCapset capset,
};
AddressSpaceStream* res =
new AddressSpaceStream((address_space_handle_t)(-1), 1, context, 0, 0, ops, healthMonitor);
new AddressSpaceStream((address_space_handle_t)(-1), 1, context, 0, 0, ops);
res->setMapping(blobMapping);
res->setResourceId(contextCreate.resourceId);

View File

@@ -19,38 +19,29 @@
#include "VirtGpu.h"
#include "address_space.h"
#include "address_space_graphics_types.h"
#include "aemu/base/AndroidHealthMonitor.h"
#include "gfxstream/guest/IOStream.h"
using gfxstream::guest::HealthMonitor;
using gfxstream::guest::IOStream;
class AddressSpaceStream : public IOStream {
public:
explicit AddressSpaceStream(
address_space_handle_t handle,
uint32_t version,
struct asg_context context,
uint64_t ringOffset,
uint64_t writeBufferOffset,
struct address_space_ops ops,
HealthMonitor<>* healthMonitor);
~AddressSpaceStream();
explicit AddressSpaceStream(address_space_handle_t handle, uint32_t version,
struct asg_context context, uint64_t ringOffset,
uint64_t writeBufferOffset, struct address_space_ops ops);
~AddressSpaceStream();
virtual size_t idealAllocSize(size_t len);
virtual void *allocBuffer(size_t minSize);
virtual int commitBuffer(size_t size);
virtual const unsigned char *readFully( void *buf, size_t len);
virtual const unsigned char *read( void *buf, size_t *inout_len);
virtual int writeFully(const void *buf, size_t len);
virtual int writeFullyAsync(const void *buf, size_t len);
virtual const unsigned char *commitBufferAndReadFully(size_t size, void *buf, size_t len);
virtual size_t idealAllocSize(size_t len);
virtual void* allocBuffer(size_t minSize);
virtual int commitBuffer(size_t size);
virtual const unsigned char* readFully(void* buf, size_t len);
virtual const unsigned char* read(void* buf, size_t* inout_len);
virtual int writeFully(const void* buf, size_t len);
virtual int writeFullyAsync(const void* buf, size_t len);
virtual const unsigned char* commitBufferAndReadFully(size_t size, void* buf, size_t len);
void setMapping(VirtGpuResourceMappingPtr mapping) { m_mapping = mapping; }
void setMapping(VirtGpuResourceMappingPtr mapping) { m_mapping = mapping; }
void setResourceId(uint32_t id) {
m_resourceId = id;
}
void setResourceId(uint32_t id) { m_resourceId = id; }
private:
bool isInError() const;
@@ -99,8 +90,6 @@ private:
size_t m_ringStorageSize;
uint32_t m_resourceId = 0;
HealthMonitor<>* m_healthMonitor;
};
#endif

View File

@@ -16,4 +16,4 @@
#include "AddressSpaceStream.h"
AddressSpaceStream* createGoldfishAddressSpaceStream(size_t bufSize, gfxstream::guest::HealthMonitor<>* healthMonitor);
AddressSpaceStream* createGoldfishAddressSpaceStream(size_t bufSize);

View File

@@ -16,5 +16,4 @@
#include "AddressSpaceStream.h"
AddressSpaceStream* createVirtioGpuAddressSpaceStream(
enum VirtGpuCapset capset, gfxstream::guest::HealthMonitor<>* healthMonitor);
AddressSpaceStream* createVirtioGpuAddressSpaceStream(enum VirtGpuCapset capset);

View File

@@ -25,7 +25,6 @@ class VkEncoder::Impl {
void log(const char* text) {
if (!m_logEncodes) return;
ALOGD("encoder log: %s", text);
}
void flush() {
@@ -54,15 +53,8 @@ class VkEncoder::Impl {
VkEncoder::~VkEncoder() {}
struct EncoderAutoLock {
EncoderAutoLock(VkEncoder* enc) : mEnc(enc) { mEnc->lock(); }
~EncoderAutoLock() { mEnc->unlock(); }
VkEncoder* mEnc;
};
VkEncoder::VkEncoder(gfxstream::guest::IOStream* stream,
gfxstream::guest::HealthMonitor<>* healthMonitor)
: mImpl(new VkEncoder::Impl(stream)), mHealthMonitor(healthMonitor) {}
VkEncoder::VkEncoder(gfxstream::guest::IOStream* stream)
: mImpl(new VkEncoder::Impl(stream)) {}
void VkEncoder::flush() { mImpl->flush(); }

View File

@@ -27,14 +27,11 @@ size_t* countPtr = &count;
}
bool queueSubmitWithCommandsEnabled = sFeatureBits & VULKAN_STREAM_FEATURE_QUEUE_SUBMIT_WITH_COMMANDS_BIT;
uint32_t packetSize_vkQueueFlushCommandsGOOGLE = 4 + 4 + (queueSubmitWithCommandsEnabled ? 4 : 0) + count;
healthMonitorAnnotation_packetSize =
std::make_optional(packetSize_vkQueueFlushCommandsGOOGLE);
uint8_t* streamPtr = stream->reserve(packetSize_vkQueueFlushCommandsGOOGLE - local_dataSize);
uint8_t* packetBeginPtr = streamPtr;
uint8_t** streamPtrPtr = &streamPtr;
uint32_t opcode_vkQueueFlushCommandsGOOGLE = OP_vkQueueFlushCommandsGOOGLE;
uint32_t seqno = ResourceTracker::nextSeqno();
healthMonitorAnnotation_seqno = std::make_optional(seqno);
memcpy(streamPtr, &opcode_vkQueueFlushCommandsGOOGLE, sizeof(uint32_t)); streamPtr += sizeof(uint32_t);
memcpy(streamPtr, &packetSize_vkQueueFlushCommandsGOOGLE, sizeof(uint32_t)); streamPtr += sizeof(uint32_t);
memcpy(streamPtr, &seqno, sizeof(uint32_t)); streamPtr += sizeof(uint32_t);
@@ -48,12 +45,6 @@ memcpy(*streamPtrPtr, (uint64_t*)&cgen_var_1408, 1 * 8);
*streamPtrPtr += 1 * 8;
memcpy(*streamPtrPtr, (VkDeviceSize*)&local_dataSize, sizeof(VkDeviceSize));
*streamPtrPtr += sizeof(VkDeviceSize);
if (watchdog) {
size_t watchdogBufSize = std::min<size_t>(
static_cast<size_t>(packetSize_vkQueueFlushCommandsGOOGLE), kWatchdogBufferMax);
healthMonitorAnnotation_packetContents.resize(watchdogBufSize);
memcpy(&healthMonitorAnnotation_packetContents[0], packetBeginPtr, watchdogBufSize);
}
MESA_TRACE_SCOPE("vkQueueFlush large xfer");
stream->flush();