[UDEV, PATCH] Add profiling support to Makefile

[UDEV, PATCH] Add profiling support to Makefile

[Michael Buesch]

[-- Attachment #1.1: Type: text/plain, Size: 260 bytes --]

Hi,

This patch adds an option to the Makefile to enable
profiling. Profiling is only enabled, when not building
against klibc, as klibc lacks support for mcount().

Please apply. Thanks.

-- 
Regards Michael Buesch  [ http://www.tuxsoft.de.vu ]



[-- Attachment #1.2: udev_makefile_profile.diff --]
[-- Type: text/x-diff, Size: 1234 bytes --]

===== Makefile 1.232 vs edited =====
--- 1.232/Makefile	2005-02-11 20:55:00 +01:00
+++ edited/Makefile	2005-02-13 17:15:25 +01:00
@@ -25,6 +25,12 @@
 # Leave this set to `false' for production use.
 DEBUG = false
 
+# Set the following to 'true' to let gcc generate
+# additional profiling code.
+# Profiling is NOT supported by klibc and disabled
+# automatically if USE_KLIBC=true
+PROFILE = false
+
 # Set this to compile with Security-Enhanced Linux support.
 USE_SELINUX = false
 
@@ -162,6 +168,10 @@
 	CFLAGS  += -DLOG
 endif
 
+ifeq ($(strip $(PROFILE)),true)
+	PROFILE_FLAGS = -pg -fno-omit-frame-pointer
+endif
+
 # if DEBUG is enabled, then we do not strip or optimize
 ifeq ($(strip $(DEBUG)),true)
 	CFLAGS  += -O1 -g -DDEBUG -D_GNU_SOURCE
@@ -185,6 +195,8 @@
 	# arch specific objects
 	ARCH_LIB_OBJS =	 $(KLIBC_DIR)/libc.a
 
+	# profiling is not supported by klibc (missing mcount() function)
+	PROFILE_FLAGS =
 
 	CRT0 = $(KLIBC_DIR)/crt0.o
 	LIBC = $(ARCH_LIB_OBJS) $(LIB_OBJS) $(CRT0)
@@ -218,6 +230,9 @@
 	LIB_OBJS = -lc
 	LDFLAGS =
 endif
+
+CFLAGS += $(PROFILE_FLAGS)
+LIB_OBJS += $(PROFILE_FLAGS)
 
 ifeq ($(strip $(USE_SELINUX)),true)
 	UDEV_OBJS += udev_selinux.o

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [UDEV, PATCH] Add profiling support to Makefile

[Greg KH]

On Sun, Feb 13, 2005 at 05:16:51PM +0100, Michael Buesch wrote:
> Hi,
> 
> This patch adds an option to the Makefile to enable
> profiling. Profiling is only enabled, when not building
> against klibc, as klibc lacks support for mcount().
> 
> Please apply. Thanks.

You forgot to update the documentation about this option, so I'll hold
off till you redo that :)

Also, what has profiling information helped you out with in udev?  Found
anything interesting?  Or are you just using it for coverage analysis
(which is a flawed thing, but that's another topic...)

thanks,

greg k-h


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&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

Re: [UDEV, PATCH] Add profiling support to Makefile

[Michael Buesch]

[-- Attachment #1.1: Type: text/plain, Size: 1052 bytes --]

Quoting Greg KH <greg@kroah.com>:
> On Sun, Feb 13, 2005 at 05:16:51PM +0100, Michael Buesch wrote:
> > Hi,
> > 
> > This patch adds an option to the Makefile to enable
> > profiling. Profiling is only enabled, when not building
> > against klibc, as klibc lacks support for mcount().
> > 
> > Please apply. Thanks.
> 
> You forgot to update the documentation about this option, so I'll hold
> off till you redo that :)

Oops. :O Here it is.

> Also, what has profiling information helped you out with in udev?  Found
> anything interesting?  Or are you just using it for coverage analysis
> (which is a flawed thing, but that's another topic...)

I did not do much testing, yet, because of missing time.
But I will do that.
My first testrun showed that the namedev matchrule function and the udevdb
are the slowest parts of the whole executable. So maybe there's some
optimization possible. But I'll look closer at it later.

> thanks,
> 
> greg k-h
> 
> 

-- 
Regards Michael Buesch  [ http://www.tuxsoft.de.vu ]



[-- Attachment #1.2: udev_profilesupport.diff --]
[-- Type: text/x-diff, Size: 1993 bytes --]

===== README 1.15 vs edited =====
--- 1.15/README	2004-10-06 00:51:23 +02:00
+++ edited/README	2005-02-15 09:40:07 +01:00
@@ -50,6 +50,13 @@
 	DEBUG
 		if set to 'true', debugging messages will be sent to the syslog
 		as udev is run.  Default value is 'false'.
+	PROFILE
+		if set to 'true', additional profiling code is generated by gcc.
+		Every run of any udev executable will generate the gmon.out file,
+		which can be examined by tools like gprof / kprof and so on.
+		Klibc does NOT support profiling. Profiling is automatically
+		disabled if USE_KLIBC=true.
+		Default value is 'false'.
 	KERNEL_DIR
 		If this is not set it will default to /lib/modules/`uname -r`/build
 		This is used if USE_KLIBC=true to find the kernel include
===== Makefile 1.232 vs edited =====
--- 1.232/Makefile	2005-02-11 20:55:00 +01:00
+++ edited/Makefile	2005-02-13 17:15:25 +01:00
@@ -25,6 +25,12 @@
 # Leave this set to `false' for production use.
 DEBUG = false
 
+# Set the following to 'true' to let gcc generate
+# additional profiling code.
+# Profiling is NOT supported by klibc and disabled
+# automatically if USE_KLIBC=true
+PROFILE = false
+
 # Set this to compile with Security-Enhanced Linux support.
 USE_SELINUX = false
 
@@ -162,6 +168,10 @@
 	CFLAGS  += -DLOG
 endif
 
+ifeq ($(strip $(PROFILE)),true)
+	PROFILE_FLAGS = -pg -fno-omit-frame-pointer
+endif
+
 # if DEBUG is enabled, then we do not strip or optimize
 ifeq ($(strip $(DEBUG)),true)
 	CFLAGS  += -O1 -g -DDEBUG -D_GNU_SOURCE
@@ -185,6 +195,8 @@
 	# arch specific objects
 	ARCH_LIB_OBJS =	 $(KLIBC_DIR)/libc.a
 
+	# profiling is not supported by klibc (missing mcount() function)
+	PROFILE_FLAGS =
 
 	CRT0 = $(KLIBC_DIR)/crt0.o
 	LIBC = $(ARCH_LIB_OBJS) $(LIB_OBJS) $(CRT0)
@@ -218,6 +230,9 @@
 	LIB_OBJS = -lc
 	LDFLAGS =
 endif
+
+CFLAGS += $(PROFILE_FLAGS)
+LIB_OBJS += $(PROFILE_FLAGS)
 
 ifeq ($(strip $(USE_SELINUX)),true)
 	UDEV_OBJS += udev_selinux.o

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [UDEV, PATCH] Add profiling support to Makefile

[Kay Sievers]

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

On Tue, 2005-02-15 at 09:45 +0100, Michael Buesch wrote:
>Quoting Greg KH <greg@kroah.com>:
>> On Sun, Feb 13, 2005 at 05:16:51PM +0100, Michael Buesch wrote:
>> > Hi,
>> > 
>> > This patch adds an option to the Makefile to enable
>> > profiling. Profiling is only enabled, when not building
>> > against klibc, as klibc lacks support for mcount().
>> > 
>> > Please apply. Thanks.
>> 
>> You forgot to update the documentation about this option, so I'll hold
>> off till you redo that :)
>
>Oops. :O Here it is.
>
>> Also, what has profiling information helped you out with in udev?  Found
>> anything interesting?  Or are you just using it for coverage analysis
>> (which is a flawed thing, but that's another topic...)
>
>I did not do much testing, yet, because of missing time.
>But I will do that.
>My first testrun showed that the namedev matchrule function and the udevdb
>are the slowest parts of the whole executable. So maybe there's some
>optimization possible. But I'll look closer at it later.

The biggest part of it seems libsysfs. A replacement of the attribute
reading functions with a simple open() gives ~20% speed increase for
udevstart on my box.

Kay 

[-- Attachment #2: udev-no-libsysfs-attr-read-01.patch --]
[-- Type: text/x-patch, Size: 8963 bytes --]

===== namedev.c 1.180 vs edited =====
--- 1.180/namedev.c	2005-02-13 22:03:06 +01:00
+++ edited/namedev.c	2005-02-15 10:02:15 +01:00
@@ -36,13 +36,16 @@
 #include "libsysfs/sysfs/libsysfs.h"
 #include "list.h"
 #include "udev.h"
+#include "udev_sysfs.h"
 #include "udev_utils.h"
 #include "udev_version.h"
 #include "logging.h"
 #include "namedev.h"
 #include "udev_db.h"
 
-static struct sysfs_attribute *find_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, char *attr);
+static int find_sysfs_attribute(struct sysfs_class_device *class_dev,
+				struct sysfs_device *sysfs_device, const char *attr,
+				char *value, size_t vlen);
 
 /* compare string with pattern (supports * ? [0-9] [!A-Z]) */
 static int strcmp_pattern(const char *p, const char *s)
@@ -182,7 +185,6 @@
 	char *spos;
 	char *rest;
 	int slen;
-	struct sysfs_attribute *tmpattr;
 	unsigned int next_free_number;
 	struct sysfs_class_device *class_dev_parent;
 
@@ -265,27 +267,26 @@
 			break;
 		case 's':
 			if (attr != NULL) {
-				tmpattr = find_sysfs_attribute(class_dev, sysfs_device, attr);
-				if (tmpattr == NULL) {
-					dbg("sysfa attribute '%s' not found", attr);
+				char value[VALUE_SIZE];
+
+				if (find_sysfs_attribute(class_dev, sysfs_device, attr, value, sizeof(value)) != 0) {
+					dbg("sysfs attribute '%s' not found", attr);
 					break;
 				}
 				/* strip trailing whitespace of matching value */
-				if (isspace(tmpattr->value[strlen(tmpattr->value)-1])) {
-					i = len = strlen(tmpattr->value);
-					while (i > 0 &&  isspace(tmpattr->value[i-1]))
+				if (isspace(value[strlen(value)-1])) {
+					i = len = strlen(value);
+					while (i > 0 &&  isspace(value[i-1]))
 						i--;
 					if (i < len) {
-						tmpattr->value[i] = '\0';
-						dbg("remove %i trailing whitespace chars from '%s'",
-							 len - i, tmpattr->value);
+						value[i] = '\0';
+						dbg("remove %i trailing whitespace chars from '%s'", len - i, value);
 					}
 				}
-				strfieldcatmax(string, tmpattr->value, maxsize);
-				dbg("substitute sysfs value '%s'", tmpattr->value);
-			} else {
+				strfieldcatmax(string, value, maxsize);
+				dbg("substitute sysfs value '%s'", value);
+			} else
 				dbg("missing attribute");
-			}
 			break;
 		case '%':
 			strfieldcatmax(string, "%", maxsize);
@@ -448,67 +449,64 @@
 	return retval;
 }
 
-static struct sysfs_attribute *find_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, char *attr)
+static int find_sysfs_attribute(struct sysfs_class_device *class_dev,
+				struct sysfs_device *sysfs_device, const char *attr,
+				char *value, size_t vlen)
 {
-	struct sysfs_attribute *tmpattr = NULL;
 	char *c;
 
-	dbg("look for device attribute '%s'", attr);
+	dbg("look for class device attribute '%s'", attr);
 	/* try to find the attribute in the class device directory */
-	tmpattr = sysfs_get_classdev_attr(class_dev, attr);
-	if (tmpattr)
+	if (sysfs_read_attr(class_dev->path, attr, value, vlen) == 0)
 		goto attr_found;
 
 	/* look in the class device directory if present */
 	if (sysfs_device) {
-		tmpattr = sysfs_get_device_attr(sysfs_device, attr);
-		if (tmpattr)
+		dbg("look for devices device attribute '%s'", attr);
+		if (sysfs_read_attr(sysfs_device->path, attr, value, vlen) == 0)
 			goto attr_found;
 	}
 
-	return NULL;
+	return -1;
 
 attr_found:
-	c = strchr(tmpattr->value, '\n');
+	c = strchr(value, '\n');
 	if (c != NULL)
 		c[0] = '\0';
 
-	dbg("found attribute '%s'", tmpattr->path);
-	return tmpattr;
+	dbg("found attribute '%s'", attr);
+	return 0;
 }
 
 static int compare_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, struct sysfs_pair *pair)
 {
-	struct sysfs_attribute *tmpattr;
+	char value[VALUE_SIZE];
 	int i;
 	int len;
 
-	if ((pair == NULL) || (pair->file[0] == '\0') || (pair->value == '\0'))
+	if ((pair == NULL) || (pair->file[0] == '\0') || (pair->value[0] == '\0'))
 		return -ENODEV;
 
-	tmpattr = find_sysfs_attribute(class_dev, sysfs_device, pair->file);
-	if (tmpattr == NULL)
+	if (find_sysfs_attribute(class_dev, sysfs_device, pair->file, value, sizeof(value)) != 0)
 		return -ENODEV;
 
 	/* strip trailing whitespace of value, if not asked to match for it */
 	if (! isspace(pair->value[strlen(pair->value)-1])) {
-		i = len = strlen(tmpattr->value);
-		while (i > 0 &&  isspace(tmpattr->value[i-1]))
+		i = len = strlen(value);
+		while (i > 0 &&  isspace(value[i-1]))
 			i--;
 		if (i < len) {
-			tmpattr->value[i] = '\0';
+			value[i] = '\0';
 			dbg("remove %i trailing whitespace chars from '%s'",
-			    len - i, tmpattr->value);
+			    len - i, value);
 		}
 	}
 
-	dbg("compare attribute '%s' value '%s' with '%s'",
-		  pair->file, tmpattr->value, pair->value);
-	if (strcmp_pattern(pair->value, tmpattr->value) != 0)
+	dbg("compare attribute '%s' value '%s' with '%s'", pair->file, value, pair->value);
+	if (strcmp_pattern(pair->value, value) != 0)
 		return -ENODEV;
 
-	dbg("found matching attribute '%s' with value '%s'",
-	    pair->file, pair->value);
+	dbg("found matching attribute '%s' with value '%s'", pair->file, pair->value);
 	return 0;
 }
 
===== udev_add.c 1.93 vs edited =====
--- 1.93/udev_add.c	2005-02-13 22:03:06 +01:00
+++ edited/udev_add.c	2005-02-15 10:02:15 +01:00
@@ -38,6 +38,7 @@
 
 #include "libsysfs/sysfs/libsysfs.h"
 #include "udev.h"
+#include "udev_sysfs.h"
 #include "udev_utils.h"
 #include "udev_version.h"
 #include "logging.h"
@@ -51,14 +52,13 @@
  */
 static int get_major_minor(struct sysfs_class_device *class_dev, struct udevice *udev)
 {
-	struct sysfs_attribute *attr = NULL;
+	char value[VALUE_SIZE];
 
-	attr = sysfs_get_classdev_attr(class_dev, "dev");
-	if (attr == NULL)
+	if (sysfs_read_attr(class_dev->path, "dev", value, sizeof(value)) != 0)
 		goto error;
-	dbg("dev='%s'", attr->value);
+	dbg("dev='%s'", value);
 
-	if (sscanf(attr->value, "%u:%u", &udev->major, &udev->minor) != 2)
+	if (sscanf(value, "%u:%u", &udev->major, &udev->minor) != 2)
 		goto error;
 	dbg("found major=%d, minor=%d", udev->major, udev->minor);
 
@@ -195,13 +195,12 @@
 
 	/* create all_partitions if requested */
 	if (udev->partitions) {
-		struct sysfs_attribute *attr;
+		char value[VALUE_SIZE];
 		int range;
 
 		/* take the maximum registered minor range */
-		attr = sysfs_get_classdev_attr(class_dev, "range");
-		if (attr) {
-			range = atoi(attr->value);
+		if (sysfs_read_attr(class_dev->path, "range", value, sizeof(value)) == 0) {
+			range = atoi(value);
 			if (range > 1)
 				udev->partitions = range-1;
 		}
===== udev_sysfs.c 1.22 vs edited =====
--- 1.22/udev_sysfs.c	2005-02-04 18:38:55 +01:00
+++ edited/udev_sysfs.c	2005-02-15 10:02:15 +01:00
@@ -26,6 +26,7 @@
 #include <string.h>
 #include <ctype.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <sys/stat.h>
 
 #include "libsysfs/sysfs/libsysfs.h"
@@ -33,6 +34,27 @@
 #include "udev_sysfs.h"
 #include "udev_utils.h"
 #include "logging.h"
+
+int sysfs_read_attr(const char *path, const char *attr, char *value, size_t vlen)
+{
+	char file[SYSFS_PATH_MAX];
+	int  fd;
+	ssize_t len;
+
+	snprintf(file, SYSFS_PATH_MAX, "%s/%s", path, attr);
+	file[SYSFS_PATH_MAX-1] = '\0';
+
+	fd = open(file, O_RDONLY);
+	if (fd <0)
+		return -1;
+
+	len = read(fd, value, vlen);
+	if (len <= 0)
+		return -1;
+	value[len-1] = '\0';
+
+	return 0;
+}
 
 /* list of subsystem specific files, NULL if there is no file to wait for */
 static const struct subsystem_file {
===== udev_sysfs.h 1.2 vs edited =====
--- 1.2/udev_sysfs.h	2004-11-13 04:36:46 +01:00
+++ edited/udev_sysfs.h	2005-02-15 10:02:16 +01:00
@@ -27,6 +27,9 @@
 #define WAIT_MAX_SECONDS		5
 #define WAIT_LOOP_PER_SECOND		20
 
+/* libsysfs overkill replacement */
+extern int sysfs_read_attr(const char *path, const char *attr, char *value, size_t vlen);
+
 extern int subsystem_expect_no_dev(const char *subsystem);
 
 /* /sys/class /sys/block devices */
===== libsysfs/sysfs_utils.c 1.14 vs edited =====
--- 1.14/libsysfs/sysfs_utils.c	2004-10-20 05:15:26 +02:00
+++ edited/libsysfs/sysfs_utils.c	2005-02-15 10:02:16 +01:00
@@ -109,20 +109,25 @@
  */
 int sysfs_get_mnt_path(char *mnt_path, size_t len)
 {
-	char *sysfs_path = NULL;
+	static char path[SYSFS_PATH_MAX] = "";
+	char *sysfs_path;
 	int ret = 0;
 
 	if (mnt_path == NULL || len == 0) {
 		errno = EINVAL;
 		return -1;
 	}
-	sysfs_path = getenv(SYSFS_PATH_ENV);
-	if (sysfs_path != NULL) {
-		safestrcpymax(mnt_path, sysfs_path, len);
-		if ((sysfs_remove_trailing_slash(mnt_path)) != 0)
-			return 1;
-	} else
-		ret = sysfs_get_fs_mnt_path(SYSFS_FSTYPE_NAME, mnt_path, len);
+
+	if (path[0] == '\0') {
+		sysfs_path = getenv(SYSFS_PATH_ENV);
+		if (sysfs_path != NULL) {
+			safestrcpy(path, sysfs_path);
+			if ((sysfs_remove_trailing_slash(path)) != 0)
+				return 1;
+		} else
+			ret = sysfs_get_fs_mnt_path(SYSFS_FSTYPE_NAME, path, sizeof(path));
+	}
+	safestrcpymax(mnt_path, path, len);
 
 	return ret;
 }

Re: [UDEV, PATCH] Add profiling support to Makefile

[Greg KH]

On Tue, Feb 15, 2005 at 10:12:56AM +0100, Kay Sievers wrote:
> On Tue, 2005-02-15 at 09:45 +0100, Michael Buesch wrote:
> >Quoting Greg KH <greg@kroah.com>:
> >> On Sun, Feb 13, 2005 at 05:16:51PM +0100, Michael Buesch wrote:
> >> > Hi,
> >> > 
> >> > This patch adds an option to the Makefile to enable
> >> > profiling. Profiling is only enabled, when not building
> >> > against klibc, as klibc lacks support for mcount().
> >> > 
> >> > Please apply. Thanks.
> >> 
> >> You forgot to update the documentation about this option, so I'll hold
> >> off till you redo that :)
> >
> >Oops. :O Here it is.
> >
> >> Also, what has profiling information helped you out with in udev?  Found
> >> anything interesting?  Or are you just using it for coverage analysis
> >> (which is a flawed thing, but that's another topic...)
> >
> >I did not do much testing, yet, because of missing time.
> >But I will do that.
> >My first testrun showed that the namedev matchrule function and the udevdb
> >are the slowest parts of the whole executable. So maybe there's some
> >optimization possible. But I'll look closer at it later.
> 
> The biggest part of it seems libsysfs. A replacement of the attribute
> reading functions with a simple open() gives ~20% speed increase for
> udevstart on my box.

As I doubt libsysfs will be fixed in this regard anytime soon, want to
add this to the tree?  A 20% speed increase for such a simple change is
a very good thing to have :)

thanks,

greg k-h


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&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