sdevent:inode entry is not released after creating dump
For every dump generated an "anon_inode:[pidfd]" entry is
created for the associated process and the same is not
released.
When the number of inode entries reach the maximum count
dump request will fail and the dump service takes a reset
with a core dump.
When a dump is requested a child process is spawned to
perform dump collection, parent process waits on the
child process completion using sd-event-add-child
systemd call.
sd-event-add-child adds an inode entry which needs to
be released during the callback method called after
child process exit using unref but the same is not done.
Now switching to sdeventplus::source::Child wrapper class for
sd-event-add-child which takes care of releasing the inode fd.
Tested:
'''
Without the fix for the 3 dumps created noticed 3 anon_inode:[pidfd]
are added but not cleared
root@rain135bmc:/proc/2044/fd# ls -la
dr-x------ 2 root root 0 Jun 27 08:38 .
dr-xr-xr-x 8 root root 0 Jun 27 08:38 ..
lr-x------ 1 root root 64 Jun 27 08:39 0 -> /dev/null
lrwx------ 1 root root 64 Jun 27 08:39 1 -> socket:[21586]
lrwx------ 1 root root 64 Jun 27 08:39 10 ->anon_inode:[timerfd]
lrwx------ 1 root root 64 Jun 27 08:47 11 ->anon_inode:[pidfd]
lrwx------ 1 root root 64 Jun 27 08:47 12 ->anon_inode:[pidfd]
lrwx------ 1 root root 64 Jun 27 08:47 13 ->anon_inode:[pidfd]
After the fix noticed all the anon_inode:[pidfd] are cleared
dr-x------ 2 root root 0 Jun 27 08:38 .
dr-xr-xr-x 8 root root 0 Jun 27 08:38 ..
lr-x------ 1 root root 64 Jun 27 08:39 0 -> /dev/null
lrwx------ 1 root root 64 Jun 27 08:39 1 -> socket:[21586]
lrwx------ 1 root root 64 Jun 27 08:39 10 ->anon_inode:[timerfd]
'''
Signed-off-by: Marri Devender Rao <devenrao@in.ibm.com>
Change-Id: I9a2fec0a2287bfe978b811671c1cfeb1356e59d2
diff --git a/dump_manager_bmc.cpp b/dump_manager_bmc.cpp
index e06c1f6..f95683d 100644
--- a/dump_manager_bmc.cpp
+++ b/dump_manager_bmc.cpp
@@ -13,6 +13,8 @@
#include <phosphor-logging/elog-errors.hpp>
#include <phosphor-logging/elog.hpp>
+#include <sdeventplus/exception.hpp>
+#include <sdeventplus/source/base.hpp>
#include <cmath>
#include <ctime>
@@ -124,20 +126,31 @@
}
else if (pid > 0)
{
- // local variable goes out of scope using pointer, callback method
- // need to dellocate the pointer
- Type* typePtr = new Type();
- *typePtr = type;
- int rc = sd_event_add_child(eventLoop.get(), nullptr, pid,
- WEXITED | WSTOPPED, callback,
- reinterpret_cast<void*>(typePtr));
- if (0 > rc)
+ Child::Callback callback = [this, type, pid](Child&, const siginfo_t*) {
+ if (type == Type::UserRequested)
+ {
+ log<level::INFO>(
+ "User initiated dump completed, resetting flag");
+ Manager::fUserDumpInProgress = false;
+ }
+ this->childPtrMap.erase(pid);
+ };
+ try
+ {
+ childPtrMap.emplace(pid,
+ std::make_unique<Child>(eventLoop.get(), pid,
+ WEXITED | WSTOPPED,
+ std::move(callback)));
+ }
+ catch (const sdeventplus::SdEventError& ex)
{
// Failed to add to event loop
- log<level::ERR>(fmt::format("Error occurred during the "
- "sd_event_add_child call, rc({})",
- rc)
- .c_str());
+ log<level::ERR>(
+ fmt::format(
+ "Error occurred during the sdeventplus::source::Child "
+ "creation ex({})",
+ ex.what())
+ .c_str());
elog<InternalFailure>();
}
}
diff --git a/dump_manager_bmc.hpp b/dump_manager_bmc.hpp
index 9297f6e..6dbb7ad 100644
--- a/dump_manager_bmc.hpp
+++ b/dump_manager_bmc.hpp
@@ -5,9 +5,12 @@
#include "watch.hpp"
#include "xyz/openbmc_project/Dump/Internal/Create/server.hpp"
+#include <sdeventplus/source/child.hpp>
#include <xyz/openbmc_project/Dump/Create/server.hpp>
#include <filesystem>
+#include <map>
+
namespace phosphor
{
namespace dump
@@ -30,7 +33,7 @@
sdbusplus::xyz::openbmc_project::Dump::Internal::server::Create::Type;
using Watch = phosphor::dump::inotify::Watch;
-
+using ::sdeventplus::source::Child;
// Type to dreport type string map
static const std::map<Type, std::string> TypeMap = {
{Type::ApplicationCored, "core"},
@@ -110,24 +113,6 @@
*/
uint32_t captureDump(Type type, const std::vector<std::string>& fullPaths);
- /** @brief sd_event_add_child callback
- *
- * @param[in] s - event source
- * @param[in] si - signal info
- * @param[in] userdata - pointer to Watch object
- *
- * @returns 0 on success, -1 on fail
- */
- static int callback(sd_event_source*, const siginfo_t*, void* type)
- {
- Type* ptr = reinterpret_cast<Type*>(type);
- if (*ptr == Type::UserRequested)
- {
- fUserDumpInProgress = false;
- }
- delete ptr;
- return 0;
- }
/** @brief Remove specified watch object pointer from the
* watch map and associated entry from the map.
* @param[in] path - unique identifier of the map
@@ -157,6 +142,9 @@
* [path:watch object]
*/
std::map<std::filesystem::path, std::unique_ptr<Watch>> childWatchMap;
+
+ /** @brief map of SDEventPlus child pointer added to event loop */
+ std::map<pid_t, std::unique_ptr<Child>> childPtrMap;
};
} // namespace bmc
diff --git a/meson.build b/meson.build
index 3566a89..75b57bf 100644
--- a/meson.build
+++ b/meson.build
@@ -27,6 +27,7 @@
sdbusplus_dep = dependency('sdbusplus')
sdbusplusplus_prog = find_program('sdbus++')
sdbuspp_gen_meson_prog = find_program('sdbus++-gen-meson')
+sdeventplus_dep = dependency('sdeventplus')
phosphor_dbus_interfaces_dep = dependency('phosphor-dbus-interfaces')
phosphor_logging_dep = dependency('phosphor-logging')
@@ -177,6 +178,7 @@
phosphor_dump_manager_dependency = [
phosphor_dbus_interfaces_dep,
sdbusplus_dep,
+ sdeventplus_dep,
phosphor_logging_dep,
fmt_dep,
cereal_dep,
diff --git a/subprojects/sdeventplus.wrap b/subprojects/sdeventplus.wrap
new file mode 100644
index 0000000..f871ac0
--- /dev/null
+++ b/subprojects/sdeventplus.wrap
@@ -0,0 +1,6 @@
+[wrap-git]
+url = https://github.com/openbmc/sdeventplus.git
+revision = HEAD
+
+[provide]
+sdeventplus = sdeventplus_dep