22.02.2016 17:02, Leif Lindholm пишет:
> On Mon, Feb 22, 2016 at 10:57:24AM +0300, Andrei Borzenkov wrote:
>> 19.02.2016 19:18, Leif Lindholm пишет:
>>> Some disk types have allocation requirements beyond normal grub_malloc.
>>> Add a function pointer to grub_disk_t and a wrapper function in
>>> kern/disk.c making use of that function if available, to enable these
>>> disk drivers to implement their own malloc.
>>
>> The problem is not (only) grub_disk_read_small(), but this part in
>> grub_disk_read:
>>
>> if (agglomerate)
>> {
>> grub_disk_addr_t i;
>>
>> err = (disk->dev->read) (disk, transform_sector (disk, sector),
>> agglomerate << (GRUB_DISK_CACHE_BITS
>> + GRUB_DISK_SECTOR_BITS
>> - disk->log_sector_size),
>> buf);
>>
>> which reads directly into user supplied buffer. May be we can allocate
>> contiguous cache block here but put pointers to multiple chunks inside
>> it. Possible implementation is to have second layer of reference counted
>> memory blocks with cache entries containing pointer + offset into them.
>
> Whoops!
>
> Understood.
>
> So how about merging the two concepts?
> Including a patch to go with (after) the previous two to catch any
> remaining unaligned accesses in grub_efidisk_readwrite().
> With this applied, I get no fixups from a normal Linux boot (linux +
> initrd), but see them when exploring filesystems from the command
> line.
>
> Whilst a bit clunky, this seems much short-term preferable to going
> back and redesigning the disk subsystem to understand that alignment
> matters. Although given the number of exceptions we seem to be
> amassing, that does not sound like a bad idea for future.
>
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.
From: Andrei Borzenkov <arvidj...@gmail.com>
Subject: [PATCH] disk: read into cache directly
<patch description>
---
grub-core/kern/disk.c | 94 ++++++++++++++++++++++++++++++---------------------
grub-core/lib/disk.c | 6 ++--
include/grub/disk.h | 11 +++++-
3 files changed, 69 insertions(+), 42 deletions(-)
diff --git a/grub-core/kern/disk.c b/grub-core/kern/disk.c
index 789f8c0..945c146 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,35 @@ 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;
+ 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 +371,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 +392,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 +496,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
