24.02.2016 16:57, Leif Lindholm пишет:
> On Wed, Feb 24, 2016 at 03:09:13PM +0300, Andrei Borzenkov wrote:
>>>> Could you test attached patch with your alignment fixes on top. This
>>>> implements my idea of using shared buffers. Seems to work in naive testing.
>>>
>>> Testing this with a grub-mkstandalone image, I get:
>>>
>>> kern/dl.c:556: flushing 0x10f1 bytes at 0x9ffb5ac20
>>> kern/dl.c:649: module name: tar
>>> kern/dl.c:650: init function: 0x9ffb5b220
>>> kern/disk.c:217: Opening `memdisk'...
>>> kern/fs.c:56: Detecting tarfs...
>>>
>>> And then spectacular crash in UEFI due to an EL2 translation fault.
>>
>> To be sure - is it with my patch alone or with your patches? If some
>> more patches are used - could you send exact diff to trunk to avoid
>> misunderstanding?
>
> Double checked with only your patch on top of
> 1b782e902e69516f82158203674d4951a40c82d4 (previously running with
> _only_ my alignment fixup in efidisk.c). Same behaviour.
I cannot reproduce it on x86_64 (also with mm-debug enabled) and I do
not know how to load standalone image on ppc; is it possible to use QEMU
to run ARM64 (I assume you have it)? If not what are options to test it?
Anyway, there was one problem I fixed later (although I did not get any
issue before as well), I attach updated version. Thank you for testing!
From: Andrei Borzenkov <arvidj...@gmail.com>
Subject: [PATCH] disk: read into cache directly
<patch description>
---
grub-core/kern/disk.c | 95 ++++++++++++++++++++++++++++++---------------------
grub-core/lib/disk.c | 6 ++--
include/grub/disk.h | 11 +++++-
3 files changed, 70 insertions(+), 42 deletions(-)
diff --git a/grub-core/kern/disk.c b/grub-core/kern/disk.c
index 789f8c0..04ab2ab 100644
--- a/grub-core/kern/disk.c
+++ b/grub-core/kern/disk.c
@@ -56,6 +56,20 @@ grub_err_t (*grub_disk_write_weak) (grub_disk_t disk,
#include "disk_common.c"
void
+grub_disk_cache_free (struct grub_cache_buffer *buf)
+{
+ if (!buf->count)
+ /* FIXME This means corruption; what can we do? */
+ return;
+
+ if (!--buf->count)
+ {
+ grub_free (buf->data);
+ grub_free (buf);
+ }
+}
+
+void
grub_disk_cache_invalidate_all (void)
{
unsigned i;
@@ -64,10 +78,10 @@ grub_disk_cache_invalidate_all (void)
{
struct grub_disk_cache *cache = grub_disk_cache_table + i;
- if (cache->data && ! cache->lock)
+ if (cache->buffer && ! cache->lock)
{
- grub_free (cache->data);
- cache->data = 0;
+ grub_disk_cache_free (cache->buffer);
+ cache->buffer = 0;
}
}
}
@@ -82,14 +96,14 @@ grub_disk_cache_fetch (unsigned long dev_id, unsigned long disk_id,
cache_index = grub_disk_cache_get_index (dev_id, disk_id, sector);
cache = grub_disk_cache_table + cache_index;
- if (cache->dev_id == dev_id && cache->disk_id == disk_id
+ if (cache->buffer && cache->dev_id == dev_id && cache->disk_id == disk_id
&& cache->sector == sector)
{
cache->lock = 1;
#if DISK_CACHE_STATS
grub_disk_cache_hits++;
#endif
- return cache->data;
+ return cache->buffer->data + cache->offset;
}
#if DISK_CACHE_STATS
@@ -116,28 +130,36 @@ grub_disk_cache_unlock (unsigned long dev_id, unsigned long disk_id,
static grub_err_t
grub_disk_cache_store (unsigned long dev_id, unsigned long disk_id,
- grub_disk_addr_t sector, const char *data)
+ grub_disk_addr_t sector, grub_disk_addr_t n, char *data)
{
- unsigned cache_index;
- struct grub_disk_cache *cache;
+ struct grub_cache_buffer *buf;
+ grub_addr_t offset;
- cache_index = grub_disk_cache_get_index (dev_id, disk_id, sector);
- cache = grub_disk_cache_table + cache_index;
+ buf = grub_malloc (sizeof (*buf));
+ if (! buf)
+ return grub_errno;
+ buf->data = data;
+ buf->count = 0;
- cache->lock = 1;
- grub_free (cache->data);
- cache->data = 0;
- cache->lock = 0;
+ for (offset = 0 ; n > 0; sector += GRUB_DISK_CACHE_SIZE, offset += (GRUB_DISK_CACHE_SIZE * GRUB_DISK_SECTOR_SIZE), n--)
+ {
+ unsigned cache_index;
+ struct grub_disk_cache *cache;
- cache->data = grub_malloc (GRUB_DISK_SECTOR_SIZE << GRUB_DISK_CACHE_BITS);
- if (! cache->data)
- return grub_errno;
+ cache_index = grub_disk_cache_get_index (dev_id, disk_id, sector);
+ cache = grub_disk_cache_table + cache_index;
- grub_memcpy (cache->data, data,
- GRUB_DISK_SECTOR_SIZE << GRUB_DISK_CACHE_BITS);
- cache->dev_id = dev_id;
- cache->disk_id = disk_id;
- cache->sector = sector;
+ cache->lock = 1;
+ if (cache->buffer)
+ grub_disk_cache_free (cache->buffer);
+ cache->buffer = buf;
+ cache->offset = offset;
+ buf->count++;
+ cache->lock = 0;
+ cache->dev_id = dev_id;
+ cache->disk_id = disk_id;
+ cache->sector = sector;
+ }
return GRUB_ERR_NONE;
}
@@ -350,13 +372,11 @@ grub_disk_read_small_real (grub_disk_t disk, grub_disk_addr_t sector,
/* Copy it and store it in the disk cache. */
grub_memcpy (buf, tmp_buf + offset, size);
grub_disk_cache_store (disk->dev->id, disk->id,
- sector, tmp_buf);
- grub_free (tmp_buf);
+ sector, 1, tmp_buf);
return GRUB_ERR_NONE;
}
}
- grub_free (tmp_buf);
grub_errno = GRUB_ERR_NONE;
{
@@ -373,10 +393,6 @@ grub_disk_read_small_real (grub_disk_t disk, grub_disk_addr_t sector,
num = ((size + offset + (1ULL << (disk->log_sector_size))
- 1) >> (disk->log_sector_size));
- tmp_buf = grub_malloc (num << disk->log_sector_size);
- if (!tmp_buf)
- return grub_errno;
-
if ((disk->dev->read) (disk, transform_sector (disk, aligned_sector),
num, tmp_buf))
{
@@ -481,22 +497,25 @@ grub_disk_read (grub_disk_t disk, grub_disk_addr_t sector,
if (agglomerate)
{
- grub_disk_addr_t i;
+ void *cache = grub_malloc (agglomerate << (GRUB_DISK_CACHE_BITS + GRUB_DISK_SECTOR_BITS));
+ if (!cache)
+ return grub_errno;
err = (disk->dev->read) (disk, transform_sector (disk, sector),
agglomerate << (GRUB_DISK_CACHE_BITS
+ GRUB_DISK_SECTOR_BITS
- disk->log_sector_size),
- buf);
+ cache);
if (err)
- return err;
+ {
+ grub_free (cache);
+ return err;
+ }
- for (i = 0; i < agglomerate; i ++)
- grub_disk_cache_store (disk->dev->id, disk->id,
- sector + (i << GRUB_DISK_CACHE_BITS),
- (char *) buf
- + (i << (GRUB_DISK_CACHE_BITS
- + GRUB_DISK_SECTOR_BITS)));
+ grub_memcpy (buf, cache,
+ agglomerate << (GRUB_DISK_CACHE_BITS + GRUB_DISK_SECTOR_BITS));
+ grub_disk_cache_store (disk->dev->id, disk->id,
+ sector, agglomerate, cache);
if (disk->read_hook)
diff --git a/grub-core/lib/disk.c b/grub-core/lib/disk.c
index 0f18688..07fb117 100644
--- a/grub-core/lib/disk.c
+++ b/grub-core/lib/disk.c
@@ -42,11 +42,11 @@ grub_disk_cache_invalidate (unsigned long dev_id, unsigned long disk_id,
cache = grub_disk_cache_table + cache_index;
if (cache->dev_id == dev_id && cache->disk_id == disk_id
- && cache->sector == sector && cache->data)
+ && cache->sector == sector && cache->buffer)
{
cache->lock = 1;
- grub_free (cache->data);
- cache->data = 0;
+ grub_disk_cache_free (cache->buffer);
+ cache->buffer = 0;
cache->lock = 0;
}
}
diff --git a/include/grub/disk.h b/include/grub/disk.h
index b385af8..9e60367 100644
--- a/include/grub/disk.h
+++ b/include/grub/disk.h
@@ -233,16 +233,25 @@ grub_stop_disk_firmware (void)
}
}
+struct grub_cache_buffer
+ {
+ char *data;
+ unsigned count;
+ };
+
/* Disk cache. */
struct grub_disk_cache
{
enum grub_disk_dev_id dev_id;
unsigned long disk_id;
grub_disk_addr_t sector;
- char *data;
+ struct grub_cache_buffer *buffer;
+ grub_addr_t offset;
int lock;
};
+void EXPORT_FUNC(grub_disk_cache_free) (struct grub_cache_buffer *buf);
+
extern struct grub_disk_cache EXPORT_VAR(grub_disk_cache_table)[GRUB_DISK_CACHE_NUM];
#if defined (GRUB_UTIL)
--
tg: (bc22096..) e/disk-cache-align (depends on: master)
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
