Re: [patch] udevd - cleanup and better timeout handling

[Kay Sievers <kay.sievers@vrfy.org>]

[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]

On Sun, Feb 01, 2004 at 07:16:36PM +0100, Kay Sievers wrote:
> On Sun, Feb 01, 2004 at 01:08:54AM -0800, Greg KH wrote:
> > On Sat, Jan 31, 2004 at 03:42:20AM +0100, Kay Sievers wrote:
> > > 
> > > Next cleaned up version. Hey, nobody wants to try it :)
> > > 
> > > Works for me, It's funny if I connect/disconnect my 4in1-usb-flash-reader
> > > every two seconds. The 2.6 usb rocks! I can connect/diconnect a hub with 3
> > > devices plugged in every second and don't run into any problem but a _very_
> > > big udevd queue.
> > 
> > Very sorry for the delay, been busy with other work these past few days.
> > 
> > I've applied this patch, and pushed it to the repo.  I'll try to test it
> > some more myself early next week, and let you know how it goes.
> 
> Here is a small cleanup and better Makefile integration.
> udevd and udevsender are now installed. Just switch HOTPLUG_EXEC from ROOT
> to SENDER before install and udevsend will be called.
> 
> We may add the location of the socket and lock file to the config,
> if this is needed.

Same patch with a fix for the stack size setting.

thanks,
Kay

[-- Attachment #2: 01-udevd-config.patch --]
[-- Type: text/plain, Size: 6072 bytes --]

===== Makefile 1.94 vs edited =====
--- 1.94/Makefile	Thu Jan 29 17:46:29 2004
+++ edited/Makefile	Mon Feb  2 02:44:27 2004
@@ -38,6 +38,7 @@
 INSTALL_DIR =	/usr/local/bin
 RELEASE_NAME =	$(ROOT)-$(VERSION)
 LOCAL_CFG_DIR =	etc/udev
+HOTPLUG_EXEC =	$(ROOT)
 
 DESTDIR =
 # override this to make udev look in a different location for it's config files
@@ -232,8 +233,10 @@
 	@echo \#define UDEV_CONFIG_FILE	\"$(configdir)\udev.conf\" >> $@
 	@echo \#define UDEV_RULES_FILE	\"$(configdir)\udev.rules\" >> $@
 	@echo \#define UDEV_PERMISSION_FILE	\"$(configdir)\udev.permissions\" >> $@
-	@echo \#define UDEV_BIN		\"$(PWD)/udev\" >> $@
-	@echo \#define UDEVD_BIN	\"$(PWD)/udevd\" >> $@
+	@echo \#define UDEV_BIN		\"$(DESTDIR)$(sbindir)/udev\" >> $@
+	@echo \#define UDEVD_BIN	\"$(DESTDIR)$(sbindir)/udevd\" >> $@
+	@echo \#define UDEVD_SOCK	\"$(udevdir)/\.udevd.sock\" >> $@
+	@echo \#define UDEVD_LOCK	\"$(udevdir)/\.udevd.lock\" >> $@
 
 # config files automatically generated
 GEN_CONFIGS =	$(LOCAL_CFG_DIR)/udev.conf
@@ -338,6 +341,8 @@
 	$(INSTALL) -d $(DESTDIR)$(udevdir)
 	$(INSTALL) -d $(DESTDIR)$(hotplugdir)
 	$(INSTALL_PROGRAM) -D $(ROOT) $(DESTDIR)$(sbindir)/$(ROOT)
+	$(INSTALL_PROGRAM) -D $(DAEMON) $(DESTDIR)$(sbindir)/$(DAEMON)
+	$(INSTALL_PROGRAM) -D $(SENDER) $(DESTDIR)$(sbindir)/$(SENDER)
 	$(INSTALL_PROGRAM) -D $(HELPER) $(DESTDIR)$(sbindir)/$(HELPER)
 	@if [ "x$(USE_LSB)" = "xtrue" ]; then \
 		$(INSTALL_PROGRAM) -D etc/init.d/udev.init.LSB $(DESTDIR)$(initdir)/udev; \
@@ -347,8 +352,8 @@
 	fi
 	$(INSTALL_DATA) -D udev.8 $(DESTDIR)$(mandir)/man8/udev.8
 	$(INSTALL_DATA) -D udevinfo.8 $(DESTDIR)$(mandir)/man8/udevinfo.8
-	- rm -f $(DESTDIR)$(hotplugdir)/udev.hotplug
-	- ln -f -s $(sbindir)/$(ROOT) $(DESTDIR)$(hotplugdir)/udev.hotplug
+	- rm -f $(DESTDIR)$(hotplugdir)/$(HOTPLUG_EXEC).hotplug
+	- ln -f -s $(sbindir)/$(HOTPLUG_EXEC) $(DESTDIR)$(hotplugdir)/udev.hotplug
 	@extras="$(EXTRAS)" ; for target in $$extras ; do \
 		echo $$target ; \
 		$(MAKE) prefix=$(prefix) LD="$(LD)" SYSFS="$(SYSFS)" \
@@ -364,6 +369,8 @@
 	- rm $(mandir)/man8/udev.8
 	- rm $(mandir)/man8/udevinfo.8
 	- rm $(sbindir)/$(ROOT)
+	- rm $(sbindir)/$(DAEMON)
+	- rm $(sbindir)/$(SENDER)
 	- rm $(sbindir)/$(HELPER)
 	- rmdir $(hotplugdir)
 	- rmdir $(configdir)
===== udevd.c 1.8 vs edited =====
--- 1.8/udevd.c	Sat Jan 31 04:08:44 2004
+++ edited/udevd.c	Mon Feb  2 02:41:34 2004
@@ -19,6 +19,7 @@
  *
  */
 
+#include <pthread.h>
 #include <stddef.h>
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -33,7 +34,6 @@
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/un.h>
-#include <pthread.h>
 
 #include "list.h"
 #include "udev.h"
@@ -71,7 +71,6 @@
 	    msg->seqnum, msg->action, msg->devpath, msg->subsystem);
 }
 
