* Re: [UDEV, PATCH] Add profiling support to Makefile
2005-02-13 16:16 [UDEV, PATCH] Add profiling support to Makefile Michael Buesch
2005-02-15 3:54 ` Greg KH
@ 2005-02-15 8:45 ` Michael Buesch
2005-02-15 9:12 ` Kay Sievers
2005-02-15 23:10 ` Greg KH
3 siblings, 0 replies; 5+ messages in thread
From: Michael Buesch @ 2005-02-15 8:45 UTC (permalink / raw)
To: linux-hotplug
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [UDEV, PATCH] Add profiling support to Makefile
2005-02-13 16:16 [UDEV, PATCH] Add profiling support to Makefile Michael Buesch
2005-02-15 3:54 ` Greg KH
2005-02-15 8:45 ` Michael Buesch
@ 2005-02-15 9:12 ` Kay Sievers
2005-02-15 23:10 ` Greg KH
3 siblings, 0 replies; 5+ messages in thread
From: Kay Sievers @ 2005-02-15 9:12 UTC (permalink / raw)
To: linux-hotplug
[-- 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;
}
^ permalink raw reply [flat|nested] 5+ messages in thread