diff options
author | Brad Bishop <bradleyb@fuzziesquirrel.com> | 2018-11-19 15:29:26 -0500 |
---|---|---|
committer | Adriana Kobylak <anoo@us.ibm.com> | 2018-12-03 16:09:02 -0600 |
commit | 02516d3ff949271de661666353aa87ee06d74cce (patch) | |
tree | 9d4d720ffd28be5c04ff23c7682aa1dbbac6f6f0 | |
parent | 3a19e62a59f317e5fae70054f6d8dafe985d8105 (diff) | |
download | phosphor-bmc-code-mgmt-02516d3ff949271de661666353aa87ee06d74cce.tar.gz phosphor-bmc-code-mgmt-02516d3ff949271de661666353aa87ee06d74cce.zip |
sync/watch: reduce memory usage
Currently we get a different inotify handle for every file in the
synclist, and register all of them with sd_event. A single fd will
work and saves memory in libsystemd and the kernel, so do that instead.
This in turn allows the fileMap structure to be simplified down to a
basic map of watch descriptor / filename pairs.
Remove redundant calls to both close and inotify_rm_watch - let the
kernel do the work for us. From the inotify man page:
When all file descriptors referring to an inotify instance have been
closed (using close(2)), the underlying object and its resources are
freed for reuse by the kernel; all associated watches are auto‐
matically freed.
Change-Id: Ia63cb667cdf41c171276a0351d95347a54578f2f
Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
-rw-r--r-- | sync_watch.cpp | 84 | ||||
-rw-r--r-- | sync_watch.hpp | 3 |
2 files changed, 39 insertions, 48 deletions
diff --git a/sync_watch.cpp b/sync_watch.cpp index 88ccd89..bb2a860 100644 --- a/sync_watch.cpp +++ b/sync_watch.cpp @@ -21,41 +21,38 @@ namespace fs = std::experimental::filesystem; void SyncWatch::addInotifyWatch(const fs::path& path) { - auto fd = inotify_init1(IN_NONBLOCK); - if (-1 == fd) + auto wd = + inotify_add_watch(inotifyFd, path.c_str(), IN_CLOSE_WRITE | IN_DELETE); + if (-1 == wd) { - log<level::ERR>("inotify_init1 failed", entry("ERRNO=%d", errno), + log<level::ERR>("inotify_add_watch failed", entry("ERRNO=%d", errno), entry("FILENAME=%s", path.c_str())); return; } - auto wd = inotify_add_watch(fd, path.c_str(), IN_CLOSE_WRITE | IN_DELETE); - if (-1 == wd) + fileMap[wd] = fs::path(path); +} + +SyncWatch::SyncWatch(sd_event& loop, + std::function<int(int, fs::path&)> syncCallback) : + inotifyFd(-1), + syncCallback(syncCallback), loop(loop) +{ + auto fd = inotify_init1(IN_NONBLOCK); + if (-1 == fd) { - log<level::ERR>("inotify_add_watch failed", entry("ERRNO=%d", errno), - entry("FILENAME=%s", path.c_str())); - close(fd); + log<level::ERR>("inotify_init1 failed", entry("ERRNO=%d", errno)); return; } + inotifyFd = fd; auto rc = sd_event_add_io(&loop, nullptr, fd, EPOLLIN, callback, this); if (0 > rc) { - log<level::ERR>("failed to add to event loop", entry("RC=%d", rc), - entry("FILENAME=%s", path.c_str())); - inotify_rm_watch(fd, wd); - close(fd); + log<level::ERR>("failed to add to event loop", entry("RC=%d", rc)); return; } - fileMap[fd].insert(std::make_pair(wd, fs::path(path))); -} - -SyncWatch::SyncWatch(sd_event& loop, - std::function<int(int, fs::path&)> syncCallback) : - syncCallback(syncCallback), - loop(loop) -{ auto syncfile = fs::path(SYNC_LIST_DIR_PATH) / SYNC_LIST_FILE_NAME; if (fs::exists(syncfile)) { @@ -70,13 +67,9 @@ SyncWatch::SyncWatch(sd_event& loop, SyncWatch::~SyncWatch() { - for (const auto& fd : fileMap) + if (inotifyFd != -1) { - for (const auto& wd : fd.second) - { - inotify_rm_watch(fd.first, wd.first); - } - close(fd.first); + close(inotifyFd); } } @@ -102,32 +95,29 @@ int SyncWatch::callback(sd_event_source* s, int fd, uint32_t revents, { auto event = reinterpret_cast<inotify_event*>(&buffer[offset]); - // fileMap<fd, std::map<wd, path>> - auto it1 = syncWatch->fileMap.find(fd); - if (it1 != syncWatch->fileMap.end()) + // Watch was removed, re-add it if file still exists. + if (event->mask & IN_IGNORED) { - auto it2 = it1->second.begin(); - - // Watch was removed, re-add it if file still exists. - if (event->mask & IN_IGNORED) + if (fs::exists(syncWatch->fileMap[event->wd])) { - if (fs::exists(it2->second)) - { - syncWatch->addInotifyWatch(it2->second); - } - else - { - log<level::INFO>("The inotify watch was removed", - entry("FILENAME=%s", it2->second.c_str())); - } - return 0; + syncWatch->addInotifyWatch(syncWatch->fileMap[event->wd]); } - - auto rc = syncWatch->syncCallback(event->mask, it2->second); - if (rc) + else { - return rc; + log<level::INFO>( + "The inotify watch was removed", + entry("FILENAME=%s", + (syncWatch->fileMap[event->wd]).c_str())); } + return 0; + } + + // fileMap<wd, path> + auto rc = + syncWatch->syncCallback(event->mask, syncWatch->fileMap[event->wd]); + if (rc) + { + return rc; } offset += offsetof(inotify_event, name) + event->len; diff --git a/sync_watch.hpp b/sync_watch.hpp index 3ad5afd..126c50c 100644 --- a/sync_watch.hpp +++ b/sync_watch.hpp @@ -63,7 +63,8 @@ class SyncWatch /** @brief Map of file descriptors, watch descriptors, and file paths */ using fd = int; using wd = int; - std::map<fd, std::map<wd, fs::path>> fileMap; + fd inotifyFd; + std::map<wd, fs::path> fileMap; /** @brief The callback function for processing the inotify event */ std::function<int(int, fs::path&)> syncCallback; |