-/* allocates a new message */
 static struct hotplug_msg *msg_create(void)
 {
 	struct hotplug_msg *new_msg;
@@ -81,10 +80,15 @@
 		dbg("error malloc");
 		return NULL;
 	}
-	memset(new_msg, 0x00, sizeof(struct hotplug_msg));
 	return new_msg;
 }
 
+static void msg_delete(struct hotplug_msg *msg)
+{
+	if (msg != NULL)
+		free(msg);
+}
+
 /* orders the message in the queue by sequence number */
 static void msg_queue_insert(struct hotplug_msg *msg)
 {
@@ -143,7 +147,7 @@
 	list_del_init(&msg->list);
 	pthread_mutex_unlock(&running_lock);
 
-	free(msg);
+	msg_delete(msg);
 
 	/* signal queue activity to exec manager */
 	pthread_mutex_lock(&exec_active_lock);
@@ -289,6 +293,7 @@
 
 	if (strncmp(msg->magic, UDEV_MAGIC, sizeof(UDEV_MAGIC)) != 0 ) {
 		dbg("message magic '%s' doesn't match, ignore it", msg->magic);
+		msg_delete(msg);
 		goto exit;
 	}
 
@@ -307,7 +312,7 @@
 		case SIGINT:
 		case SIGTERM:
 			unlink(UDEVD_LOCK);
-			unlink(UDEVD_SOCKET);
+			unlink(UDEVD_SOCK);
 			exit(20 + signum);
 			break;
 		default:
@@ -320,7 +325,6 @@
 	char string[50];
 	int lock_file;
 
-	/* see if we can open */
 	lock_file = open(UDEVD_LOCK, O_RDWR | O_CREAT, 0x640);
 	if (lock_file < 0)
 		return -1;
@@ -359,9 +363,9 @@
 
 	memset(&saddr, 0x00, sizeof(saddr));
 	saddr.sun_family = AF_LOCAL;
-	strcpy(saddr.sun_path, UDEVD_SOCKET);
+	strcpy(saddr.sun_path, UDEVD_SOCK);
 
-	unlink(UDEVD_SOCKET);
+	unlink(UDEVD_SOCK);
 	ssock = socket(AF_LOCAL, SOCK_STREAM, 0);
 	if (ssock == -1) {
 		dbg("error getting socket");
@@ -389,6 +393,7 @@
 	/* set default attributes for created threads */
 	pthread_attr_init(&thr_attr);
 	pthread_attr_setdetachstate(&thr_attr, PTHREAD_CREATE_DETACHED);
+	pthread_attr_setstacksize(&thr_attr, 16 * 1024);
 
 	/* init queue management */
 	pthread_create(&mgr_msg_tid, &thr_attr, msg_queue_manager, NULL);
@@ -399,14 +404,13 @@
 	while (1) {
 		csock = accept(ssock, &caddr, &clen);
 		if (csock < 0) {
-			if (errno == EINTR)
-				continue;
 			dbg("client accept failed\n");
+			continue;
 		}
 		pthread_create(&cli_tid, &thr_attr, client_threads, (void *) csock);
 	}
 exit:
 	close(ssock);
-	unlink(UDEVD_SOCKET);
+	unlink(UDEVD_SOCK);
 	exit(1);
 }
===== udevd.h 1.5 vs edited =====
--- 1.5/udevd.h	Sat Jan 31 04:12:52 2004
+++ edited/udevd.h	Mon Feb  2 02:40:48 2004
@@ -24,18 +24,9 @@
 
 #include "list.h"
 
-/*
- * FIXME: udev_root is post compile configurable and may also be
- * mounted over at any time and /var/run/ and /tmp/ is unusable,
- * cause it's cleaned at system startup, long _after_ udevd is
- * already running. Should we use udev_init_config()?
- */
-
-#define UDEV_MAGIC			"udev_" UDEV_VERSION
+#define UDEV_MAGIC			"udevd_" UDEV_VERSION
 #define EVENT_TIMEOUT_SEC		5
 #define UDEVSEND_CONNECT_RETRY		20 /* x 100 millisec */
-#define UDEVD_SOCKET			UDEV_ROOT ".udevd.socket"
-#define UDEVD_LOCK			UDEV_ROOT ".udevd.pid"
 
 struct hotplug_msg {
 	char magic[20];
===== udevsend.c 1.9 vs edited =====
--- 1.9/udevsend.c	Sat Jan 31 03:53:31 2004
+++ edited/udevsend.c	Mon Feb  2 02:40:48 2004
@@ -159,7 +159,7 @@
 
 	memset(&saddr, 0x00, sizeof(saddr));
 	saddr.sun_family = AF_LOCAL;
-	strcpy(saddr.sun_path, UDEVD_SOCKET);
+	strcpy(saddr.sun_path, UDEVD_SOCK);
 
 	/* try to connect, if it fails start daemon */
 	retval = connect(sock, &saddr, sizeof(saddr));