On IRC, Vladimir expressed enthusiasm, but also concerns about possible
problems with cache coherency as a result of this patch, and would
prefer to leave it until post-1.98. I can sympathise with this and
won't push it.
Here's an updated version that fixes a regression in grub-setup by
making sure to reopen the device when the access mode (O_RDONLY vs.
O_WRONLY) changes. It also makes sure to close data->fd if it was
previously open, fixing a file descriptor leak.
This is now in my 'hostdisk-speedup' branch, and merged into
experimental.
=== added file 'ChangeLog.hostdisk-speedup'
--- ChangeLog.hostdisk-speedup 1970-01-01 00:00:00 +0000
+++ ChangeLog.hostdisk-speedup 2010-03-03 10:43:43 +0000
@@ -0,0 +1,18 @@
+2010-03-03 Colin Watson <cjwat...@ubuntu.com>
+
+ * 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 10:42:45 +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 10:42:45 +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 10:42:45 +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 10:42:45 +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 10:42:45 +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 20:03:03 +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,13 @@ struct
char *device;
} map[256];
+struct grub_util_biosdisk_data
+{
+ char *dev;
+ int access_mode;
+ int fd;
+};
+
#ifdef __linux__
/* Check if we have devfs support. */
static int
@@ -165,6 +173,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 +182,10 @@ 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->access_mode = 0;
+ data->fd = -1;
/* Get the size. */
#if defined(__MINGW32__)
@@ -254,6 +267,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 +286,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 +306,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 +336,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 +358,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 +385,35 @@ 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 &&
+ data->access_mode == (flags & O_ACCMODE))
{
- 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
+ {
+ free (data->dev);
+ if (data->fd != -1)
+ close (data->fd);
+
+ /* 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);
+
+ data->dev = xstrdup (dev);
+ data->access_mode = (flags & O_ACCMODE);
+ data->fd = fd;
+ }
if (is_partition)
sector -= disk->partition->start;
@@ -375,7 +437,26 @@ 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 &&
+ data->access_mode == (flags & O_ACCMODE))
+ {
+ grub_dprintf ("hostdisk", "reusing open device `%s'\n", data->dev);
+ fd = data->fd;
+ }
+ else
+ {
+ free (data->dev);
+ if (data->fd != -1)
+ close (data->fd);
+
+ fd = open (map[disk->id].device, flags);
+ if (fd >= 0)
+ {
+ data->dev = xstrdup (map[disk->id].device);
+ data->access_mode = (flags & O_ACCMODE);
+ data->fd = fd;
+ }
+ }
#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
if (! (sysctl_oldflags & 0x10)
@@ -535,7 +616,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 +650,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
--
Colin Watson [cjwat...@ubuntu.com]
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
