* [PATCH 1/7] Add common initialization code for struct device
2011-11-25 9:59 [PATCH 0/7] Reduce system calls Zdenek Kabelac
@ 2011-11-25 9:59 ` Zdenek Kabelac
2011-11-25 9:59 ` [PATCH 2/7] Add caching of BLKGETSIZE64 ioctl Zdenek Kabelac
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Zdenek Kabelac @ 2011-11-25 9:59 UTC (permalink / raw)
To: lvm-devel
Avoid duplicate code and add _dev_init() where all common
member values are initialized.
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
lib/device/dev-cache.c | 34 +++++++++++++++++-----------------
1 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c
index 3c44333..22e5cce 100644
--- a/lib/device/dev-cache.c
+++ b/lib/device/dev-cache.c
@@ -54,6 +54,19 @@ static struct {
static int _insert(const char *path, int rec, int check_with_udev_db);
+static void _dev_init(struct device *dev)
+{
+ dev->block_size = -1;
+ dev->end = UINT64_C(0);
+ dev->fd = -1;
+ dev->max_error_count = dev_disable_after_error_count();
+ dev->open_count = 0;
+ dev->read_ahead = -1;
+ memset(dev->pvid, 0, sizeof(dev->pvid));
+ dm_list_init(&dev->aliases);
+ dm_list_init(&dev->open_list);
+}
+
struct device *dev_create_file(const char *filename, struct device *dev,
struct str_list *alias, int use_malloc)
{
@@ -97,19 +110,12 @@ struct device *dev_create_file(const char *filename, struct device *dev,
return NULL;
}
- dev->flags |= DEV_REGULAR;
- dm_list_init(&dev->aliases);
+ _dev_init(dev);
+ dev->flags = DEV_REGULAR;
dm_list_add(&dev->aliases, &alias->list);
- dev->end = UINT64_C(0);
dev->dev = 0;
- dev->fd = -1;
- dev->open_count = 0;
dev->error_count = 0;
dev->max_error_count = NO_DEV_ERROR_COUNT_LIMIT;
- dev->block_size = -1;
- dev->read_ahead = -1;
- memset(dev->pvid, 0, sizeof(dev->pvid));
- dm_list_init(&dev->open_list);
return dev;
}
@@ -122,17 +128,11 @@ static struct device *_dev_create(dev_t d)
log_error("struct device allocation failed");
return NULL;
}
+
+ _dev_init(dev);
dev->flags = 0;
- dm_list_init(&dev->aliases);
dev->dev = d;
- dev->fd = -1;
- dev->open_count = 0;
dev->max_error_count = dev_disable_after_error_count();
- dev->block_size = -1;
- dev->read_ahead = -1;
- dev->end = UINT64_C(0);
- memset(dev->pvid, 0, sizeof(dev->pvid));
- dm_list_init(&dev->open_list);
return dev;
}
--
1.7.7.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/7] Add caching of BLKGETSIZE64 ioctl
2011-11-25 9:59 [PATCH 0/7] Reduce system calls Zdenek Kabelac
2011-11-25 9:59 ` [PATCH 1/7] Add common initialization code for struct device Zdenek Kabelac
@ 2011-11-25 9:59 ` Zdenek Kabelac
2011-11-25 9:59 ` [PATCH 3/7] Do not drop cache block_size for cached device Zdenek Kabelac
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Zdenek Kabelac @ 2011-11-25 9:59 UTC (permalink / raw)
To: lvm-devel
Do not open/ioctl/close same device multiple times,
when its in device cache.
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
lib/device/dev-cache.c | 1 +
lib/device/dev-io.c | 27 ++++++++++++++++-----------
lib/device/device.h | 1 +
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c
index 22e5cce..61f479d 100644
--- a/lib/device/dev-cache.c
+++ b/lib/device/dev-cache.c
@@ -56,6 +56,7 @@ static int _insert(const char *path, int rec, int check_with_udev_db);
static void _dev_init(struct device *dev)
{
+ dev->blkgetsize64 = -1;
dev->block_size = -1;
dev->end = UINT64_C(0);
dev->fd = -1;
diff --git a/lib/device/dev-io.c b/lib/device/dev-io.c
index 9b486a9..46b90b5 100644
--- a/lib/device/dev-io.c
+++ b/lib/device/dev-io.c
@@ -252,21 +252,26 @@ static int _dev_get_size_dev(const struct device *dev, uint64_t *size)
int fd;
const char *name = dev_name(dev);
- if ((fd = open(name, O_RDONLY)) < 0) {
- log_sys_error("open", name);
- return 0;
- }
+ if (dev->blkgetsize64 == -1) {
+ if ((fd = open(name, O_RDONLY)) < 0) {
+ log_sys_error("open", name);
+ return 0;
+ }
+
+ if (ioctl(fd, BLKGETSIZE64, &dev->blkgetsize64) < 0) {
+ log_sys_error("ioctl BLKGETSIZE64", name);
+ if (close(fd))
+ log_sys_error("close", name);
+ return 0;
+ }
- if (ioctl(fd, BLKGETSIZE64, size) < 0) {
- log_sys_error("ioctl BLKGETSIZE64", name);
if (close(fd))
log_sys_error("close", name);
- return 0;
- }
- *size >>= BLKSIZE_SHIFT; /* Convert to sectors */
- if (close(fd))
- log_sys_error("close", name);
+ } else
+ log_debug("Using cached size for device %s.", name);
+
+ *size = dev->blkgetsize64 >> BLKSIZE_SHIFT; /* Convert to sectors */
log_very_verbose("%s: size is %" PRIu64 " sectors", name, *size);
diff --git a/lib/device/device.h b/lib/device/device.h
index 8c32a03..1d6d27d 100644
--- a/lib/device/device.h
+++ b/lib/device/device.h
@@ -45,6 +45,7 @@ struct device {
int read_ahead;
uint32_t flags;
uint64_t end;
+ uint64_t blkgetsize64;
struct dm_list open_list;
char pvid[ID_LEN + 1];
--
1.7.7.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/7] Do not drop cache block_size for cached device.
2011-11-25 9:59 [PATCH 0/7] Reduce system calls Zdenek Kabelac
2011-11-25 9:59 ` [PATCH 1/7] Add common initialization code for struct device Zdenek Kabelac
2011-11-25 9:59 ` [PATCH 2/7] Add caching of BLKGETSIZE64 ioctl Zdenek Kabelac
@ 2011-11-25 9:59 ` Zdenek Kabelac
2011-11-25 9:59 ` [PATCH 4/7] Do not lstat common path prefix Zdenek Kabelac
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Zdenek Kabelac @ 2011-11-25 9:59 UTC (permalink / raw)
To: lvm-devel
When we read i.e. read_ahead value - we open/close device,
which will cause dropping and rechecking device for the
block_size value.
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
lib/device/dev-io.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/lib/device/dev-io.c b/lib/device/dev-io.c
index 46b90b5..bf30fe0 100644
--- a/lib/device/dev-io.c
+++ b/lib/device/dev-io.c
@@ -587,7 +587,6 @@ static void _close(struct device *dev)
if (close(dev->fd))
log_sys_error("close", dev_name(dev));
dev->fd = -1;
- dev->block_size = -1;
dm_list_del(&dev->open_list);
log_debug("Closed %s", dev_name(dev));
--
1.7.7.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/7] Do not lstat common path prefix
2011-11-25 9:59 [PATCH 0/7] Reduce system calls Zdenek Kabelac
` (2 preceding siblings ...)
2011-11-25 9:59 ` [PATCH 3/7] Do not drop cache block_size for cached device Zdenek Kabelac
@ 2011-11-25 9:59 ` Zdenek Kabelac
2011-11-25 9:59 ` [PATCH 5/7] Minor cleanup of strchr analyzer warning Zdenek Kabelac
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Zdenek Kabelac @ 2011-11-25 9:59 UTC (permalink / raw)
To: lvm-devel
When both path have common prefix i.e. /dev/disk/by-id
skip 2xlstat for /dev /dev/disk /dev/disk/by-id
and directly lstat only different part of path.
Reduces greatly amount of lstat calls on system with
lots of devices.
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
lib/device/dev-cache.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c
index 61f479d..9b401ee 100644
--- a/lib/device/dev-cache.c
+++ b/lib/device/dev-cache.c
@@ -266,6 +266,8 @@ static int _compare_paths(const char *path0, const char *path1)
strncpy(p1, path1, PATH_MAX);
s0 = &p0[0] + 1;
s1 = &p1[0] + 1;
+ while (*s0 && *s0 == *s1)
+ s0++, s1++;
/* We prefer symlinks - they exist for a reason!
* So we prefer a shorter path before the first symlink in the name.
--
1.7.7.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 5/7] Minor cleanup of strchr analyzer warning
2011-11-25 9:59 [PATCH 0/7] Reduce system calls Zdenek Kabelac
` (3 preceding siblings ...)
2011-11-25 9:59 ` [PATCH 4/7] Do not lstat common path prefix Zdenek Kabelac
@ 2011-11-25 9:59 ` Zdenek Kabelac
2011-11-25 9:59 ` [PATCH 6/7] Drop extra stat before open of device Zdenek Kabelac
2011-11-25 9:59 ` [PATCH 7/7] Skip double stat() call with each _insert Zdenek Kabelac
6 siblings, 0 replies; 8+ messages in thread
From: Zdenek Kabelac @ 2011-11-25 9:59 UTC (permalink / raw)
To: lvm-devel
While the code knows, path0 and path1 has the same amount of '/'
otherwise the shorter path wins - the static analyzer is confused.
So just add extra check also for 's1' strchr to clean away warning
about unchecked strchr().
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
lib/device/dev-cache.c | 18 ++++++++----------
1 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c
index 9b401ee..161bff7 100644
--- a/lib/device/dev-cache.c
+++ b/lib/device/dev-cache.c
@@ -264,8 +264,8 @@ static int _compare_paths(const char *path0, const char *path1)
strncpy(p0, path0, PATH_MAX);
strncpy(p1, path1, PATH_MAX);
- s0 = &p0[0] + 1;
- s1 = &p1[0] + 1;
+ s0 = p0 + 1;
+ s1 = p1 + 1;
while (*s0 && *s0 == *s1)
s0++, s1++;
@@ -273,12 +273,11 @@ static int _compare_paths(const char *path0, const char *path1)
* So we prefer a shorter path before the first symlink in the name.
* FIXME Configuration option to invert this? */
while (s0) {
- s0 = strchr(s0, '/');
- s1 = strchr(s1, '/');
- if (s0) {
+ if ((s0 = strchr(s0, '/')))
*s0 = '\0';
+ if ((s1 = strchr(s1, '/')))
*s1 = '\0';
- }
+
if (lstat(p0, &stat0)) {
log_sys_very_verbose("lstat", p0);
return 1;
@@ -291,10 +290,9 @@ static int _compare_paths(const char *path0, const char *path1)
return 0;
if (!S_ISLNK(stat0.st_mode) && S_ISLNK(stat1.st_mode))
return 1;
- if (s0) {
- *s0++ = '/';
- *s1++ = '/';
- }
+
+ if (s0) *s0++ = '/';
+ if (s1) *s1++ = '/';
}
/* ASCII comparison */
--
1.7.7.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 6/7] Drop extra stat before open of device
2011-11-25 9:59 [PATCH 0/7] Reduce system calls Zdenek Kabelac
` (4 preceding siblings ...)
2011-11-25 9:59 ` [PATCH 5/7] Minor cleanup of strchr analyzer warning Zdenek Kabelac
@ 2011-11-25 9:59 ` Zdenek Kabelac
2011-11-25 9:59 ` [PATCH 7/7] Skip double stat() call with each _insert Zdenek Kabelac
6 siblings, 0 replies; 8+ messages in thread
From: Zdenek Kabelac @ 2011-11-25 9:59 UTC (permalink / raw)
To: lvm-devel
Use the cached value of stat() and dropping extra call for 'stat()'
before calling 'open()' and expect, that if device has been changed,
it would be catched with the 'fstat()' call that follows the 'open()'
call. If the device has dissappered 'open()' will fail.
This safes one extra 'stat()' call per each device.
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
lib/device/dev-io.c | 11 -----------
1 files changed, 0 insertions(+), 11 deletions(-)
diff --git a/lib/device/dev-io.c b/lib/device/dev-io.c
index bf30fe0..950e197 100644
--- a/lib/device/dev-io.c
+++ b/lib/device/dev-io.c
@@ -454,17 +454,6 @@ int dev_open_flags(struct device *dev, int flags, int direct, int quiet)
else if (!(name = dev_name_confirmed(dev, quiet)))
return_0;
- if (!(dev->flags & DEV_REGULAR)) {
- if (stat(name, &buf) < 0) {
- log_sys_error("%s: stat failed", name);
- return 0;
- }
- if (buf.st_rdev != dev->dev) {
- log_error("%s: device changed", name);
- return 0;
- }
- }
-
#ifdef O_DIRECT_SUPPORT
if (direct) {
if (!(dev->flags & DEV_O_DIRECT_TESTED))
--
1.7.7.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 7/7] Skip double stat() call with each _insert
2011-11-25 9:59 [PATCH 0/7] Reduce system calls Zdenek Kabelac
` (5 preceding siblings ...)
2011-11-25 9:59 ` [PATCH 6/7] Drop extra stat before open of device Zdenek Kabelac
@ 2011-11-25 9:59 ` Zdenek Kabelac
6 siblings, 0 replies; 8+ messages in thread
From: Zdenek Kabelac @ 2011-11-25 9:59 UTC (permalink / raw)
To: lvm-devel
When the device is inserted in dev_name_confirmed() stat() is
called twice as _insert() has it's own stat() call.
Extend _insert() parameter with struct stat* - which could be reused
if it has been just obtained. Udev path passes NULL and code is
doing then its own stat() as before.
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
lib/device/dev-cache.c | 48 ++++++++++++++++++++++++++++++------------------
1 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c
index 161bff7..9abaf1c 100644
--- a/lib/device/dev-cache.c
+++ b/lib/device/dev-cache.c
@@ -52,7 +52,8 @@ static struct {
#define _free(x) dm_pool_free(_cache.mem, (x))
#define _strdup(x) dm_pool_strdup(_cache.mem, (x))
-static int _insert(const char *path, int rec, int check_with_udev_db);
+static int _insert(const char *path, const struct stat *info,
+ int rec, int check_with_udev_db);
static void _dev_init(struct device *dev)
{
@@ -443,7 +444,7 @@ static int _insert_dir(const char *dir)
return_0;
_collapse_slashes(path);
- r &= _insert(path, 1, 0);
+ r &= _insert(path, NULL, 1, 0);
dm_free(path);
free(dirent[n]);
@@ -511,11 +512,11 @@ static int _insert_udev_dir(struct udev *udev, const char *dir)
device = udev_device_new_from_syspath(udev, udev_list_entry_get_name(device_entry));
node_name = udev_device_get_devnode(device);
- r &= _insert(node_name, 0, 0);
+ r &= _insert(node_name, NULL, 0, 0);
udev_list_entry_foreach(symlink_entry, udev_device_get_devlinks_list_entry(device)) {
symlink_name = udev_list_entry_get_name(symlink_entry);
- r &= _insert(symlink_name, 0, 0);
+ r &= _insert(symlink_name, NULL, 0, 0);
}
udev_device_unref(device);
@@ -569,29 +570,33 @@ static void _insert_dirs(struct dm_list *dirs)
#endif /* UDEV_SYNC_SUPPORT */
-static int _insert(const char *path, int rec, int check_with_udev_db)
+static int _insert(const char *path, const struct stat *info,
+ int rec, int check_with_udev_db)
{
- struct stat info;
+ struct stat tinfo;
int r = 0;
- if (stat(path, &info) < 0) {
- log_sys_very_verbose("stat", path);
- return 0;
+ if (!info) {
+ if (stat(path, &tinfo) < 0) {
+ log_sys_very_verbose("stat", path);
+ return 0;
+ }
+ info = &tinfo;
}
- if (check_with_udev_db && !_device_in_udev_db(info.st_rdev)) {
+ if (check_with_udev_db && !_device_in_udev_db(info->st_rdev)) {
log_very_verbose("%s: Not in udev db", path);
return 0;
}
- if (S_ISDIR(info.st_mode)) { /* add a directory */
+ if (S_ISDIR(info->st_mode)) { /* add a directory */
/* check it's not a symbolic link */
- if (lstat(path, &info) < 0) {
+ if (lstat(path, &tinfo) < 0) {
log_sys_very_verbose("lstat", path);
return 0;
}
- if (S_ISLNK(info.st_mode)) {
+ if (S_ISLNK(tinfo.st_mode)) {
log_debug("%s: Symbolic link to directory", path);
return 0;
}
@@ -600,12 +605,12 @@ static int _insert(const char *path, int rec, int check_with_udev_db)
r = _insert_dir(path);
} else { /* add a device */
- if (!S_ISBLK(info.st_mode)) {
+ if (!S_ISBLK(info->st_mode)) {
log_debug("%s: Not a block device", path);
return 0;
}
- if (!_insert_dev(path, info.st_rdev))
+ if (!_insert_dev(path, info->st_rdev))
return_0;
r = 1;
@@ -865,7 +870,7 @@ const char *dev_name_confirmed(struct device *dev, int quiet)
if (dm_list_size(&dev->aliases) > 1) {
dm_list_del(dev->aliases.n);
if (!r)
- _insert(name, 0, obtain_device_list_from_udev());
+ _insert(name, &buf, 0, obtain_device_list_from_udev());
continue;
}
@@ -887,13 +892,20 @@ struct device *dev_cache_get(const char *name, struct dev_filter *f)
return d;
/* If the entry's wrong, remove it */
- if (d && (stat(name, &buf) || (buf.st_rdev != d->dev))) {
+ if (stat(name, &buf) < 0) {
+ if (d)
+ dm_hash_remove(_cache.names, name);
+ log_sys_very_verbose("stat", name);
+ return NULL;
+ }
+
+ if (d && (buf.st_rdev != d->dev)) {
dm_hash_remove(_cache.names, name);
d = NULL;
}
if (!d) {
- _insert(name, 0, obtain_device_list_from_udev());
+ _insert(name, &buf, 0, obtain_device_list_from_udev());
d = (struct device *) dm_hash_lookup(_cache.names, name);
if (!d) {
_full_scan(0);
--
1.7.7.3
^ permalink raw reply related [flat|nested] 8+ messages in thread