[PATCH] udevd race conditions and performance, assorted cleanups - take 2

here is a patch on top of your nice improvements.
I fixed the whitespace and it hopefully fixes the stupid timestamp bug in
udevd. Some stupid OS sets the hwclock to localtime and linux changes it
to UTC while starting. If any events are pending they may be delayed by
the users time distance from UTC :) So we use the uptime seconds now.

diff --git a/Makefile b/Makefile
index d0ca3a18d91f916ba01a82f09c2f5d3d33ecda03..652e712d775deca5f948bd42bb7a0e9adcd6fb03 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -220,6 +220,7 @@ HEADERS =   udev.h          \
 
 ifeq ($(strip $(USE_KLIBC)),true)
        OBJS += klibc_fixups.o
 
 ifeq ($(strip $(USE_KLIBC)),true)
        OBJS += klibc_fixups.o

+       KLIBC_FIXUP = klibc_fixups.o

 endif
 
 # header files automatically generated
 endif
 
 # header files automatically generated


@@ -266,8 +267,8 @@ $(INFO): $(INFO).o $(OBJS) $(HEADERS) $(LIBC)
        $(LD) $(LDFLAGS) -o $@ $(CRT0) udevinfo.o udev_lib.o udev_config.o udevdb.o $(SYSFS) $(TDB) $(LIB_OBJS) $(ARCH_LIB_OBJS)
        $(STRIPCMD) $@
 
        $(LD) $(LDFLAGS) -o $@ $(CRT0) udevinfo.o udev_lib.o udev_config.o udevdb.o $(SYSFS) $(TDB) $(LIB_OBJS) $(ARCH_LIB_OBJS)
        $(STRIPCMD) $@
 

-$(DAEMON): $(DAEMON).o udevd.h $(LIBC)
-       $(LD) $(LDFLAGS) -o $@ $(CRT0) udevd.o $(LIB_OBJS) $(ARCH_LIB_OBJS)
+$(DAEMON): $(DAEMON).o $(OBJS) udevd.h $(LIBC)
+       $(LD) $(LDFLAGS) -o $@ $(CRT0) udevd.o udev_lib.o $(KLIBC_FIXUP) $(LIB_OBJS) $(ARCH_LIB_OBJS)

        $(STRIPCMD) $@
 
 $(SENDER): $(SENDER).o udevd.h $(LIBC)
        $(STRIPCMD) $@
 
 $(SENDER): $(SENDER).o udevd.h $(LIBC)

diff --git a/klibc_fixups.c b/klibc_fixups.c
index eaa240c183fa59c6a34161d21c8ad30ea3d9f9d9..c4d44bbc490d3dd27752628fca1ac1fe9a7bdc58 100644 (file)
--- a/klibc_fixups.c
+++ b/klibc_fixups.c
@@ -30,12 +30,14 @@
 
 #include "udev.h"
 #include "klibc_fixups.h"
 
 #include "udev.h"
 #include "klibc_fixups.h"

+#include "udev_lib.h"

 #include "logging.h"
 
 #define PW_FILE                "/etc/passwd"
 #define GR_FILE                "/etc/group"
 #define UTMP_FILE      "/var/run/utmp"
 
 #include "logging.h"
 
 #define PW_FILE                "/etc/passwd"
 #define GR_FILE                "/etc/group"
 #define UTMP_FILE      "/var/run/utmp"
 

+_syscall1(int, sysinfo, struct sysinfo *, info);

 
 /* return the id of a passwd style line, selected by the users name */
 static unsigned long get_id_by_name(const char *uname, const char *dbfile)
 
 /* return the id of a passwd style line, selected by the users name */
 static unsigned long get_id_by_name(const char *uname, const char *dbfile)

diff --git a/klibc_fixups.h b/klibc_fixups.h
index f6c91cdd9444fb61e11055a19417c85fd191b409..6dfd5a5c17e631fbb6e49bddcb9328f00636fbee 100644 (file)
--- a/klibc_fixups.h
+++ b/klibc_fixups.h
@@ -3,6 +3,11 @@
 #ifndef KLIBC_FIXUPS_H
 #define KLIBC_FIXUPS_H
 
 #ifndef KLIBC_FIXUPS_H
 #define KLIBC_FIXUPS_H
 

