From 7727b694db7b8e83b0f9991f197b78f35bd065e0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 10 Oct 2024 10:33:22 +0900 Subject: [PATCH] udev-node: remove lockfile and stack directory when not necessary if possible Replaces 09373c1a50297079e6b0447ea97af4e9a60f77fa. Let's remove stack directories and their lock files by workers if possible. Now, lock files must be created before creating stack directories, hence lock files are moved to /run/udev/links.lock/ , e.g., Before: /run/udev/links/disk\x2fby-diskseq\x2f1/.lock After: /run/udev/links.lock/disk\x2fby-diskseq\x2f1 Fixes ##34637. --- src/udev/udev-node.c | 73 ++++++++++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 26 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index a0bf0a96195..589a56e5025 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -339,7 +339,7 @@ toolong: } static int stack_directory_get_name(const char *slink, char **ret) { - _cleanup_free_ char *s = NULL, *dirname = NULL; + _cleanup_free_ char *s = NULL; char name_enc[NAME_MAX+1]; const char *name; int r; @@ -360,45 +360,59 @@ static int stack_directory_get_name(const char *slink, char **ret) { udev_node_escape_path(name, name_enc, sizeof(name_enc)); - dirname = path_join("/run/udev/links", name_enc); - if (!dirname) - return -ENOMEM; - - *ret = TAKE_PTR(dirname); - return 0; + return strdup_to(ret, name_enc); } -static int stack_directory_open(sd_device *dev, const char *slink, int *ret_dirfd, int *ret_lockfd) { - _cleanup_close_ int dirfd = -EBADF, lockfd = -EBADF; - _cleanup_free_ char *dirname = NULL; +static int stack_directory_open_and_lock( + sd_device *dev, + const char *slink, + char **ret_dirpath, + int *ret_dirfd, + LockFile *ret_lockfile) { + + _cleanup_(release_lock_file) LockFile lockfile = LOCK_FILE_INIT; + _cleanup_close_ int dirfd = -EBADF; + _cleanup_free_ char *name = NULL, *dirpath = NULL, *lockname = NULL; int r; assert(dev); assert(slink); + assert(ret_dirpath); assert(ret_dirfd); - assert(ret_lockfd); + assert(ret_lockfile); - r = stack_directory_get_name(slink, &dirname); + r = stack_directory_get_name(slink, &name); if (r < 0) return log_device_debug_errno(dev, r, "Failed to build stack directory name for '%s': %m", slink); - r = mkdir_parents(dirname, 0755); - if (r < 0) - return log_device_debug_errno(dev, r, "Failed to create stack directory '%s': %m", dirname); + FOREACH_STRING(s, "/run/udev/links/", "/run/udev/links.lock/") { + r = mkdir_p(s, 0755); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to create '%s': %m", s); + } - dirfd = open_mkdir(dirname, O_CLOEXEC | O_DIRECTORY | O_NOFOLLOW | O_RDONLY, 0755); - if (dirfd < 0) - return log_device_debug_errno(dev, dirfd, "Failed to open stack directory '%s': %m", dirname); + /* 1. Take a lock for the stack directory. */ + lockname = path_join("/run/udev/links.lock/", name); + if (!lockname) + return -ENOMEM; + + r = make_lock_file(lockname, LOCK_EX, &lockfile); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to create and lock '%s': %m", lockname); - lockfd = openat(dirfd, ".lock", O_CLOEXEC | O_NOFOLLOW | O_RDONLY | O_CREAT, 0600); - if (lockfd < 0) - return log_device_debug_errno(dev, errno, "Failed to create lock file for stack directory '%s': %m", dirname); + /* 2. Create and open the stack directory. Do not create the stack directory before taking a lock, + * otherwise the directory may be removed by another worker. */ + dirpath = path_join("/run/udev/links/", name); + if (!dirpath) + return -ENOMEM; - if (flock(lockfd, LOCK_EX) < 0) - return log_device_debug_errno(dev, errno, "Failed to place a lock on lock file for %s: %m", dirname); + dirfd = open_mkdir(dirpath, O_CLOEXEC | O_DIRECTORY | O_NOFOLLOW | O_RDONLY, 0755); + if (dirfd < 0) + return log_device_debug_errno(dev, dirfd, "Failed to open stack directory '%s': %m", dirpath); + *ret_dirpath = TAKE_PTR(dirpath); *ret_dirfd = TAKE_FD(dirfd); - *ret_lockfd = TAKE_FD(lockfd); + *ret_lockfile = TAKE_GENERIC(lockfile, LockFile, LOCK_FILE_INIT); return 0; } @@ -514,8 +528,15 @@ static int link_update_diskseq(sd_device *dev, const char *slink, bool add) { } static int link_update(sd_device *dev, const char *slink, bool add) { + /* On cleaning up, + * 1. close the stack directory, + * 2. remove the stack directory if it is empty, + * 3. then finally release the lock. + * Hence, the variables must be declared in the reverse order. */ + _cleanup_(release_lock_file) LockFile lockfile = LOCK_FILE_INIT; /* #3 */ + _cleanup_(rmdir_and_freep) char *dirpath = NULL; /* #2 */ + _cleanup_close_ int dirfd = -EBADF; /* #1 */ _cleanup_free_ char *current_id = NULL, *devnode = NULL; - _cleanup_close_ int dirfd = -EBADF, lockfd = -EBADF; int r, current_prio; assert(dev); @@ -525,7 +546,7 @@ static int link_update(sd_device *dev, const char *slink, bool add) { if (r != 0) return r; - r = stack_directory_open(dev, slink, &dirfd, &lockfd); + r = stack_directory_open_and_lock(dev, slink, &dirpath, &dirfd, &lockfile); if (r < 0) return r; -- 2.47.3