Skip to content

Commit aa85d71

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 <bellekallu.rajkiran@intel.com>
1 parent a1c7f41 commit aa85d71

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
@@ -37,6 +37,7 @@ class FdCache {
3737
int getFd(std::string file);
3838

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

4243
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
@@ -250,13 +250,58 @@ TEST(FdCacheTest, GivenValidFdCacheWhenCallingGetFdOnSameFileThenVerifyCacheIsUp
250250
// Get Fd after the cache is full.
251251
EXPECT_LE(0, pFdCache->getFd("dummy.txt"));
252252

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

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

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

262307
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
@@ -37,6 +37,7 @@ class FdCache {
3737
int getFd(std::string file);
3838

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

4243
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
@@ -488,13 +488,58 @@ TEST(FdCacheTest, GivenValidFdCacheWhenCallingGetFdOnSameFileThenVerifyCacheIsUp
488488
// Get Fd after the cache is full.
489489
EXPECT_LE(0, pFdCache->getFd("dummy.txt"));
490490

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

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

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

500545
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
@@ -47,6 +47,7 @@ extern DIR *(*sysCallsOpendir)(const char *name);
4747
extern struct dirent *(*sysCallsReaddir)(DIR *dir);
4848
extern int (*sysCallsClosedir)(DIR *dir);
4949
extern int (*sysCallsGetDevicePath)(int deviceFd, char *buf, size_t &bufSize);
50+
extern int (*sysCallsClose)(int fileDescriptor);
5051

5152
extern int flockRetVal;
5253
extern int openFuncRetVal;

0 commit comments

Comments
 (0)