Skip to content

Commit

Permalink
calls rewinddir before calling readdirat
Browse files Browse the repository at this point in the history
Summary:
https://pubs.opengroup.org/onlinepubs/007904875/functions/rewinddir.html
> The rewinddir() function shall reset the position of the directory stream to which dirp refers to the beginning of the directory. It shall also cause the directory stream to refer to the current state of the corresponding directory, as a call to opendir() would have done.

For some FS (like btrfs on newer kernels), new entries added after calling `opendir` and `rewinddir` are not available from readdir. Therefore, we should call `rewinddir` right before calling `readdir`.

Reviewed By: dschatzberg

Differential Revision: D58393743

fbshipit-source-id: 4a195b288ff7b304d43241075ce9128e38080e27
  • Loading branch information
lnyng authored and facebook-github-bot committed Jun 12, 2024
1 parent 78b8a47 commit b1018d8
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 48 deletions.
84 changes: 41 additions & 43 deletions src/oomd/CgroupContextTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -405,48 +405,44 @@ TEST_F(CgroupContextTest, DataLifeCycle) {
tempDir_,
{F::makeDir(
"system.slice",
{
F::makeFile(
"cgroup.stat",
{"nr_descendants 3\n"
"nr_dying_descendants 2\n"}),
F::makeFile(
"io.pressure",
{"some avg10=0.04 avg60=0.03 avg300=0.02 total=12346\n"
"full avg10=0.04 avg60=0.03 avg300=0.02 total=23457\n"}),
F::makeFile(
"io.stat",
{"1:10"
" rbytes=1111112 wbytes=2222223 rios=34 wios=45"
" dbytes=5555555556 dios=7\n"
"1:11"
" rbytes=2222223 wbytes=3333334 rios=45 wios=56"
" dbytes=6666666667 dios=8\n"}),
F::makeFile("memory.current", {"1122334456\n"}),
F::makeFile("memory.high", {"2233445567\n"}),
F::makeFile("memory.high.tmp", {"max 0\n"}),
F::makeFile("memory.low", {"11223345\n"}),
F::makeFile("memory.min", {"112234\n"}),
F::makeFile("memory.max", {"3344556678\n"}),
F::makeFile(
"memory.pressure",
{"some avg10=0.31 avg60=0.21 avg300=0.11 total=1234568\n"
"full avg10=0.31 avg60=0.21 avg300=0.11 total=2345679\n"}),
F::makeFile(
"memory.stat",
{"anon 123456790\n"
"file 12345679\n"
"shmem 1234568\n"
"pgscan 5678901234\n"}),
F::makeFile("memory.swap.current", {"1235\n"}),
F::makeFile("memory.swap.max", {"2048\n"}),
F::makeDir("service1.service", {}),
F::makeDir("service2.service", {}),
F::makeDir("service3.service", {}),
// For some reason newly added dir does not show up in readdir
// Remove it for now
// F::makeDir("service4.service", {}),
})}));
{F::makeFile(
"cgroup.stat",
{"nr_descendants 3\n"
"nr_dying_descendants 2\n"}),
F::makeFile(
"io.pressure",
{"some avg10=0.04 avg60=0.03 avg300=0.02 total=12346\n"
"full avg10=0.04 avg60=0.03 avg300=0.02 total=23457\n"}),
F::makeFile(
"io.stat",
{"1:10"
" rbytes=1111112 wbytes=2222223 rios=34 wios=45"
" dbytes=5555555556 dios=7\n"
"1:11"
" rbytes=2222223 wbytes=3333334 rios=45 wios=56"
" dbytes=6666666667 dios=8\n"}),
F::makeFile("memory.current", {"1122334456\n"}),
F::makeFile("memory.high", {"2233445567\n"}),
F::makeFile("memory.high.tmp", {"max 0\n"}),
F::makeFile("memory.low", {"11223345\n"}),
F::makeFile("memory.min", {"112234\n"}),
F::makeFile("memory.max", {"3344556678\n"}),
F::makeFile(
"memory.pressure",
{"some avg10=0.31 avg60=0.21 avg300=0.11 total=1234568\n"
"full avg10=0.31 avg60=0.21 avg300=0.11 total=2345679\n"}),
F::makeFile(
"memory.stat",
{"anon 123456790\n"
"file 12345679\n"
"shmem 1234568\n"
"pgscan 5678901234\n"}),
F::makeFile("memory.swap.current", {"1235\n"}),
F::makeFile("memory.swap.max", {"2048\n"}),
F::makeDir("service1.service", {}),
F::makeDir("service2.service", {}),
F::makeDir("service3.service", {}),
F::makeDir("service4.service", {})})}));
F::rmrChecked(tempDir_ + "/system.slice/service2.service");

// All values should stay the same
Expand Down Expand Up @@ -481,7 +477,9 @@ TEST_F(CgroupContextTest, DataLifeCycle) {

// Data are now updated
EXPECT_THAT(
*children, UnorderedElementsAre("service1.service", "service3.service"));
*children,
UnorderedElementsAre(
"service1.service", "service3.service", "service4.service"));
EXPECT_FLOAT_EQ(mem_pressure->sec_10, 0.31);
EXPECT_FLOAT_EQ(mem_pressure->sec_60, 0.21);
EXPECT_FLOAT_EQ(mem_pressure->sec_300, 0.11);
Expand Down
12 changes: 7 additions & 5 deletions src/oomd/util/Fs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,14 @@ SystemMaybe<Fs::DirEnts> Fs::readDirAt(const DirFd& dirfd, int flags) {
return SYSTEM_ERROR(e);
}

OOMD_SCOPE_EXIT {
// even though we dup()ed dirfd.fd(), the copied fd shares state with
// dirfd.fd(). For dirfd to be reusable again, we need to rewind it, which
// we can do through d.
::rewinddir(d);
// even though we dup()ed dirfd.fd(), the copied fd shares state with
// dirfd.fd(). For dirfd to be reusable again, we need to rewind it, which
// we can do through d.
// rewinddir causes DIR to refer to the current state of the directory.
// Therefore call it right before readdir to ensure seeing new entries.
::rewinddir(d);

OOMD_SCOPE_EXIT {
::closedir(d);
};

Expand Down

0 comments on commit b1018d8

Please sign in to comment.