* [PATCH[ udevd race conditions and performance, assorted cleanups
@ 2004-03-27 4:48 Chris Friesen
2004-03-27 11:30 ` Kay Sievers
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Chris Friesen @ 2004-03-27 4:48 UTC (permalink / raw)
To: linux-hotplug
[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]
This patch covers a number of areas:
1) sysfs.h is fixed up to use the common dbg() macro. This fixes the
case where DEBUG is defined but USE_LOG isn't.
2) udevstart.c is modified to include the proper headers, rather than
getting them indirectly which can break depending on Makefile flags
3) udevd.c gets some major changes:
a) I added a pipe from the signal handler. This fixes the race
conditions that I mentioned earlier. Basically, the point of the pipe
is to force the select() call to return immediately if a signal handler
fired before we actually started the select() call. This then lets us
run the appropriate code based on flags set in the signal handler proper.
b) I added a number of flags to coalesce calls to common routines. This
should make things slightly more efficient.
c) since most calls will tend to come in with a sequence number larger
than what has been received, I switched msg_queue_insert() to scan the
msg_list backwards to improve performance.
Comments?
Chris
[-- Attachment #2: udevd.diff --]
[-- Type: text/plain, Size: 7923 bytes --]
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet udev/libsysfs/sysfs.h myudev/libsysfs/sysfs.h
--- udev/libsysfs/sysfs.h Fri Mar 26 21:01:44 2004
+++ myudev/libsysfs/sysfs.h Fri Mar 26 22:08:11 2004
@@ -37,9 +37,7 @@
#ifdef DEBUG
#include "../logging.h"
#define dprintf(format, arg...) \
- do { \
- log_message (LOG_DEBUG , "%s: " format , __FUNCTION__ , ## arg); \
- } while (0)
+ dbg(format, ##arg)
#else
#define dprintf(format, arg...) do { } while (0)
#endif
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet udev/udevd.c myudev/udevd.c
--- udev/udevd.c Fri Mar 26 21:01:44 2004
+++ myudev/udevd.c Fri Mar 26 22:50:41 2004
@@ -33,6 +33,7 @@
#include <sys/socket.h>
#include <sys/un.h>
#include <sys/time.h>
+#include <fcntl.h>
#include "list.h"
#include "udev.h"
@@ -41,9 +42,12 @@
#include "udevd.h"
#include "logging.h"
+static int pipefds[2];
static int expected_seqnum = 0;
volatile static int children_waiting;
-volatile static int msg_q_timeout;
+volatile static int run_msg_q;
+volatile static int sig_flag;
+static int run_exec_q;
static LIST_HEAD(msg_list);
static LIST_HEAD(exec_list);
@@ -51,6 +55,8 @@
static void exec_queue_manager(void);
static void msg_queue_manager(void);
+static void user_sighandler(void);
+static void reap_kids(void);
#ifdef LOG
unsigned char logname[LOGNAME_SIZE];
@@ -66,10 +72,12 @@
static void msg_dump_queue(void)
{
+#ifdef DEBUG
struct hotplug_msg *msg;
list_for_each_entry(msg, &msg_list, list)
dbg("sequence %d in queue", msg->seqnum);
+#endif
}
static void msg_dump(struct hotplug_msg *msg)
@@ -92,7 +100,6 @@
{
list_del(&msg->list);
free(msg);
- exec_queue_manager();
}
/* orders the message in the queue by sequence number */
@@ -100,18 +107,20 @@
{
struct hotplug_msg *loop_msg;
- /* sort message by sequence number into list*/
- list_for_each_entry(loop_msg, &msg_list, list)
- if (loop_msg->seqnum > msg->seqnum)
+ /* sort message by sequence number into list. events
+ * will tend to come in order, so scan the list backwards
+ */
+ list_for_each_entry_reverse(loop_msg, &msg_list, list)
+ if (loop_msg->seqnum < msg->seqnum)
break;
- list_add_tail(&msg->list, &loop_msg->list);
+ list_add(&msg->list, &loop_msg->list);
dbg("queued message seq %d", msg->seqnum);
/* store timestamp of queuing */
msg->queue_time = time(NULL);
/* run msg queue manager */
- msg_queue_manager();
+ run_msg_q = 1;
return ;
}
@@ -138,6 +147,9 @@
case -1:
dbg("fork of child failed");
run_queue_delete(msg);
+ /* note: we never managed to run, so we had no impact on
+ * running_with_devpath(), so don't bother setting run_exec_q
+ */
break;
default:
/* get SIGCHLD in main loop */
@@ -180,7 +192,7 @@
static void msg_move_exec(struct hotplug_msg *msg)
{
list_move_tail(&msg->list, &exec_list);
- exec_queue_manager();
+ run_exec_q = 1;
expected_seqnum = msg->seqnum+1;
dbg("moved seq %d to exec, next expected is %d",
msg->seqnum, expected_seqnum);
@@ -276,7 +288,7 @@
/* if no seqnum is given, we move straight to exec queue */
if (msg->seqnum == -1) {
list_add(&msg->list, &exec_list);
- exec_queue_manager();
+ run_exec_q = 1;
} else {
msg_queue_insert(msg);
}
@@ -289,19 +301,37 @@
static void sig_handler(int signum)
{
+ int rc;
switch (signum) {
case SIGINT:
case SIGTERM:
exit(20 + signum);
break;
case SIGALRM:
- msg_q_timeout = 1;
+ /* set flag, then write to pipe if needed */
+ run_msg_q = 1;
+ goto do_write;
break;
case SIGCHLD:
+ /* set flag, then write to pipe if needed */
children_waiting = 1;
+ goto do_write;
break;
default:
dbg("unhandled signal");
+ return;
+ }
+
+do_write:
+ /* if pipe is empty, write to pipe to force select to return
+ * immediately when it gets called
+ */
+ if (!sig_flag) {
+ rc = write(pipefds[1],&signum,sizeof(signum));
+ if (rc < 0)
+ dbg("unable to write to pipe");
+ else
+ sig_flag = 1;
}
}
@@ -314,19 +344,53 @@
if (msg->pid == pid) {
dbg("<== exec seq %d came back", msg->seqnum);
run_queue_delete(msg);
+
+ /* we want to run the exec queue manager since there may
+ * be events waiting with the devpath of the one that
+ * just finished
+ */
+ run_exec_q = 1;
return;
}
}
}
+static void reap_kids()
+{
+ /* reap all dead children */
+ while(1) {
+ int pid = waitpid(-1, 0, WNOHANG);
+ if ((pid == -1) || (pid == 0))
+ break;
+ udev_done(pid);
+ }
+}
+
+/* just read everything from the pipe and clear the flag,
+ * the useful flags were set in the signal handler
+ */
+static void user_sighandler()
+{
+ int sig;
+ while(1) {
+ int rc = read(pipefds[0],&sig,sizeof(sig));
+ if (rc < 0)
+ break;
+
+ sig_flag = 0;
+ }
+}
+
+
int main(int argc, char *argv[])
{
- int ssock;
+ int ssock, maxsockplus;
struct sockaddr_un saddr;
socklen_t addrlen;
int retval;
const int on = 1;
struct sigaction act;
+ fd_set readfds;
init_logging("udevd");
dbg("version %s", UDEV_VERSION);
@@ -335,16 +399,32 @@
dbg("need to be root, exit");
exit(1);
}
+
+ /* 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);
+ }
- /* set signal handler */
+ /* set signal handlers */
act.sa_handler = sig_handler;
- sigemptyset (&act.sa_mask);
+ sigemptyset(&act.sa_mask);
act.sa_flags = SA_RESTART;
sigaction(SIGINT, &act, NULL);
sigaction(SIGTERM, &act, NULL);
-
- /* we want these two to interrupt system calls */
- act.sa_flags = 0;
sigaction(SIGALRM, &act, NULL);
sigaction(SIGCHLD, &act, NULL);
@@ -370,23 +450,50 @@
/* 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);
+ maxsockplus = ssock+1;
while (1) {
- handle_msg(ssock);
-
- while(msg_q_timeout) {
- msg_q_timeout = 0;
- msg_queue_manager();
+ fd_set workreadfds = readfds;
+ retval = select(maxsockplus, &workreadfds, NULL, NULL, NULL);
+
+ if (retval < 0) {
+ dbg("error in select: %s", strerror(errno));
+ continue;
}
-
- while(children_waiting) {
+
+ if (FD_ISSET(ssock, &workreadfds))
+ handle_msg(ssock);
+
+ if (FD_ISSET(pipefds[0], &workreadfds))
+ user_sighandler();
+
+ if (children_waiting) {
children_waiting = 0;
- /* reap all dead children */
- while(1) {
- int pid = waitpid(-1, 0, WNOHANG);
- if ((pid == -1) || (pid == 0))
- break;
- udev_done(pid);
+ reap_kids();
+ }
+
+ if (run_msg_q) {
+ run_msg_q = 0;
+ msg_queue_manager();
+ }
+
+ 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().
+ */
+
+ if (children_waiting) {
+ children_waiting = 0;
+ reap_kids();
}
+
+ run_exec_q = 0;
+ exec_queue_manager();
}
}
exit:
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet udev/udevstart.c myudev/udevstart.c
--- udev/udevstart.c Fri Mar 26 21:01:44 2004
+++ myudev/udevstart.c Fri Mar 26 23:33:41 2004
@@ -29,6 +29,8 @@
#include <ctype.h>
#include <dirent.h>
#include <sys/wait.h>
+#include <sys/types.h>
+#include <unistd.h>
#include "logging.h"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH[ udevd race conditions and performance, assorted cleanups
2004-03-27 4:48 [PATCH[ udevd race conditions and performance, assorted cleanups Chris Friesen
@ 2004-03-27 11:30 ` Kay Sievers
2004-03-28 1:08 ` Kay Sievers
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Kay Sievers @ 2004-03-27 11:30 UTC (permalink / raw)
To: linux-hotplug
On Fri, Mar 26, 2004 at 11:48:15PM -0500, Chris Friesen wrote:
>
> This patch covers a number of areas:
>
> 1) sysfs.h is fixed up to use the common dbg() macro. This fixes the
> case where DEBUG is defined but USE_LOG isn't.
>
> 2) udevstart.c is modified to include the proper headers, rather than
> getting them indirectly which can break depending on Makefile flags
>
> 3) udevd.c gets some major changes:
> a) I added a pipe from the signal handler. This fixes the race
> conditions that I mentioned earlier. Basically, the point of the pipe
> is to force the select() call to return immediately if a signal handler
> fired before we actually started the select() call. This then lets us
> run the appropriate code based on flags set in the signal handler proper.
> b) I added a number of flags to coalesce calls to common routines. This
> should make things slightly more efficient.
> c) since most calls will tend to come in with a sequence number larger
> than what has been received, I switched msg_queue_insert() to scan the
> msg_list backwards to improve performance.
Looks really nice, seems a bit faster too.
Whitespace is broken, no tabs?
> while (1) {
> - handle_msg(ssock);
> -
> - while(msg_q_timeout) {
> - msg_q_timeout = 0;
> - msg_queue_manager();
> + fd_set workreadfds = readfds;
> + retval = select(maxsockplus, &workreadfds, NULL, NULL, NULL);
> +
> + if (retval < 0) {
> + dbg("error in select: %s", strerror(errno));
> + continue;
We interrupt select() intentionally, so better do not call it
"error" in syslog :)
thanks,
Kay
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH[ udevd race conditions and performance, assorted cleanups
2004-03-27 4:48 [PATCH[ udevd race conditions and performance, assorted cleanups Chris Friesen
2004-03-27 11:30 ` Kay Sievers
@ 2004-03-28 1:08 ` Kay Sievers
2004-03-29 4:47 ` Chris Friesen
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Kay Sievers @ 2004-03-28 1:08 UTC (permalink / raw)
To: linux-hotplug
[-- Attachment #1: Type: text/plain, Size: 1459 bytes --]
On Fri, Mar 26, 2004 at 11:48:15PM -0500, Chris Friesen wrote:
>
> This patch covers a number of areas:
>
> 1) sysfs.h is fixed up to use the common dbg() macro. This fixes the
> case where DEBUG is defined but USE_LOG isn't.
>
> 2) udevstart.c is modified to include the proper headers, rather than
> getting them indirectly which can break depending on Makefile flags
>
> 3) udevd.c gets some major changes:
> a) I added a pipe from the signal handler. This fixes the race
> conditions that I mentioned earlier. Basically, the point of the pipe
> is to force the select() call to return immediately if a signal handler
> fired before we actually started the select() call. This then lets us
> run the appropriate code based on flags set in the signal handler proper.
> b) I added a number of flags to coalesce calls to common routines. This
> should make things slightly more efficient.
> c) since most calls will tend to come in with a sequence number larger
> than what has been received, I switched msg_queue_insert() to scan the
> msg_list backwards to improve performance.
Chris,
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.
thanks,
Kay
[-- Attachment #2: 01-udevd.patch --]
[-- Type: text/plain, Size: 8958 bytes --]
===== Makefile 1.148 vs edited =====
--- 1.148/Makefile Sat Mar 27 02:23:20 2004
+++ edited/Makefile Sun Mar 28 01:36:10 2004
@@ -220,6 +220,7 @@
ifeq ($(strip $(USE_KLIBC)),true)
OBJS += klibc_fixups.o
+ KLIBC_FIXUP = klibc_fixups.o
endif
# header files automatically generated
@@ -266,8 +267,8 @@
$(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)
===== klibc_fixups.c 1.8 vs edited =====
--- 1.8/klibc_fixups.c Wed Mar 17 23:40:12 2004
+++ edited/klibc_fixups.c Sun Mar 28 00:55:06 2004
@@ -30,12 +30,14 @@
#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"
+_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)
===== klibc_fixups.h 1.6 vs edited =====
--- 1.6/klibc_fixups.h Sun Feb 29 05:51:47 2004
+++ edited/klibc_fixups.h Sun Mar 28 01:05:19 2004
@@ -3,6 +3,11 @@
#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 */
===== udevd.c 1.26 vs edited =====
--- 1.26/udevd.c Sat Mar 27 23:32:48 2004
+++ edited/udevd.c Sun Mar 28 01:48:23 2004
@@ -28,12 +28,15 @@
#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/time.h>
#include <fcntl.h>
+#include "klibc_fixups.h"
+#ifndef __KLIBC__
+#include <sys/sysinfo.h>
+#endif
#include "list.h"
#include "udev.h"
@@ -106,6 +109,7 @@
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
@@ -113,11 +117,13 @@
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 */
- 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;
@@ -203,7 +209,8 @@
{
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:
@@ -215,7 +222,9 @@
}
/* 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;
@@ -226,11 +235,10 @@
msg_dump_queue();
+ /* set timeout for remaining queued events */
if (list_empty(&msg_list) == 0) {
- /* set timeout for remaining queued events */
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);
}
}
@@ -399,25 +407,25 @@
dbg("need to be root, exit");
exit(1);
}
-
+
/* 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;
@@ -450,43 +458,42 @@
/* 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);
-
+
if (retval < 0) {
- dbg("error in select: %s", strerror(errno));
+ if (errno != EINTR)
+ dbg("error in select: %s", strerror(errno));
continue;
}
-
+
if (FD_ISSET(ssock, &workreadfds))
handle_msg(ssock);
-
+
if (FD_ISSET(pipefds[0], &workreadfds))
user_sighandler();
-
+
if (children_waiting) {
children_waiting = 0;
reap_kids();
}
-
+
if (run_msg_q) {
run_msg_q = 0;
msg_queue_manager();
}
-
+
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().
*/
-
if (children_waiting) {
children_waiting = 0;
reap_kids();
===== udevd.h 1.8 vs edited =====
--- 1.8/udevd.h Thu Feb 26 04:09:18 2004
+++ edited/udevd.h Sat Mar 27 23:40:46 2004
@@ -34,7 +34,7 @@
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];
===== udevsend.c 1.28 vs edited =====
--- 1.28/udevsend.c Wed Mar 17 23:40:12 2004
+++ edited/udevsend.c Sun Mar 28 00:08:58 2004
@@ -26,13 +26,13 @@
#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 <time.h>
#include <linux/stddef.h>
#include "udev.h"
@@ -53,16 +53,15 @@
}
#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);
- return sizeof(struct hotplug_msg);
}
static int start_daemon(void)
@@ -108,7 +107,6 @@
char *seqnum;
int seq;
int retval = 1;
- int size;
int loop;
struct timespec tspec;
int sock;
@@ -155,18 +153,19 @@
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;
- 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--) {
- 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;
===== libsysfs/sysfs.h 1.8 vs edited =====
--- 1.8/libsysfs/sysfs.h Sat Mar 27 23:32:48 2004
+++ edited/libsysfs/sysfs.h Sat Mar 27 23:33:06 2004
@@ -36,8 +36,7 @@
/* 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH[ udevd race conditions and performance, assorted cleanups
2004-03-27 4:48 [PATCH[ udevd race conditions and performance, assorted cleanups Chris Friesen
2004-03-27 11:30 ` Kay Sievers
2004-03-28 1:08 ` Kay Sievers
@ 2004-03-29 4:47 ` Chris Friesen
2004-03-31 23:04 ` Greg KH
2004-03-31 23:04 ` Greg KH
4 siblings, 0 replies; 6+ messages in thread
From: Chris Friesen @ 2004-03-29 4:47 UTC (permalink / raw)
To: linux-hotplug
Kay Sievers wrote:
> Chris,
> 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.
Looks good. Sorry about the whitespace stuff...I forgot to change my
settings back after working on another project.
About the time stuff. Wouldn't it be nice if glibc had a wrapper around
the various ways to get a high resolution monotonically increasing
64-bit timestamp of "time since boot"? So for x86 you'd end up using
the tsc, for ppc you'd use the tbr, etc, etc. Currently we've got no
portable way to get a high res time that can't go backwards.
Chris
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH[ udevd race conditions and performance, assorted cleanups
2004-03-27 4:48 [PATCH[ udevd race conditions and performance, assorted cleanups Chris Friesen
` (2 preceding siblings ...)
2004-03-29 4:47 ` Chris Friesen
@ 2004-03-31 23:04 ` Greg KH
2004-03-31 23:04 ` Greg KH
4 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2004-03-31 23:04 UTC (permalink / raw)
To: linux-hotplug
On Fri, Mar 26, 2004 at 11:48:15PM -0500, Chris Friesen wrote:
>
> This patch covers a number of areas:
>
> 1) sysfs.h is fixed up to use the common dbg() macro. This fixes the
> case where DEBUG is defined but USE_LOG isn't.
>
> 2) udevstart.c is modified to include the proper headers, rather than
> getting them indirectly which can break depending on Makefile flags
>
> 3) udevd.c gets some major changes:
> a) I added a pipe from the signal handler. This fixes the race
> conditions that I mentioned earlier. Basically, the point of the pipe
> is to force the select() call to return immediately if a signal handler
> fired before we actually started the select() call. This then lets us
> run the appropriate code based on flags set in the signal handler proper.
> b) I added a number of flags to coalesce calls to common routines. This
> should make things slightly more efficient.
> c) since most calls will tend to come in with a sequence number larger
> than what has been received, I switched msg_queue_insert() to scan the
> msg_list backwards to improve performance.
>
> Comments?
Thanks for doing this, applied.
greg k-h
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH[ udevd race conditions and performance, assorted cleanups
2004-03-27 4:48 [PATCH[ udevd race conditions and performance, assorted cleanups Chris Friesen
` (3 preceding siblings ...)
2004-03-31 23:04 ` Greg KH
@ 2004-03-31 23:04 ` Greg KH
4 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2004-03-31 23:04 UTC (permalink / raw)
To: linux-hotplug
On Sun, Mar 28, 2004 at 03:08:45AM +0200, Kay Sievers wrote:
> 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.
Nice fix, applied.
greg k-h
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-03-31 23:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-03-27 4:48 [PATCH[ udevd race conditions and performance, assorted cleanups Chris Friesen
2004-03-27 11:30 ` Kay Sievers
2004-03-28 1:08 ` Kay Sievers
2004-03-29 4:47 ` Chris Friesen
2004-03-31 23:04 ` Greg KH
2004-03-31 23:04 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).