diff mbox series

shared/fdset: add detailed debug logging to fdset_new_fill()

Message ID 20260303081244.3330595-1-git-patches@bmwtechworks.in
State Under Review
Headers show
Series shared/fdset: add detailed debug logging to fdset_new_fill() | expand

Commit Message

Ashish Kumar Mishra March 3, 2026, 8:12 a.m. UTC
From: AshishKumar Mishra <ashishkumar.mishra@bmwtechworks.in>

Currently, when fdset_new_fill() fails to open /proc/self/fd or
encounters an error while processing individual file descriptors
(such as fcntl or fstat failures), it returns a silent error code.

For debugging rarely reproducible failures it becomes difficult to
know the exact cause of failure
This commit updates the function to use log_debug_errno() for all
error paths and hence  provides better visibility into why FD collection
failed, including the path of the problematic FD (via fd_get_path)
and its inode type.

Upstream-Status: Backport [https://github.com/systemd/systemd/pull/40385]

Signed-off-by: AshishKumar Mishra <ashishkumar.mishra@bmwtechworks.in>
---
 ...-detailed-debug-logging-to-fdset_new.patch | 97 +++++++++++++++++++
 meta/recipes-core/systemd/systemd_258.1.bb    |  1 +
 2 files changed, 98 insertions(+)
 create mode 100644 meta/recipes-core/systemd/systemd/0018-shared-fdset-add-detailed-debug-logging-to-fdset_new.patch

Comments

AshishKumar Mishra March 9, 2026, 4:30 a.m. UTC | #1
Hi Team,

Any input on this patch
I have backported this patch which i have merged in systemd upstream code base
I could not see this patch in master-next branch of openembedded-core - OpenEmbedded Core layer ( https://git.openembedded.org/openembedded-core/log/?h=master-next )
Hence wanted to check if there is any feedback from team

Thanks .
Ashish
Mathieu Dubois-Briand March 9, 2026, 7:26 a.m. UTC | #2
On Mon Mar 9, 2026 at 5:30 AM CET, AshishKumar Mishra via lists.openembedded.org wrote:
> Hi Team,
>
> Any input on this patch
> I have backported this patch which i have merged in systemd upstream code base
> I could not see this patch in master-next branch of openembedded-core - OpenEmbedded Core layer ( https://git.openembedded.org/openembedded-core/log/?h=master-next )
> Hence wanted to check if there is any feedback from team
>
> Thanks .
> Ashish

Hi Ashish,

Yes, I had your patch aside for a few days, as a systemd upgrade to
259.1/259.3 was already sent and conflicting with your patch. This
upgrade is now merged.

Is your patch still needed with systemd 259.3? If that's the case, can
you please rebase on top of master?

Thanks,
Mathieu
AshishKumar Mishra March 10, 2026, 8:51 a.m. UTC | #3
Hi Mathiew ,

Thanks for sharing the details.
Have rebased the patch to latest version of systemd in yocto
https://lists.openembedded.org/g/openembedded-core/message/232763

Can you please consider this for merging on master branch

Thanks,
Ashish
diff mbox series

Patch

diff --git a/meta/recipes-core/systemd/systemd/0018-shared-fdset-add-detailed-debug-logging-to-fdset_new.patch b/meta/recipes-core/systemd/systemd/0018-shared-fdset-add-detailed-debug-logging-to-fdset_new.patch
new file mode 100644
index 0000000000..849f5cfda1
--- /dev/null
+++ b/meta/recipes-core/systemd/systemd/0018-shared-fdset-add-detailed-debug-logging-to-fdset_new.patch
@@ -0,0 +1,97 @@ 
+From 0565f9f27323a8f9e62d85f2add542af99cea06a Mon Sep 17 00:00:00 2001
+From: AshishKumar Mishra <ashishkumar.mishra@bmwtechworks.in>
+Date: Wed, 21 Jan 2026 14:13:29 +0530
+Subject: [PATCH] shared/fdset: add detailed debug logging to fdset_new_fill()
+
+Currently, when fdset_new_fill() fails to open /proc/self/fd or
+encounters an error while processing individual file descriptors
+(such as fcntl or fstat failures), it returns a silent error code.
+
+For debugging rarely reproducible failures it becomes difficult to
+know the exact cause of failure
+This commit updates the function to use log_debug_errno() for all
+error paths and hence  provides better visibility into why FD collection
+failed, including the path of the problematic FD (via fd_get_path)
+and its inode type.
+
+Upstream-Status: Backport [https://github.com/systemd/systemd/pull/40385]
+
+Signed-off-by: AshishKumar Mishra <ashishkumar.mishra@bmwtechworks.in>
+---
+ src/shared/fdset.c | 35 ++++++++++++++++++++++++++---------
+ 1 file changed, 26 insertions(+), 9 deletions(-)
+
+diff --git a/src/shared/fdset.c b/src/shared/fdset.c
+index 832e7fda60..f340f41b0e 100644
+--- a/src/shared/fdset.c
++++ b/src/shared/fdset.c
+@@ -8,6 +8,7 @@
+ #include "alloc-util.h"
+ #include "async.h"
+ #include "dirent-util.h"
++#include "errno-util.h"
+ #include "fd-util.h"
+ #include "fdset.h"
+ #include "log.h"
+@@ -179,9 +180,10 @@ int fdset_new_fill(
+         d = opendir("/proc/self/fd");
+         if (!d) {
+                 if (errno == ENOENT && proc_mounted() == 0)
+-                        return -ENOSYS;
++                        return log_debug_errno(SYNTHETIC_ERRNO(ENOSYS),
++                                               "Failed to open /proc/self/fd/, /proc/ is not mounted.");
+ 
+-                return -errno;
++                return log_debug_errno(errno, "Failed to open /proc/self/fd/: %m ");
+         }
+ 
+         s = fdset_new();
+@@ -210,9 +212,14 @@ int fdset_new_fill(
+                          * been passed in can be collected and fds which have been created locally can be
+                          * ignored, under the assumption that only the latter have O_CLOEXEC set. */
+ 
+-                        fl = fcntl(fd, F_GETFD);
+-                        if (fl < 0)
+-                                return -errno;
++                        fl = RET_NERRNO(fcntl(fd, F_GETFD));
++                        if (fl < 0) {
++                                _cleanup_free_ char *path = NULL;
++                                (void) fd_get_path(fd, &path);
++                                return log_debug_errno(fl,
++                                                       "Failed to get flag of fd=%d (%s): %m ",
++                                                       fd, strna(path));
++                        }
+ 
+                         if (FLAGS_SET(fl, FD_CLOEXEC) != !!filter_cloexec)
+                                 continue;
+@@ -221,13 +228,23 @@ int fdset_new_fill(
+                 /* We need to set CLOEXEC manually only if we're collecting non-CLOEXEC fds. */
+                 if (filter_cloexec <= 0) {
+                         r = fd_cloexec(fd, true);
+-                        if (r < 0)
+-                                return r;
++                        if (r < 0) {
++                                _cleanup_free_ char *path = NULL;
++                                (void) fd_get_path(fd, &path);
++                                return log_debug_errno(r,
++                                                       "Failed to set CLOEXEC flag fd=%d (%s): %m ",
++                                                       fd, strna(path));
++                        }
+                 }
+ 
+                 r = fdset_put(s, fd);
+-                if (r < 0)
+-                        return r;
++                if (r < 0) {
++                        _cleanup_free_ char *path = NULL;
++                        (void) fd_get_path(fd, &path);
++                        return log_debug_errno(r,
++                                               "Failed to put fd=%d (%s) into fdset: %m ",
++                                               fd, strna(path));
++                }
+         }
+ 
+         *ret = TAKE_PTR(s);
+-- 
+2.34.1
+
diff --git a/meta/recipes-core/systemd/systemd_258.1.bb b/meta/recipes-core/systemd/systemd_258.1.bb
index fdf4dc5bdc..0259bacb37 100644
--- a/meta/recipes-core/systemd/systemd_258.1.bb
+++ b/meta/recipes-core/systemd/systemd_258.1.bb
@@ -33,6 +33,7 @@  SRC_URI += " \
            file://0001-binfmt-Don-t-install-dependency-links-at-install-tim.patch \
            file://0002-implment-systemd-sysv-install-for-OE.patch \
            file://0003-Do-not-create-var-log-README.patch \
+           file://0018-shared-fdset-add-detailed-debug-logging-to-fdset_new.patch \
            "
 
 # patches needed by musl