grub-probe filesystem reads are seriously slow right now:
$ sudo time ./grub-probe -t fs /usr/share/grub/unicode.pf2
ext2
0.12user 3.27system 0:11.75elapsed 28%CPU (0avgtext+0avgdata 24320maxresident)k
43592inputs+8outputs (0major+1572minor)pagefaults 0swaps
This is a serious problem when running update-grub, which of course runs
grub-probe several times. It's essentially due to the way hostdisk
opens and closes devices all the time, wasting lots of expensive
syscalls and probably defeating buffer caching into the bargain. With
this patch, consecutive reads from the same device don't need to reopen
that device, speeding it up by an order of magnitude:
$ sudo time ./grub-probe -t fs /usr/share/grub/unicode.pf2
ext2
0.06user 0.05system 0:00.71elapsed 15%CPU (0avgtext+0avgdata 24304maxresident)k
11120inputs+8outputs (0major+1571minor)pagefaults 0swaps
After review, is there any chance at all that this could get into 1.98?
I think I'm going to end up applying something like this in Ubuntu 10.04
anyway, but obviously I'd rather have it upstream if possible.
2010-03-03 Colin Watson <address@hidden>
* util/hostdisk.c (struct grub_util_biosdisk_data): New structure.
(grub_util_biosdisk_open): Initialise disk->data.
(struct linux_partition_cache): New structure.
(linux_find_partition): Cache partition start positions; these are
expensive to compute on every read and write.
(open_device): Cache open file descriptor in disk->data, so that we
don't have to reopen it and flush the buffer cache for consecutive
operations on the same device.
(grub_util_biosdisk_close): New function.
(grub_util_biosdisk_dev): Set `close' member.
* conf/common.rmk (grub_probe_SOURCES): Add kern/list.c.
* conf/i386-efi.rmk (grub_setup_SOURCES): Likewise.
* conf/i386-pc.rmk (grub_setup_SOURCES): Likewise.
* conf/sparc64-ieee1275.rmk (grub_setup_SOURCES): Likewise.
* conf/x86_64-efi.rmk (grub_setup_SOURCES): Likewise.
=== modified file 'conf/common.rmk'
--- conf/common.rmk 2010-02-25 14:10:18 +0000
+++ conf/common.rmk 2010-03-03 09:44:19 +0000
@@ -24,7 +24,7 @@ util/grub-probe.c_DEPENDENCIES = grub_pr
grub_probe_SOURCES = gnulib/progname.c util/grub-probe.c \
util/hostdisk.c util/misc.c util/getroot.c \
kern/device.c kern/disk.c kern/err.c kern/misc.c \
- kern/parser.c kern/partition.c kern/file.c \
+ kern/parser.c kern/partition.c kern/file.c kern/list.c \
\
fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c \
fs/hfsplus.c fs/iso9660.c fs/udf.c fs/jfs.c fs/minix.c \
=== modified file 'conf/i386-efi.rmk'
--- conf/i386-efi.rmk 2010-01-20 20:31:39 +0000
+++ conf/i386-efi.rmk 2010-03-03 09:44:31 +0000
@@ -21,7 +21,7 @@ util/i386/efi/grub-mkimage.c_DEPENDENCIE
# kern/err.c kern/misc.c fs/fat.c fs/ext2.c fs/xfs.c fs/affs.c \
# fs/sfs.c kern/parser.c kern/partition.c partmap/msdos.c \
# fs/ufs.c fs/ufs2.c fs/minix.c fs/hfs.c fs/jfs.c fs/hfsplus.c
kern/file.c \
-# kern/fs.c kern/env.c fs/fshelp.c
+# kern/fs.c kern/env.c kern/list.c fs/fshelp.c
# Scripts.
sbin_SCRIPTS = grub-install
=== modified file 'conf/i386-pc.rmk'
--- conf/i386-pc.rmk 2010-02-03 00:24:07 +0000
+++ conf/i386-pc.rmk 2010-03-03 09:44:54 +0000
@@ -96,7 +96,8 @@ grub_setup_SOURCES = gnulib/progname.c \
util/i386/pc/grub-setup.c util/hostdisk.c \
util/misc.c util/getroot.c kern/device.c kern/disk.c \
kern/err.c kern/misc.c kern/parser.c kern/partition.c \
- kern/file.c kern/fs.c kern/env.c fs/fshelp.c \
+ kern/file.c kern/fs.c kern/env.c kern/list.c \
+ fs/fshelp.c \
\
fs/affs.c fs/cpio.c fs/ext2.c fs/fat.c fs/hfs.c \
fs/hfsplus.c fs/iso9660.c fs/udf.c fs/jfs.c fs/minix.c \
=== modified file 'conf/sparc64-ieee1275.rmk'
--- conf/sparc64-ieee1275.rmk 2010-01-20 20:31:39 +0000
+++ conf/sparc64-ieee1275.rmk 2010-03-03 09:45:17 +0000
@@ -70,7 +70,8 @@ util/sparc64/ieee1275/grub-setup.c_DEPEN
grub_setup_SOURCES = util/sparc64/ieee1275/grub-setup.c util/hostdisk.c \
util/misc.c util/getroot.c kern/device.c kern/disk.c \
kern/err.c kern/misc.c kern/parser.c kern/partition.c \
- kern/file.c kern/fs.c kern/env.c fs/fshelp.c \
+ kern/file.c kern/fs.c kern/env.c kern/list.c \
+ fs/fshelp.c \
\
fs/affs.c fs/cpio.c fs/ext2.c fs/fat.c fs/hfs.c \
fs/hfsplus.c fs/iso9660.c fs/udf.c fs/jfs.c fs/minix.c \
=== modified file 'conf/x86_64-efi.rmk'
--- conf/x86_64-efi.rmk 2010-01-20 20:31:39 +0000
+++ conf/x86_64-efi.rmk 2010-03-03 09:45:29 +0000
@@ -20,7 +20,7 @@ grub_mkimage_SOURCES = gnulib/progname.c
# kern/err.c kern/misc.c fs/fat.c fs/ext2.c fs/xfs.c fs/affs.c \
# fs/sfs.c kern/parser.c kern/partition.c partmap/msdos.c \
# fs/ufs.c fs/ufs2.c fs/minix.c fs/hfs.c fs/jfs.c fs/hfsplus.c
kern/file.c \
-# kern/fs.c kern/env.c fs/fshelp.c
+# kern/fs.c kern/env.c kern/list.c fs/fshelp.c
# Scripts.
sbin_SCRIPTS = grub-install
=== modified file 'util/hostdisk.c'
--- util/hostdisk.c 2010-02-07 01:47:18 +0000
+++ util/hostdisk.c 2010-03-03 10:05:41 +0000
@@ -26,6 +26,7 @@
#include <grub/util/hostdisk.h>
#include <grub/misc.h>
#include <grub/i18n.h>
+#include <grub/list.h>
#include <stdio.h>
#include <stdlib.h>
@@ -103,6 +104,12 @@ struct
char *device;
} map[256];
+struct grub_util_biosdisk_data
+{
+ char *dev;
+ int fd;
+};
+
#ifdef __linux__
/* Check if we have devfs support. */
static int
@@ -165,6 +172,7 @@ grub_util_biosdisk_open (const char *nam
{
int drive;
struct stat st;
+ struct grub_util_biosdisk_data *data;
drive = find_grub_drive (name);
if (drive < 0)
@@ -173,6 +181,9 @@ grub_util_biosdisk_open (const char *nam
disk->has_partitions = 1;
disk->id = drive;
+ disk->data = data = xmalloc (sizeof (struct grub_util_biosdisk_data));
+ data->dev = NULL;
+ data->fd = -1;
/* Get the size. */
#if defined(__MINGW32__)
@@ -254,6 +265,17 @@ grub_util_biosdisk_open (const char *nam
}
#ifdef __linux__
+/* Cache of partition start sectors for each disk. */
+struct linux_partition_cache
+{
+ struct linux_partition_cache *next;
+ char *dev;
+ unsigned long start;
+ int partno;
+};
+
+struct linux_partition_cache *linux_partition_cache_list;
+
static int
linux_find_partition (char *dev, unsigned long sector)
{
@@ -262,6 +284,7 @@ linux_find_partition (char *dev, unsigne
char *p;
int i;
char real_dev[PATH_MAX];
+ struct linux_partition_cache *cache;
strcpy(real_dev, dev);
@@ -281,6 +304,16 @@ linux_find_partition (char *dev, unsigne
format = "%d";
}
+ for (cache = linux_partition_cache_list; cache; cache = cache->next)
+ {
+ if (strcmp (cache->dev, dev) == 0 && cache->start == sector)
+ {
+ sprintf (p, format, cache->partno);
+ strcpy (dev, real_dev);
+ return 1;
+ }
+ }
+
for (i = 1; i < 10000; i++)
{
int fd;
@@ -301,6 +334,15 @@ linux_find_partition (char *dev, unsigne
if (hdg.start == sector)
{
+ struct linux_partition_cache *new_cache_item;
+
+ new_cache_item = xmalloc (sizeof *new_cache_item);
+ new_cache_item->dev = xstrdup (dev);
+ new_cache_item->start = hdg.start;
+ new_cache_item->partno = i;
+ grub_list_push (GRUB_AS_LIST_P (&linux_partition_cache_list),
+ GRUB_AS_LIST (new_cache_item));
+
strcpy (dev, real_dev);
return 1;
}
@@ -314,6 +356,7 @@ static int
open_device (const grub_disk_t disk, grub_disk_addr_t sector, int flags)
{
int fd;
+ struct grub_util_biosdisk_data *data = disk->data;
#ifdef O_LARGEFILE
flags |= O_LARGEFILE;
@@ -340,18 +383,30 @@ open_device (const grub_disk_t disk, gru
&& strncmp (map[disk->id].device, "/dev/", 5) == 0)
is_partition = linux_find_partition (dev, disk->partition->start);
- /* Open the partition. */
- grub_dprintf ("hostdisk", "opening the device `%s' in open_device()\n",
dev);
- fd = open (dev, flags);
- if (fd < 0)
+ if (data->dev && strcmp (data->dev, dev) == 0)
{
- grub_error (GRUB_ERR_BAD_DEVICE, "cannot open `%s'", dev);
- return -1;
+ grub_dprintf ("hostdisk", "reusing open device `%s'\n", dev);
+ fd = data->fd;
}
+ else
+ {
+ /* Open the partition. */
+ grub_dprintf ("hostdisk", "opening the device `%s' in open_device()\n",
dev);
+ fd = open (dev, flags);
+ if (fd < 0)
+ {
+ grub_error (GRUB_ERR_BAD_DEVICE, "cannot open `%s'", dev);
+ return -1;
+ }
- /* Flush the buffer cache to the physical disk.
- XXX: This also empties the buffer cache. */
- ioctl (fd, BLKFLSBUF, 0);
+ /* Flush the buffer cache to the physical disk.
+ XXX: This also empties the buffer cache. */
+ ioctl (fd, BLKFLSBUF, 0);
+
+ free (data->dev);
+ data->dev = xstrdup (dev);
+ data->fd = fd;
+ }
if (is_partition)
sector -= disk->partition->start;
@@ -375,7 +430,21 @@ open_device (const grub_disk_t disk, gru
}
#endif
- fd = open (map[disk->id].device, flags);
+ if (data->dev && strcmp (data->dev, map[disk->id].device) == 0)
+ {
+ grub_dprintf ("hostdisk", "reusing open device `%s'\n", data->dev);
+ fd = data->fd;
+ }
+ else
+ {
+ fd = open (map[disk->id].device, flags);
+ if (fd >= 0)
+ {
+ free (data->dev);
+ data->dev = xstrdup (map[disk->id].device);
+ data->fd = fd;
+ }
+ }
#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
if (! (sysctl_oldflags & 0x10)
@@ -535,7 +604,6 @@ grub_util_biosdisk_read (grub_disk_t dis
!= (ssize_t) (size << GRUB_DISK_SECTOR_BITS))
grub_error (GRUB_ERR_READ_ERROR, "cannot read from `%s'",
map[disk->id].device);
- close (fd);
return grub_errno;
}
@@ -570,17 +638,27 @@ grub_util_biosdisk_write (grub_disk_t di
!= (ssize_t) (size << GRUB_DISK_SECTOR_BITS))
grub_error (GRUB_ERR_WRITE_ERROR, "cannot write to `%s'",
map[disk->id].device);
- close (fd);
return grub_errno;
}
+static void
+grub_util_biosdisk_close (struct grub_disk *disk)
+{
+ struct grub_util_biosdisk_data *data = disk->data;
+
+ free (data->dev);
+ if (data->fd != -1)
+ close (data->fd);
+ free (data);
+}
+
static struct grub_disk_dev grub_util_biosdisk_dev =
{
.name = "biosdisk",
.id = GRUB_DISK_DEVICE_BIOSDISK_ID,
.iterate = grub_util_biosdisk_iterate,
.open = grub_util_biosdisk_open,
- .close = 0,
+ .close = grub_util_biosdisk_close,
.read = grub_util_biosdisk_read,
.write = grub_util_biosdisk_write,
.next = 0
Thanks,
--
Colin Watson address@hidden
_______________________________________________
Grub-devel mailing list
address@hidden
http://lists.gnu.org/mailman/listinfo/grub-devel