zramctl: wait for device being initialized and unlocked by udevd

systemd-udevd takes a lock during processing the uevent for a block
device. The kernel refuses 'reset' attribute for zram device is written
when the device node is opened. Hence, during systemd-udevd processes a
uevent for zram device, we cannot write 'reset' attribute.
Let's wait for the device being initialized and unlocked by udevd.

Note, there still exists a race window, as we need to release the lock
before writing 'reset' attribute. But, the situation should be better
now.

diff --git a/configure.ac b/configure.ac
index 39bf2c75781e8e0ebd6393109f92d6ff9e3fad07..43fbce31701b2419032e0c08d7365e1af180d236 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -2718,6 +2718,8 @@ AS_IF([test "x$with_systemd" != xno], [
        AC_DEFINE([HAVE_LIBSYSTEMD], [1], [Define if libsystemd is available])
        AC_DEFINE([USE_SYSTEMD], [1], [Define if systemd support is wanted ])
        AC_CHECK_DECLS([sd_session_get_username], [], [], [#include <systemd/sd-login.h>])
+       AC_CHECK_DECLS([sd_device_new_from_syspath], [], [], [#include <systemd/sd-device.h>])
+       AC_CHECK_DECLS([sd_device_open], [], [], [#include <systemd/sd-device.h>])
   )
 ])
 AM_CONDITIONAL([HAVE_SYSTEMD], [test "x$have_systemd" = xyes])

diff --git a/meson.build b/meson.build
index 3852a1afd499681afa578d3dfd581f81d5ab8939..db5f0cddba8dfe1debc9a78c10735a4fb4ac37e9 100644 (file)
--- a/meson.build
+++ b/meson.build
@@ -373,6 +373,19 @@ lib_systemd = dependency(
   required : get_option('systemd'))
 conf.set('HAVE_LIBSYSTEMD', lib_systemd.found() ? 1 : false)
 conf.set('USE_SYSTEMD', lib_systemd.found() ? 1 : false)
+summary('libsystemd', lib_systemd.found() ? 'enabled' : 'disabled', section : 'components')
+
+# Since systemd-240, which provides basic functions in sd-device.
+have = cc.has_function(
+  'sd_device_new_from_syspath',
+  dependencies : lib_systemd)
+conf.set('HAVE_DECL_SD_DEVICE_NEW_FROM_SYSPATH', have ? 1 : false)
+
+# Since systemd-251, which also newly provides sd_device_new_from_devname().
+have = cc.has_function(
+  'sd_device_open',
+  dependencies : lib_systemd)
+conf.set('HAVE_DECL_SD_DEVICE_OPEN', have ? 1 : false)
 
 have = cc.has_function(
   'sd_session_get_username',
@@ -1858,6 +1871,8 @@ exe = executable(
   include_directories : includes,
   link_with : [lib_common,
                lib_smartcols],
+  # requires systemd v240 or newer
+  dependencies : [conf.get('HAVE_DECL_SD_DEVICE_NEW_FROM_SYSPATH') == 1 ? lib_systemd : [] ],
   install_dir : sbindir,
   install : opt,
   build_by_default : opt)

diff --git a/sys-utils/Makemodule.am b/sys-utils/Makemodule.am
index ec3f27f06dbfac6f74aad4c875ec3bb14c0c1dd7..0850ff6e21267068fca92ab18d6838bcb7cd1cea 100644 (file)
--- a/sys-utils/Makemodule.am
+++ b/sys-utils/Makemodule.am
@@ -332,7 +332,11 @@ zramctl_SOURCES = sys-utils/zramctl.c \
                  lib/ismounted.c
 zramctl_LDADD = $(LDADD) libcommon.la libsmartcols.la
 zramctl_CFLAGS = $(AM_CFLAGS) -I$(ul_libsmartcols_incdir)
+if HAVE_SYSTEMD
+zramctl_LDADD += $(SYSTEMD_LIBS)
+zramctl_CFLAGS += $(SYSTEMD_CFLAGS)
 endif
+endif # BUILD_ZRAMCTL
 
 
 if BUILD_PRLIMIT
@@ -593,4 +597,4 @@ if HAVE_LINUX_LANDLOCK_H
 setpriv_SOURCES += sys-utils/setpriv-landlock.c
 endif
 setpriv_LDADD = $(LDADD) -lcap-ng libcommon.la
-endif
\ No newline at end of file
+endif

diff --git a/sys-utils/zramctl.c b/sys-utils/zramctl.c
index dfe752bc4eb5b44da404e9b1c8be841741517e97..72daaed03dbdcbaf7f69aaf9746147617ee4ebb2 100644 (file)
--- a/sys-utils/zramctl.c
+++ b/sys-utils/zramctl.c
@@ -23,11 +23,16 @@
 #include <string.h>
 #include <stdarg.h>
 #include <assert.h>
+#include <sys/file.h>
 #include <sys/types.h>
 #include <dirent.h>
 
 #include <libsmartcols.h>
 
+#if HAVE_DECL_SD_DEVICE_NEW_FROM_SYSPATH
+#include <systemd/sd-device.h>
+#endif
+
 #include "c.h"
 #include "nls.h"
 #include "closestream.h"
@@ -42,6 +47,8 @@
 
 /*#define CONFIG_ZRAM_DEBUG*/
 
+#define _cleanup_(x) __attribute__((__cleanup__(x)))
+
 #ifdef CONFIG_ZRAM_DEBUG
 # define DBG(x)         do { fputs("zram: ", stderr); x; fputc('\n', stderr); } while(0)
 #else
@@ -113,9 +120,14 @@ static const char *const mm_stat_names[] = {
 
 struct zram {
        char    devname[32];
+       int     lock_fd;
        struct  path_cxt *sysfs;        /* device specific sysfs directory */
        char    **mm_stat;
 
+#if HAVE_DECL_SD_DEVICE_NEW_FROM_SYSPATH
+       sd_device       *device;
+#endif
+
        unsigned int mm_stat_probed : 1,
                     control_probed : 1,
                     has_control : 1;   /* has /sys/class/zram-control/ */
@@ -150,6 +162,149 @@ static int column_name_to_id(const char *name, size_t namesz)
        return -1;
 }
 
+#if HAVE_DECL_SD_DEVICE_NEW_FROM_SYSPATH
+static int monitor_callback(sd_device_monitor *m, sd_device *device, void *userdata)
+{
+       struct zram *z = userdata;
+       sd_device_action_t a;
+       const char *s;
+
+       assert(z);
+       assert(device);
+
+       if (sd_device_get_action(device, &a) < 0)
+               return 0;
+
+       if (a == SD_DEVICE_REMOVE)
+               return 0;
+
+       if (sd_device_get_devname(device, &s) < 0)
+               return 0;
+
+       if (strcmp(s, z->devname) != 0)
+               return 0;
+
+       if (sd_device_get_is_initialized(device) <= 0)
+               return 0;
+
+       assert(!z->device);
+       z->device = sd_device_ref(device);
+
+       return sd_event_exit(sd_device_monitor_get_event(m), 0);
+}
+#endif
+
+static int zram_wait_initialized(struct zram *z)
+{
+#if HAVE_DECL_SD_DEVICE_NEW_FROM_SYSPATH
+       _cleanup_(sd_device_unrefp) sd_device *dev = NULL;
+       _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *m = NULL;
+       sd_event *event;
+       int r;
+
+       assert(z);
+
+       z->device = sd_device_unref(z->device);
+
+       /* prepare device monitor. */
+       r = sd_device_monitor_new(&m);
+       if (r < 0)
+               return r;
+
+       r = sd_device_monitor_filter_add_match_subsystem_devtype(m, "block", "disk");
+       if (r < 0)
+               return r;
+
+       r = sd_device_monitor_start(m, monitor_callback, z);
+       if (r < 0)
+               return r;
+
+       event = sd_device_monitor_get_event(m);
+
+       /* Wait up to 3 seconds. */
+       r = sd_event_add_time_relative(event, NULL, CLOCK_BOOTTIME, 3 * 1000 * 1000, 0, NULL, (void*) (intptr_t) (-ETIMEDOUT));
+       if (r < 0)
+               return r;
+
+       /* Check if the device is already initialized. */
+#if HAVE_DECL_SD_DEVICE_OPEN
+       /* sd_device_new_from_devname() and sd_device_open() are both since systemd-251 */
+       r = sd_device_new_from_devname(&dev, z->devname);
+#else
+       {
+               char syspath[PATH_MAX];
+               const char *slash;
+
+               slash = strrchr(z->devname, '/');
+               if (!slash)
+                       return -EINVAL;
+               snprintf(syspath, sizeof(syspath), "/sys/class/block/%s", slash+1);
+
+               r = sd_device_new_from_syspath(&dev, syspath);
+       }
+#endif
+       if (r < 0)
+               return r;
+
+       r = sd_device_get_is_initialized(dev);
+       if (r < 0)
+               return r;
+       if (r > 0) {
+               /* Already initialized. It is not necessary to wait with the monitor. */
+               z->device = dev;
+               dev = NULL;
+               return 0;
+       }
+
+       return sd_event_loop(event);
+#else
+       assert(z);
+       return 0;
+#endif
+}
+
+static int zram_lock(struct zram *z, int operation)
+{
+       int fd, r;
+
+       assert(z);
+       assert((operation & ~LOCK_NB) == LOCK_SH ||
+              (operation & ~LOCK_NB) == LOCK_EX);
+
+       if (z->lock_fd >= 0)
+               return 0;
+
+#if HAVE_DECL_SD_DEVICE_OPEN
+       if (z->device) {
+               fd = sd_device_open(z->device, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
+               if (fd < 0)
+                       return fd;
+       } else {
+#endif
+               fd = open(z->devname, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
+               if (fd < 0)
+                       return -errno;
+#if HAVE_DECL_SD_DEVICE_OPEN
+       }
+#endif
+
+       if (flock(fd, operation) < 0) {
+               r = -errno;
+               close(fd);
+               return r;
+       }
+
+       z->lock_fd = fd;
+       return 0;
+}
+
+static void zram_unlock(struct zram *z) {
+       if (z && z->lock_fd >= 0) {
+               close(z->lock_fd);
+               z->lock_fd = -EBADF;
+       }
+}
+
 static void zram_reset_stat(struct zram *z)
 {
        if (z) {
@@ -192,6 +347,8 @@ static struct zram *new_zram(const char *devname)
        DBG(fprintf(stderr, "new: %p", z));
        if (devname)
                zram_set_devname(z, devname, 0);
+
+       z->lock_fd = -EBADF;
        return z;
 }
 
@@ -202,6 +359,12 @@ static void free_zram(struct zram *z)
        DBG(fprintf(stderr, "free: %p", z));
        ul_unref_path(z->sysfs);
        zram_reset_stat(z);
+       zram_unlock(z);
+
+#if HAVE_DECL_SD_DEVICE_NEW_FROM_SYSPATH
+       sd_device_unref(z->device);
+#endif
+
        free(z);
 }
 
@@ -760,8 +923,10 @@ int main(int argc, char **argv)
                        errx(EXIT_FAILURE, _("no device specified"));
                while (optind < argc) {
                        zram = new_zram(argv[optind]);
-                       if (!zram_exist(zram)
-                           || zram_set_u64parm(zram, "reset", 1)) {
+                       if (!zram_exist(zram) ||
+                           zram_wait_initialized(zram) ||
+                           zram_lock(zram, LOCK_EX | LOCK_NB) ||
+                           (zram_unlock(zram), zram_set_u64parm(zram, "reset", 1))) {
                                warn(_("%s: failed to reset"), zram->devname);
                                rc = 1;
                        }
@@ -790,6 +955,19 @@ int main(int argc, char **argv)
                                err(EXIT_FAILURE, "%s", zram->devname);
                }
 
+               /* Wait for udevd initialized the device. */
+               if (zram_wait_initialized(zram))
+                       err(EXIT_FAILURE, _("%s: failed to wait for initialized"), zram->devname);
+
+               /* Even if the device has been initialized by udevd, the device may be still opened and
+                * locked by udevd. Let's wait for the lock taken by udevd is released. */
+               if (zram_lock(zram, LOCK_EX))
+                       err(EXIT_FAILURE, _("%s: failed to lock"), zram->devname);
+
+               /* Writting 'reset' attribute is refused by the kernel when the device node is opened.
+                * Hence, we cannot keep the lock, unfortunately. */
+               zram_unlock(zram);
+
                if (zram_set_u64parm(zram, "reset", 1))
                        err(EXIT_FAILURE, _("%s: failed to reset"), zram->devname);