Skip to content

Commit 00f2609

Browse files
fix: Fix sysman abort on destruction
Invoking close on invalid fd result in undesired behaviour. Pass proper file descriptor for close to avoid unnecessary aborts during destruction. Related-To: GSD-6816 Signed-off-by: Bellekallu Rajkiran <[email protected]> Source: aa85d71
1 parent 5f9d018 commit 00f2609

File tree

9 files changed

+99
-5
lines changed

9 files changed

+99
-5
lines changed

level_zero/sysman/source/linux/sysman_fs_access.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ int FdCache::getFd(std::string file) {
6767

6868
FdCache::~FdCache() {
6969
for (auto it = fdMap.begin(); it != fdMap.end(); ++it) {
70-
NEO::SysCalls::close(it->second.second);
70+
NEO::SysCalls::close(it->second.first);
7171
}
7272
fdMap.clear();
7373
}

level_zero/sysman/source/linux/sysman_fs_access.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ class FdCache {
3838
int getFd(std::string file);
3939

4040
protected:
41+
// Map of File name to pair of file descriptor and reference count to file.
4142
std::map<std::string, std::pair<int, uint32_t>> fdMap = {};
4243

4344
private:

level_zero/sysman/source/shared/linux/sysman_fs_access_interface.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ int FdCacheInterface::getFd(std::string file) {
6464

6565
FdCacheInterface::~FdCacheInterface() {
6666
for (auto it = fdMap.begin(); it != fdMap.end(); ++it) {
67-
NEO::SysCalls::close(it->second.second);
67+
NEO::SysCalls::close(it->second.first);
6868
}
6969
fdMap.clear();
7070
}

level_zero/sysman/source/shared/linux/sysman_fs_access_interface.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class FdCacheInterface {
2727
int getFd(std::string file);
2828

2929
protected:
30+
// Map of File name to pair of file descriptor and reference count to file.
3031
std::map<std::string, std::pair<int, uint32_t>> fdMap = {};
3132

3233
private:

level_zero/sysman/test/unit_tests/sources/linux/test_sysman.cpp

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,13 +249,58 @@ TEST(FdCacheTest, GivenValidFdCacheWhenCallingGetFdOnSameFileThenVerifyCacheIsUp
249249
// Get Fd after the cache is full.
250250
EXPECT_LE(0, pFdCache->getFd("dummy.txt"));
251251

252-
// Verify Cache have the elemets that are accessed more number of times
252+
// Verify Cache have the elements that are accessed more number of times
253253
EXPECT_NE(pFdCache->fdMap.end(), pFdCache->fdMap.find("mockfile0.txt"));
254254

255255
// Verify cache doesn't have an element that is accessed less number of times.
256256
EXPECT_EQ(pFdCache->fdMap.end(), pFdCache->fdMap.find("mockfile9.txt"));
257257
}
258258

259+
TEST(FdCacheTest, GivenValidFdCacheWhenClearingCacheThenVerifyProperFdsAreClosedAndCacheIsUpdatedProperly) {
260+
261+
class MockFdCache : public FdCache {
262+
public:
263+
using FdCache::fdMap;
264+
};
265+
266+
VariableBackup<decltype(NEO::SysCalls::sysCallsOpen)> mockOpen(&NEO::SysCalls::sysCallsOpen, [](const char *pathname, int flags) -> int {
267+
return 1;
268+
});
269+
270+
VariableBackup<decltype(NEO::SysCalls::sysCallsClose)> mockClose(&NEO::SysCalls::sysCallsClose, [](int fileDescriptor) -> int {
271+
EXPECT_EQ(fileDescriptor, 1);
272+
return 0;
273+
});
274+
275+
MockFdCache *pFdCache = new MockFdCache();
276+
std::string fileName = {};
277+
for (auto i = 0; i < L0::Sysman::FdCache::maxSize; i++) {
278+
fileName = "mockfile" + std::to_string(i) + ".txt";
279+
EXPECT_LE(0, pFdCache->getFd(fileName));
280+
}
281+
282+
fileName = "mockfile0.txt";
283+
for (auto i = 0; i < 3; i++) {
284+
EXPECT_LE(0, pFdCache->getFd(fileName));
285+
}
286+
287+
for (auto i = 1; i < L0::Sysman::FdCache::maxSize - 1; i++) {
288+
fileName = "mockfile" + std::to_string(i) + ".txt";
289+
EXPECT_LE(0, pFdCache->getFd(fileName));
290+
}
291+
292+
// Get Fd after the cache is full.
293+
EXPECT_LE(0, pFdCache->getFd("dummy.txt"));
294+
295+
// Verify Cache have the elements that are accessed more number of times
296+
EXPECT_NE(pFdCache->fdMap.end(), pFdCache->fdMap.find("mockfile0.txt"));
297+
298+
// Verify cache doesn't have an element that is accessed less number of times.
299+
EXPECT_EQ(pFdCache->fdMap.end(), pFdCache->fdMap.find("mockfile9.txt"));
300+
301+
delete pFdCache;
302+
}
303+
259304
TEST_F(SysmanDeviceFixture, GivenSysfsAccessClassAndOpenSysCallFailsWhenCallingReadThenFailureIsReturned) {
260305

261306
VariableBackup<decltype(NEO::SysCalls::sysCallsOpen)> mockOpen(&NEO::SysCalls::sysCallsOpen, [](const char *pathname, int flags) -> int {

level_zero/tools/source/sysman/linux/fs_access.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ int FdCache::getFd(std::string file) {
6565

6666
FdCache::~FdCache() {
6767
for (auto it = fdMap.begin(); it != fdMap.end(); ++it) {
68-
NEO::SysCalls::close(it->second.second);
68+
NEO::SysCalls::close(it->second.first);
6969
}
7070
fdMap.clear();
7171
}

level_zero/tools/source/sysman/linux/fs_access.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ class FdCache {
3838
int getFd(std::string file);
3939

4040
protected:
41+
// Map of File name to pair of file descriptor and reference count to file.
4142
std::map<std::string, std::pair<int, uint32_t>> fdMap = {};
4243

4344
private:

level_zero/tools/test/unit_tests/sources/sysman/linux/test_sysman.cpp

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,13 +487,58 @@ TEST(FdCacheTest, GivenValidFdCacheWhenCallingGetFdOnSameFileThenVerifyCacheIsUp
487487
// Get Fd after the cache is full.
488488
EXPECT_LE(0, pFdCache->getFd("dummy.txt"));
489489

490-
// Verify Cache have the elemets that are accessed more number of times
490+
// Verify Cache have the elements that are accessed more number of times
491491
EXPECT_NE(pFdCache->fdMap.end(), pFdCache->fdMap.find("mockfile0.txt"));
492492

493493
// Verify cache doesn't have an element that is accessed less number of times.
494494
EXPECT_EQ(pFdCache->fdMap.end(), pFdCache->fdMap.find("mockfile9.txt"));
495495
}
496496

497+
TEST(FdCacheTest, GivenValidFdCacheWhenClearingCacheThenVerifyProperFdsAreClosedAndCacheIsUpdatedProperly) {
498+
499+
class MockFdCache : public FdCache {
500+
public:
501+
using FdCache::fdMap;
502+
};
503+
504+
VariableBackup<decltype(NEO::SysCalls::sysCallsOpen)> mockOpen(&NEO::SysCalls::sysCallsOpen, [](const char *pathname, int flags) -> int {
505+
return 1;
506+
});
507+
508+
VariableBackup<decltype(NEO::SysCalls::sysCallsClose)> mockClose(&NEO::SysCalls::sysCallsClose, [](int fileDescriptor) -> int {
509+
EXPECT_EQ(fileDescriptor, 1);
510+
return 0;
511+
});
512+
513+
MockFdCache *pFdCache = new MockFdCache();
514+
std::string fileName = {};
515+
for (auto i = 0; i < L0::FdCache::maxSize; i++) {
516+
fileName = "mockfile" + std::to_string(i) + ".txt";
517+
EXPECT_LE(0, pFdCache->getFd(fileName));
518+
}
519+
520+
fileName = "mockfile0.txt";
521+
for (auto i = 0; i < 3; i++) {
522+
EXPECT_LE(0, pFdCache->getFd(fileName));
523+
}
524+
525+
for (auto i = 1; i < L0::FdCache::maxSize - 1; i++) {
526+
fileName = "mockfile" + std::to_string(i) + ".txt";
527+
EXPECT_LE(0, pFdCache->getFd(fileName));
528+
}
529+
530+
// Get Fd after the cache is full.
531+
EXPECT_LE(0, pFdCache->getFd("dummy.txt"));
532+
533+
// Verify Cache have the elements that are accessed more number of times
534+
EXPECT_NE(pFdCache->fdMap.end(), pFdCache->fdMap.find("mockfile0.txt"));
535+
536+
// Verify cache doesn't have an element that is accessed less number of times.
537+
EXPECT_EQ(pFdCache->fdMap.end(), pFdCache->fdMap.find("mockfile9.txt"));
538+
539+
delete pFdCache;
540+
}
541+
497542
TEST_F(SysmanDeviceFixture, GivenSysfsAccessClassAndUnsignedIntegerWhenCallingReadThenSuccessIsReturned) {
498543

499544
VariableBackup<decltype(NEO::SysCalls::sysCallsOpen)> mockOpen(&NEO::SysCalls::sysCallsOpen, [](const char *pathname, int flags) -> int {

shared/test/common/os_interface/linux/sys_calls_linux_ult.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ extern DIR *(*sysCallsOpendir)(const char *name);
4646
extern struct dirent *(*sysCallsReaddir)(DIR *dir);
4747
extern int (*sysCallsClosedir)(DIR *dir);
4848
extern int (*sysCallsGetDevicePath)(int deviceFd, char *buf, size_t &bufSize);
49+
extern int (*sysCallsClose)(int fileDescriptor);
4950

5051
extern int flockRetVal;
5152
extern int closeFuncRetVal;

0 commit comments

Comments
 (0)