+#include <linux/kernel.h>
+#include <linux/unistd.h>
+
+int sysinfo(struct sysinfo *info);
+

 struct passwd {
        char    *pw_name;       /* user name */
        char    *pw_passwd;     /* user password */
 struct passwd {
        char    *pw_name;       /* user name */
        char    *pw_passwd;     /* user password */

diff --git a/libsysfs/sysfs.h b/libsysfs/sysfs.h
index 0fdb38af193b073dc8ac4dcd0b281dbf4935aeb3..7ac6efc678e804fca81ec1072284c58d4670b8d5 100644 (file)
--- a/libsysfs/sysfs.h
+++ b/libsysfs/sysfs.h
@@ -36,8 +36,7 @@
 /* Debugging */
 #ifdef DEBUG
 #include "../logging.h" 
 /* Debugging */
 #ifdef DEBUG
 #include "../logging.h" 

-#define dprintf(format, arg...)                                                                \
-       dbg(format, ##arg)
+#define dprintf(format, arg...) dbg(format, ##arg)

 #else
 #define dprintf(format, arg...) do { } while (0)
 #endif
 #else
 #define dprintf(format, arg...) do { } while (0)
 #endif

diff --git a/udevd.c b/udevd.c
index 3b91a458da7f5bebcfca0547f3ec92a7df46bc84..301625fd927a58e7868cf1da31551a3d993f7e4a 100644 (file)
--- a/udevd.c
+++ b/udevd.c
@@ -28,12 +28,15 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>

-#include <time.h>
+#include <sys/time.h>

 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/un.h>

-#include <sys/time.h>

 #include <fcntl.h>
 #include <fcntl.h>

+#include "klibc_fixups.h"
+#ifndef __KLIBC__
+#include <sys/sysinfo.h>
+#endif

 
 #include "list.h"
 #include "udev.h"
 
 #include "list.h"
 #include "udev.h"


@@ -106,6 +109,7 @@ static void run_queue_delete(struct hotplug_msg *msg)
 static void msg_queue_insert(struct hotplug_msg *msg)
 {
        struct hotplug_msg *loop_msg;
 static void msg_queue_insert(struct hotplug_msg *msg)
 {
        struct hotplug_msg *loop_msg;

+       struct sysinfo info;

 
        /* sort message by sequence number into list. events
         * will tend to come in order, so scan the list backwards
 
        /* sort message by sequence number into list. events
         * will tend to come in order, so scan the list backwards


@@ -113,11 +117,13 @@ static void msg_queue_insert(struct hotplug_msg *msg)
        list_for_each_entry_reverse(loop_msg, &msg_list, list)
                if (loop_msg->seqnum < msg->seqnum)
                        break;
        list_for_each_entry_reverse(loop_msg, &msg_list, list)
                if (loop_msg->seqnum < msg->seqnum)
                        break;

-       list_add(&msg->list, &loop_msg->list);
-       dbg("queued message seq %d", msg->seqnum);

 
        /* store timestamp of queuing */
 
        /* store timestamp of queuing */

-       msg->queue_time = time(NULL);
+       sysinfo(&info);
+       msg->queue_time = info.uptime;
+
+       list_add(&msg->list, &loop_msg->list);
+       dbg("queued message seq %d", msg->seqnum);

 
        /* run msg queue manager */
        run_msg_q = 1;
 
        /* run msg queue manager */
        run_msg_q = 1;


@@ -203,7 +209,8 @@ static void msg_queue_manager()
 {
        struct hotplug_msg *loop_msg;
        struct hotplug_msg *tmp_msg;
 {
        struct hotplug_msg *loop_msg;
        struct hotplug_msg *tmp_msg;

-       time_t msg_age = 0;
+       struct sysinfo info;
+       long msg_age = 0;

 
        dbg("msg queue manager, next expected is %d", expected_seqnum);
 recheck:
 
        dbg("msg queue manager, next expected is %d", expected_seqnum);
 recheck:


@@ -215,7 +222,9 @@ recheck:
                }
 
                /* move event with expired timeout to the exec list */
                }
 
                /* move event with expired timeout to the exec list */

-               msg_age = time(NULL) - loop_msg->queue_time;
+               sysinfo(&info);
+               msg_age = info.uptime - loop_msg->queue_time;
+               dbg("seq %d is %li seconds old", loop_msg->seqnum, msg_age);

                if (msg_age > EVENT_TIMEOUT_SEC-1) {
                        msg_move_exec(loop_msg);
                        goto recheck;
                if (msg_age > EVENT_TIMEOUT_SEC-1) {
                        msg_move_exec(loop_msg);
                        goto recheck;


@@ -226,11 +235,10 @@ recheck:
 
        msg_dump_queue();
 
 
        msg_dump_queue();
 

+       /* set timeout for remaining queued events */

        if (list_empty(&msg_list) == 0) {
        if (list_empty(&msg_list) == 0) {

-               /* set timeout for remaining queued events */

                struct itimerval itv = {{0, 0}, {EVENT_TIMEOUT_SEC - msg_age, 0}};
                struct itimerval itv = {{0, 0}, {EVENT_TIMEOUT_SEC - msg_age, 0}};

-               dbg("next event expires in %li seconds",
-                   EVENT_TIMEOUT_SEC - msg_age);
+               dbg("next event expires in %li seconds", EVENT_TIMEOUT_SEC - msg_age);

                setitimer(ITIMER_REAL, &itv, 0);
        }
 }
                setitimer(ITIMER_REAL, &itv, 0);
        }
 }


@@ -399,25 +407,25 @@ int main(int argc, char *argv[])
                dbg("need to be root, exit");
                exit(1);
        }
                dbg("need to be root, exit");
                exit(1);
        }

-       
+

        /* setup signal handler pipe */
        /* setup signal handler pipe */

-   retval = pipe(pipefds);
-   if (retval < 0) {
-      dbg("error getting pipes: %s", strerror(errno));
-      exit(1);
-   }
-       
-   retval = fcntl(pipefds[0], F_SETFL, O_NONBLOCK);
-   if (retval < 0) {
-      dbg("fcntl on read pipe: %s", strerror(errno));
-      exit(1);
-   }
-   
-   retval = fcntl(pipefds[1], F_SETFL, O_NONBLOCK);
-   if (retval < 0) {
-      dbg("fcntl on write pipe: %s", strerror(errno));
-      exit(1);
-   }
+       retval = pipe(pipefds);
+       if (retval < 0) {
+               dbg("error getting pipes: %s", strerror(errno));
+               exit(1);
+       }
+
+       retval = fcntl(pipefds[0], F_SETFL, O_NONBLOCK);
+               if (retval < 0) {
+               dbg("error fcntl on read pipe: %s", strerror(errno));
+               exit(1);
+       }
+
+       retval = fcntl(pipefds[1], F_SETFL, O_NONBLOCK);
+       if (retval < 0) {
+               dbg("error fcntl on write pipe: %s", strerror(errno));
+               exit(1);
+       }

 
        /* set signal handlers */
        act.sa_handler = sig_handler;
 
        /* set signal handlers */
        act.sa_handler = sig_handler;


@@ -450,43 +458,42 @@ int main(int argc, char *argv[])
        /* enable receiving of the sender credentials */
        setsockopt(ssock, SOL_SOCKET, SO_PASSCRED, &on, sizeof(on));
 
        /* enable receiving of the sender credentials */
        setsockopt(ssock, SOL_SOCKET, SO_PASSCRED, &on, sizeof(on));
 

-   FD_ZERO(&readfds);
-   FD_SET(ssock, &readfds);
-   FD_SET(pipefds[0], &readfds);
+       FD_ZERO(&readfds);
+       FD_SET(ssock, &readfds);
+       FD_SET(pipefds[0], &readfds);

        maxsockplus = ssock+1;
        while (1) {
                fd_set workreadfds = readfds;
                retval = select(maxsockplus, &workreadfds, NULL, NULL, NULL);
        maxsockplus = ssock+1;
        while (1) {
                fd_set workreadfds = readfds;
                retval = select(maxsockplus, &workreadfds, NULL, NULL, NULL);

-               
+

                if (retval < 0) {
                if (retval < 0) {

-                       dbg("error in select: %s", strerror(errno));
+                       if (errno != EINTR)
+                               dbg("error in select: %s", strerror(errno));

                        continue;
                }
                        continue;
                }

-               
+

                if (FD_ISSET(ssock, &workreadfds))
                        handle_msg(ssock);
                if (FD_ISSET(ssock, &workreadfds))
                        handle_msg(ssock);

-               
+

                if (FD_ISSET(pipefds[0], &workreadfds))
                        user_sighandler();
                if (FD_ISSET(pipefds[0], &workreadfds))
                        user_sighandler();

-               
+

                if (children_waiting) {
                        children_waiting = 0;
                        reap_kids();
                }
                if (children_waiting) {
                        children_waiting = 0;
                        reap_kids();
                }

-               
+

                if (run_msg_q) {
                        run_msg_q = 0;
                        msg_queue_manager();
                }
                if (run_msg_q) {
                        run_msg_q = 0;
                        msg_queue_manager();
                }

-               
+

                if (run_exec_q) {
                if (run_exec_q) {

-                       

                        /* this is tricky.  exec_queue_manager() loops over exec_list, and
                         * calls running_with_devpath(), which loops over running_list. This gives
                         * O(N*M), which can get *nasty*.  Clean up running_list before
                         * calling exec_queue_manager().
                         */
                        /* this is tricky.  exec_queue_manager() loops over exec_list, and
                         * calls running_with_devpath(), which loops over running_list. This gives
                         * O(N*M), which can get *nasty*.  Clean up running_list before
                         * calling exec_queue_manager().
                         */

-                       

                        if (children_waiting) {
                                children_waiting = 0;
                                reap_kids();
                        if (children_waiting) {
                                children_waiting = 0;
                                reap_kids();

diff --git a/udevd.h b/udevd.h
index 8b82ff9a7015ae19a85626cc644fc487bc153c21..96f179ad44b6065cc13f82dac9c26eed693f750d 100644 (file)
--- a/udevd.h
+++ b/udevd.h
@@ -34,7 +34,7 @@ struct hotplug_msg {
        struct list_head list;
        pid_t pid;
        int seqnum;
        struct list_head list;
        pid_t pid;
        int seqnum;

-       time_t queue_time;
+       long queue_time;

        char action[ACTION_SIZE];
        char devpath[DEVPATH_SIZE];
        char subsystem[SUBSYSTEM_SIZE];
        char action[ACTION_SIZE];
        char devpath[DEVPATH_SIZE];
        char subsystem[SUBSYSTEM_SIZE];

diff --git a/udevsend.c b/udevsend.c
index f1c46d33672e918bb548c798a84265e65b51390a..f28e445433ab156caae58a7eb7b0a74d8f6bd482 100644 (file)
--- a/udevsend.c
+++ b/udevsend.c
@@ -26,13 +26,13 @@
 #include <sys/socket.h>
 #include <sys/wait.h>
 #include <sys/un.h>
 #include <sys/socket.h>
 #include <sys/wait.h>
 #include <sys/un.h>

+#include <time.h>

 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stddef.h>
 #include <string.h>
 #include <unistd.h>
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stddef.h>
 #include <string.h>
 #include <unistd.h>

-#include <time.h>

 #include <linux/stddef.h>
 
 #include "udev.h"
 #include <linux/stddef.h>
 
 #include "udev.h"


@@ -53,16 +53,15 @@ void log_message (int level, const char *format, ...)
 }
 #endif
 
 }
 #endif
 

-static int build_hotplugmsg(struct hotplug_msg *msg, char *action,
-                           char *devpath, char *subsystem, int seqnum)
+static void build_hotplugmsg(struct hotplug_msg *msg, char *action,
+                            char *devpath, char *subsystem, int seqnum)

 {
 {

-       memset(msg, 0x00, sizeof(*msg));
+       memset(msg, 0x00, sizeof(struct hotplug_msg));

        strfieldcpy(msg->magic, UDEV_MAGIC);
        msg->seqnum = seqnum;
        strfieldcpy(msg->action, action);
        strfieldcpy(msg->devpath, devpath);
        strfieldcpy(msg->subsystem, subsystem);
        strfieldcpy(msg->magic, UDEV_MAGIC);
        msg->seqnum = seqnum;
        strfieldcpy(msg->action, action);
        strfieldcpy(msg->devpath, devpath);
        strfieldcpy(msg->subsystem, subsystem);

-       return sizeof(struct hotplug_msg);

 }
 
 static int start_daemon(void)
 }
 
 static int start_daemon(void)


@@ -108,7 +107,6 @@ int main(int argc, char* argv[])
        char *seqnum;
        int seq;
        int retval = 1;
        char *seqnum;
        int seq;
        int retval = 1;

-       int size;

        int loop;
        struct timespec tspec;
        int sock;
        int loop;
        struct timespec tspec;
        int sock;


@@ -155,18 +153,19 @@ int main(int argc, char* argv[])
                goto exit;
        }
 
                goto exit;
        }
 

-       memset(&saddr, 0x00, sizeof(saddr));
+       memset(&saddr, 0x00, sizeof(struct sockaddr_un));

        saddr.sun_family = AF_LOCAL;
        /* use abstract namespace for socket path */
        strcpy(&saddr.sun_path[1], UDEVD_SOCK_PATH);
        addrlen = offsetof(struct sockaddr_un, sun_path) + strlen(saddr.sun_path+1) + 1;
 
        saddr.sun_family = AF_LOCAL;
        /* use abstract namespace for socket path */
        strcpy(&saddr.sun_path[1], UDEVD_SOCK_PATH);
        addrlen = offsetof(struct sockaddr_un, sun_path) + strlen(saddr.sun_path+1) + 1;
 

-       size = build_hotplugmsg(&msg, action, devpath, subsystem, seq);
+       build_hotplugmsg(&msg, action, devpath, subsystem, seq);

 
        /* If we can't send, try to start daemon and resend message */
        loop = UDEVSEND_CONNECT_RETRY;
        while (loop--) {
 
        /* If we can't send, try to start daemon and resend message */
        loop = UDEVSEND_CONNECT_RETRY;
        while (loop--) {

-               retval = sendto(sock, &msg, size, 0, (struct sockaddr *)&saddr, addrlen);
+               retval = sendto(sock, &msg, sizeof(struct hotplug_msg), 0,
+                               (struct sockaddr *)&saddr, addrlen);

                if (retval != -1) {
                        retval = 0;
                        goto close_and_exit;
                if (retval != -1) {
                        retval = 0;
                        goto close_and_exit;