summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBrad Bishop <bradleyb@fuzziesquirrel.com>2018-11-19 15:29:26 -0500
committerAdriana Kobylak <anoo@us.ibm.com>2018-12-03 16:09:02 -0600
commit02516d3ff949271de661666353aa87ee06d74cce (patch)
tree9d4d720ffd28be5c04ff23c7682aa1dbbac6f6f0
parent3a19e62a59f317e5fae70054f6d8dafe985d8105 (diff)
downloadphosphor-bmc-code-mgmt-02516d3ff949271de661666353aa87ee06d74cce.zip
phosphor-bmc-code-mgmt-02516d3ff949271de661666353aa87ee06d74cce.tar.gz
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.cpp84
-rw-r--r--sync_watch.hpp3
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;
OpenPOWER on IntegriCloud