Hi,
> That would limit paralellism to some degree.
>
> Retrys won't be needed very often; they would surely have a lower
> overhead. Less code too :-). It's not completely out of left field;
> both seqlocks and transactional memory use retries. There's no risk
> of livelock (i.e. infinite retries)...
>
> I'm lazy and don't want udev to run measurably slower. The only
> advantage I see of flock() is that it tells the reader "I'm a
> synchronization primitive, solving a concurrency problem here". I
> think we can get close enough by explaining the race in a comment next
> to the definition of create_path().
plus, it seems to be the simpler solution by far - which is why I went
with flock() in my suggested fix you find below. Untested, as usual, and
I don't really have a clue whether there are any unhealthy interactions
with the rest of udev in this solution. Also, you most likely will want
to change the path of the lock file ;-)
Florian
diff --git a/libudev/libudev-device-private.c b/libudev/libudev-device-private.c
index 68dc0a5..43cf152 100644
--- a/libudev/libudev-device-private.c
+++ b/libudev/libudev-device-private.c
@@ -86,6 +86,7 @@ file:
f = fopen(filename, "w");
if (f == NULL) {
err(udev, "unable to create db file '%s': %m\n", filename);
+ util_create_path_done();
return -1;
}
info(udev, "created db file for '%s' in '%s'\n", udev_device_get_devpath(udev_device), filename);
@@ -114,6 +115,7 @@ file:
}
fclose(f);
out:
+ util_create_path_done();
return 0;
}
diff --git a/libudev/libudev-queue-private.c b/libudev/libudev-queue-private.c
index 0427b65..f2f3905 100644
--- a/libudev/libudev-queue-private.c
+++ b/libudev/libudev-queue-private.c
@@ -417,6 +417,7 @@ static void update_failed(struct udev_queue_export *udev_queue_export,
udev_selinux_setfscreatecon(udev, filename, S_IFLNK);
symlink(udev_device_get_devpath(udev_device), filename);
udev_selinux_resetfscreatecon(udev);
+ util_create_path_done();
break;
case DEVICE_QUEUED:
diff --git a/libudev/libudev-util-private.c b/libudev/libudev-util-private.c
index c309945..9d205d4 100644
--- a/libudev/libudev-util-private.c
+++ b/libudev/libudev-util-private.c
@@ -25,6 +25,29 @@
#include "libudev.h"
#include "libudev-private.h"
+static int util_path_lock_fd = -1;
+
+static void util_path_unlock(void)
+{
+ if (util_path_lock_fd >= 0) {
+ close(util_create_path_lock_fd);
+ util_create_path_lock_fd = -1;
+ }
+}
+
+static int util_path_lock(void)
+{
+ if (util_path_lock_fd < 0) {
+ if ((util_path_lock_fd = open("/dev/udev_util_path_lock", O_WRONLY | O_CREAT)) < 0)
+ return -1;
+ if (flock(util_path_lock_fd, LOCK_EX)) {
+ util_path_unlock();
+ return -1;
+ }
+ }
+ return 0;
+}
+
int util_create_path(struct udev *udev, const char *path)
{
char p[UTIL_PATH_SIZE];
@@ -32,6 +55,8 @@ int util_create_path(struct udev *udev, const char *path)
struct stat stats;
int ret;
+ if (util_path_lock())
+ return -1;
util_strscpy(p, sizeof(p), path);
pos = strrchr(p, '/');
if (pos == NULL)
@@ -63,6 +88,11 @@ int util_create_path(struct udev *udev, const char *path)
return -1;
}
+void util_create_path_done(void)
+{
+ util_path_unlock();
+}
+
int util_delete_path(struct udev *udev, const char *path)
{
char p[UTIL_PATH_SIZE];
@@ -75,6 +105,8 @@ int util_delete_path(struct udev *udev, const char *path)
if (pos == p || pos == NULL)
return 0;
+ if (util_path_lock())
+ return 0;
while (1) {
*pos = '\0';
pos = strrchr(p, '/');
@@ -88,13 +120,13 @@ int util_delete_path(struct udev *udev, const char *path)
if (errno == ENOENT)
retval = 0;
if (retval) {
- if (errno == ENOTEMPTY)
- return 0;
- err(udev, "rmdir(%s) failed: %m\n", p);
+ if (errno != ENOTEMPTY)
+ err(udev, "rmdir(%s) failed: %m\n", p);
break;
}
dbg(udev, "removed '%s'\n", p);
}
+ util_path_unlock();
return 0;
}
diff --git a/udev/udev-node.c b/udev/udev-node.c
index d73bc6c..74de922 100644
--- a/udev/udev-node.c
+++ b/udev/udev-node.c
@@ -48,6 +48,7 @@ static int name_index(struct udev *udev, const char *devpath, const char *name,
dbg(udev, "creating index: '%s'\n", filename);
util_create_path(udev, filename);
fd = open(filename, O_WRONLY|O_TRUNC|O_CREAT, 0644);
+ util_create_path_done();
if (fd > 0)
close(fd);
} else {
@@ -356,6 +357,7 @@ static int update_link(struct udev_device *dev, const char *slink)
info(udev, "'%s' with target '%s' has the highest priority %i, create it\n", slink, target, priority);
util_create_path(udev, slink);
node_symlink(udev, target, slink);
+ util_create_path_done();
out:
return rc;
}
@@ -456,6 +458,7 @@ int udev_node_add(struct udev_device *dev, mode_t mode, uid_t uid, gid_t gid)
update_link(dev, udev_list_entry_get_name(list_entry));
}
exit:
+ util_create_path_done();
return err;
}
diff --git a/udev/udev-rules.c b/udev/udev-rules.c
index 54a54c3..f19ead8 100644
--- a/udev/udev-rules.c
+++ b/udev/udev-rules.c
@@ -1733,6 +1733,7 @@ struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names)
udev_selinux_setfscreatecon(udev, filename, S_IFDIR|0755);
mkdir(filename, 0755);
udev_selinux_resetfscreatecon(udev);
+ util_create_path_done();
}
udev_list_init(&sort_list);
add_matching_files(udev, &sort_list, filename, ".rules");
diff --git a/udev/udev-watch.c b/udev/udev-watch.c
index 5a49c96..3eb45a1 100644
--- a/udev/udev-watch.c
+++ b/udev/udev-watch.c
@@ -127,6 +127,7 @@ void udev_watch_begin(struct udev *udev, struct udev_device *dev)
util_create_path(udev, filename);
unlink(filename);
symlink(udev_device_get_devpath(dev), filename);
+ util_create_path_done();
udev_device_set_watch_handle(dev, wd);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
