From dd6fc61c2d21e4e2a075fe94604cf2a84d1a0e2c Mon Sep 17 00:00:00 2001 From: Graham Sider Date: Mon, 19 Jul 2021 17:11:59 -0400 Subject: [PATCH] libhsakmt: Remove redundant checks, small bug fixes Some g_system and Node ID checks already performed by validate_nodeid(). Fixes bug where hsaKmtGetNodeProperties() would return on validate_nodeid() error without releasing lock. Switches some conditionals to return INVALID_NODE_UNIT instead of INVALID_PARAMETER if NodeId >= NumNodes. Removes some g_system assertions to return error code rather than trigger a hard fault. Signed-off-by: Graham Sider Change-Id: Id1604b20c2cef8808b98cdad61bd47aa7ea3d229 [ROCm/ROCR-Runtime commit: 29369753f5d2dea1df58214ff326c736f6e83ca6] --- projects/rocr-runtime/src/topology.c | 37 +++---------------- .../tests/kfdtest/src/KFDTopologyTest.cpp | 2 +- 2 files changed, 6 insertions(+), 33 deletions(-) diff --git a/projects/rocr-runtime/src/topology.c b/projects/rocr-runtime/src/topology.c index b562d7690a..8475e41af8 100644 --- a/projects/rocr-runtime/src/topology.c +++ b/projects/rocr-runtime/src/topology.c @@ -2036,21 +2036,9 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtGetNodeProperties(HSAuint32 NodeId, CHECK_KFD_OPEN(); pthread_mutex_lock(&hsakmt_mutex); - /* KFD ADD page 18, snapshot protocol violation */ - if (!g_system) { - err = HSAKMT_STATUS_INVALID_NODE_UNIT; - assert(g_system); - goto out; - } - - if (NodeId >= g_system->NumNodes) { - err = HSAKMT_STATUS_INVALID_PARAMETER; - goto out; - } - err = validate_nodeid(NodeId, &gpu_id); if (err != HSAKMT_STATUS_SUCCESS) - return err; + goto out; *NodeProperties = g_props[NodeId].node; /* For CPU only node don't add any additional GPU memory banks. */ @@ -2085,19 +2073,6 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtGetNodeMemoryProperties(HSAuint32 NodeId, CHECK_KFD_OPEN(); pthread_mutex_lock(&hsakmt_mutex); - /* KFD ADD page 18, snapshot protocol violation */ - if (!g_system) { - err = HSAKMT_STATUS_INVALID_NODE_UNIT; - assert(g_system); - goto out; - } - - /* Check still necessary */ - if (NodeId >= g_system->NumNodes) { - err = HSAKMT_STATUS_INVALID_PARAMETER; - goto out; - } - err = validate_nodeid(NodeId, &gpu_id); if (err != HSAKMT_STATUS_SUCCESS) goto out; @@ -2183,13 +2158,12 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtGetNodeCacheProperties(HSAuint32 NodeId, pthread_mutex_lock(&hsakmt_mutex); /* KFD ADD page 18, snapshot protocol violation */ - if (!g_system) { + if (!g_system || NodeId >= g_system->NumNodes) { err = HSAKMT_STATUS_INVALID_NODE_UNIT; - assert(g_system); goto out; } - if (NodeId >= g_system->NumNodes || NumCaches > g_props[NodeId].node.NumCaches) { + if (NumCaches > g_props[NodeId].node.NumCaches) { err = HSAKMT_STATUS_INVALID_PARAMETER; goto out; } @@ -2221,13 +2195,12 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtGetNodeIoLinkProperties(HSAuint32 NodeId, pthread_mutex_lock(&hsakmt_mutex); /* KFD ADD page 18, snapshot protocol violation */ - if (!g_system) { + if (!g_system || NodeId >= g_system->NumNodes ) { err = HSAKMT_STATUS_INVALID_NODE_UNIT; - assert(g_system); goto out; } - if (NodeId >= g_system->NumNodes || NumIoLinks > g_props[NodeId].node.NumIOLinks) { + if (NumIoLinks > g_props[NodeId].node.NumIOLinks) { err = HSAKMT_STATUS_INVALID_PARAMETER; goto out; } diff --git a/projects/rocr-runtime/tests/kfdtest/src/KFDTopologyTest.cpp b/projects/rocr-runtime/tests/kfdtest/src/KFDTopologyTest.cpp index bad2533d49..b4b406a24a 100644 --- a/projects/rocr-runtime/tests/kfdtest/src/KFDTopologyTest.cpp +++ b/projects/rocr-runtime/tests/kfdtest/src/KFDTopologyTest.cpp @@ -90,7 +90,7 @@ TEST_F(KFDTopologyTest, GetNodePropertiesInvalidNodeNum) { HsaNodeProperties nodeProperties; memset(&nodeProperties, 0, sizeof(nodeProperties)); - EXPECT_EQ(HSAKMT_STATUS_INVALID_PARAMETER, hsaKmtGetNodeProperties(m_SystemProperties.NumNodes, &nodeProperties)); + EXPECT_EQ(HSAKMT_STATUS_INVALID_NODE_UNIT, hsaKmtGetNodeProperties(m_SystemProperties.NumNodes, &nodeProperties)); TEST_END }