convert udevsend/udevd to using datagram sockets

PATCH In review Chris Friesen 3 participants 7 messages Feb 4, 2004 – Feb 5, 2004 Kbuild

lore

Share & tools

lore

Chris Friesen

author

2004-02-04 07:00 UTC #0 lore raw

PATCH 1/1 convert udevsend/udevd to using datagram sockets

This patch does a couple of things to udevsend/udevd. It doesn't touch udev itself at all.

1) It switches to using the abstract namespace. This simplifies things since you don't have to go around unlinking. When the socket goes away, the address is gone.

2) The protocol between udevsend and udevd is fundamentally message-based. This patch recognizes this by using SOCK_DGRAM rather than SOCK_STREAM, which simplifies the message protocol. (Note that with unix sockets, you can still detect if the daemon is not running with SOCK_DGRAM. )

3) The main loop in udevd is much simplified. Part of this is due to the lower complexity of datagram sockets. As well, the new main loop just calls a plain function that sits on a blocking recv() to handle incoming messages instead of spawning a thread that does almost nothing and then exits.

There is still a hole if the message gets delivered and the server crashes before it gets handled--to close that we would need an ack message. If anyone wants us to be that paranoid, it would be easy to implement.

Any comments?

Chris

Makefile +0 -1

diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet udev/Makefile myudev/Makefile
@@ -241,7 +241,6 @@
 	@echo \#define UDEV_PERMISSION_FILE	\"$(configdir)\udev.permissions\" >> $@
 	@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

udevd.c +16 ~~-34~~

121 lines not on this page · raw message

udevd.h +1 -0

diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet udev/udevd.h myudev/udevd.h
@@ -24,6 +24,7 @@
 
 #include "list.h"
 
+#define UDEV_SOCK_NAME "udevd"
 #define UDEV_MAGIC			"udevd_" UDEV_VERSION
 #define EVENT_TIMEOUT_SEC		5
 #define UDEVSEND_CONNECT_RETRY		20 /* x 100 millisec */

udevsend.c +46 ~~-42~~

121 lines not on this page · raw message

Kay Sievers

2004-02-04 14:02 UTC #1 lore raw

Quoted 6 lines · Chris Friesen

This patch does a couple of things to udevsend/udevd. It doesn't touch udev itself at all.
1) It switches to using the abstract namespace. This simplifies things since you don't have to go around unlinking. When the socket goes away, the address is gone.

This is really nice, I like it. Didn't know that this is possible.

Do you have any idea what we can do with the lock file. It would be very nice to get rid of it, cause we remount the filesystem and it's gone.

Quoted 11 lines

2) The protocol between udevsend and udevd is fundamentally message-based. This patch recognizes this by using SOCK_DGRAM rather than SOCK_STREAM, which simplifies the message protocol. (Note that with unix sockets, you can still detect if the daemon is not running with SOCK_DGRAM. )
3) The main loop in udevd is much simplified. Part of this is due to the lower complexity of datagram sockets. As well, the new main loop just calls a plain function that sits on a blocking recv() to handle incoming messages instead of spawning a thread that does almost nothing and then exits.

You are talking about complexity, but what is about speed?

blocking daemon:
time for i in seq 1 100; do SEQNUM=$i ACTIONd DEVPATH=$i ./udevsend x; done

  real    0m1.175s
  user    0m0.054s
  sys     0m0.081s

threading daemon:
time for i in seq 1 100; do SEQNUM=$i ACTIONd DEVPATH=$i ./udevsend x; done

  real    0m0.048s
  user    0m0.018s
  sys     0m0.029s

I don't see what we get with the blocking I/O. It seems more like personal taste than numbers. :)
You may catch up with NPTL and old thread issues.
Linux is _now_ really really nice with threads.

Quoted 4 lines

There is still a hole if the message gets delivered and the server crashes before it gets handled--to close that we would need an ack message. If anyone wants us to be that paranoid, it would be easy to implement.

Uhh, first switch to unreliable communication and the make a protocol by yourself? Sounds a bit crazy.

Quoted 1 line

Any comments?

I can see your motivation, but unless we can get rid of _all_ the threads or at least getting faster, I don't want to change it.

thanks,
Kay

-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

[email protected]

author

2004-02-04 15:34 UTC #2 lore raw

In reply to Chris Friesen

Quoted 7 lines

From: Kay Sievers <[email protected]>
On Wed, Feb 04, 2004 at 02:00:35AM -0500, Chris Friesen wrote:
Do you have any idea what we can do with the lock file. It would be very nice to get rid of it, cause we remount the filesystem and it's gone.

I'll think about it and get back to you.

Quoted 10 lines

Quoted 5 lines
3) The main loop in udevd is much simplified. Part of this is due to the lower complexity of datagram sockets. As well, the new main loop just calls a plain function that sits on a blocking recv() to handle incoming messages instead of spawning a thread that does almost nothing and then exits.
You are talking about complexity, but what is about speed?
blocking daemon:
time for i in seq 1 100; do SEQNUM=$i ACTIONd DEVPATH=$i ./udevsend

x; done

Quoted 6 lines

  real    0m1.175s   
  user    0m0.054s   
  sys     0m0.081s   
threading daemon:
time for i in seq 1 100; do SEQNUM=$i ACTIONd DEVPATH=$i ./udevsend

x; done

Quoted 6 lines

  real    0m0.048s   
  user    0m0.018s   
  sys     0m0.029s   
I don't see what we get with the blocking I/O. It seems more like personal taste than numbers. :)

Interesting. I'll have to take a look at that. Is the threaded case using stream or datagram?

Quoted 7 lines

Quoted 4 lines
There is still a hole if the message gets delivered and the server crashes before it gets handled--to close that we would need an ack message. If anyone wants us to be that paranoid, it would be easy to implement.
Uhh, first switch to unreliable communication and the make a protocol by yourself? Sounds a bit crazy.

The hole is still open in the threaded case. If udevd crashes after the recv() call, you lost the information. With either communication type, you need an ack *after* udev has been called and completed (with udevsend retrying periodically until it gets that ack) before you're totally covered.

I'm at work right now. I'll take another look this evening.

Chris

Mike Waychison

2004-02-04 21:06 UTC #3 lore raw

In reply to Chris Friesen

Quoted 9 lines · Chris Friesen

This patch does a couple of things to udevsend/udevd. It doesn't touch udev itself at all.
1) It switches to using the abstract namespace. This simplifies things since you don't have to go around unlinking. When the socket goes away, the address is gone.

<snip>

Quoted 22 lines

	memset(&saddr, 0x00, sizeof(saddr));
	saddr.sun_family = AF_LOCAL;

udevd.c +8 -3

-	strcpy(saddr.sun_path, UDEVD_SOCK);
+   saddr.sun_path[0] = 0;
+   strcpy(saddr.sun_path+1, UDEV_SOCK_NAME);
+	
+   /* The size of the address is the offset of the start of the filename,
+    * plus its length, plus one for the beginning null byte.
+	 */
+   addrlen = offsetof(struct sockaddr_un, sun_path) + strlen (saddr.sun_path+1) + 1;
 
-	unlink(UDEVD_SOCK);
-	ssock = socket(AF_LOCAL, SOCK_STREAM, 0);
+	ssock = socket(AF_LOCAL, SOCK_DGRAM, 0);

	if (ssock = -1) {
		dbg("error getting socket");
		exit(1);
	}

udevd.c line 372

Quoted 2 lines

udevd.c +1 -1

-	retval = bind(ssock, &saddr, sizeof(saddr));
+	retval = bind(ssock, &saddr, addrlen);

The addrlen parameter should still be sizeof(saddr), regarless of the contents of the abstract namespace address. The same would go for the sendto command.

Signature · 23 lines

Mike Waychison
Sun Microsystems, Inc.
1 (650) 352-5299 voice
1 (416) 202-8336 voice
mailto: [email protected]
http://www.sun.com

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
NOTICE:  The opinions expressed in this email are held by me, 
and may not represent the views of Sun Microsystems, Inc.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 



-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Chris Friesen

author

2004-02-05 01:24 UTC #4 lore raw

In reply to Chris Friesen

Quoted 3 lines · Mike Waychison

The addrlen parameter should still be sizeof(saddr), regarless of the contents of the abstract namespace address. The same would go for the sendto command.

Do you have any definitive references for this? I don't have any references at home, but this style is all over the web. If you give it the full length, then you waste time passing a bunch of zeros to the kernel and then hashing over them. As long as the length is the same on both sides, it should work just fine, no?

Chris

Mike Waychison

2004-02-05 02:08 UTC #5 lore raw

In reply to Chris Friesen

Quoted 7 lines · Chris Friesen

Quoted 3 lines · Mike Waychison
The addrlen parameter should still be sizeof(saddr), regarless of the contents of the abstract namespace address. The same would go for the sendto command.

Quoted 5 lines

Do you have any definitive references for this? I don't have any references at home, but this style is all over the web. If you give it the full length, then you waste time passing a bunch of zeros to the kernel and then hashing over them. As long as the length is the same on both sides, it should work just fine, no?

I stand corrected and indeed, this is the method used all over the internet (heck, even docs.sun.com documents it as sizeof(addr.sun_family) + strlen(addr.sun_path)).

I guess I'm so used to seeing sizeof(addr) in connects and binds that I took it for a hard rule. Another thing that I wasn't clear on when I posted was that I assumed that the abstract namespace used all 108 bytes of sun_path. I've tested this and you are right in that the length does infact differentiate the names. Sorry 'bout that :)

Signature · 23 lines

Mike Waychison
Sun Microsystems, Inc.
1 (650) 352-5299 voice
1 (416) 202-8336 voice
mailto: [email protected]
http://www.sun.com

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
NOTICE:  The opinions expressed in this email are held by me, 
and may not represent the views of Sun Microsystems, Inc.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 



-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Chris Friesen

author

2004-02-05 06:53 UTC #6 lore raw

In reply to Chris Friesen

Quoted 6 lines · Kay Sievers

You are talking about complexity, but what is about speed?
blocking daemon: time for i in seq 1 100; do SEQNUM=$i ACTIONd
DEVPATH=$i ./udevsend x; done real 0m1.175s user 0m0.054s sys
0m0.081s

Quoted 6 lines

threading daemon: time for i in seq 1 100; do SEQNUM=$i ACTIONd
DEVPATH=$i ./udevsend x; done real 0m0.048s user 0m0.018s sys
0m0.029s
I don't see what we get with the blocking I/O. It seems more like personal taste than numbers. :)

Interesting numbers. I instrumented the code in udevd using the x86 tsc and got the following amounts of time to receive and queue all the requests:

blocking recv() and datagrams: .13-.18 ms
threaded and streams: .05-.18 ms

I'm curious why my blocking recv() times are roughly on par with the threaded numbers, while yours are much worse.

When I time how long it takes until all the requests are satisfied (i.e. run_threads() has run 100 times), its 4-4.8 seconds either way--it doesn't seem to matter. Looks like this is due to the size of EVENT_TIMEOUT_SEC more than anything else.

Note that these numbers are with a 1.6GHz Athlon XP, 2.6.1 kernel, and linuxthreads rather than NPTL. (I know, I need to get NTPL going....)

I'm playing with a single-threaded version. I'll let you know if it amounts to anything.

Chris