[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] disk: Add support for device-specific malloc function
From: |
Andrei Borzenkov |
Subject: |
Re: [PATCH 1/2] disk: Add support for device-specific malloc function |
Date: |
Wed, 24 Feb 2016 15:09:13 +0300 |
On Wed, Feb 24, 2016 at 2:59 PM, Leif Lindholm <address@hidden> wrote:
> On Mon, Feb 22, 2016 at 07:56:25PM +0300, Andrei Borzenkov wrote:
>> 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.
>
> 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?
- [PATCH 0/2] Alternative efidisk alignment resolution, Leif Lindholm, 2016/02/19
- [PATCH 1/2] disk: Add support for device-specific malloc function, Leif Lindholm, 2016/02/19
- Re: [PATCH 1/2] disk: Add support for device-specific malloc function, Andrei Borzenkov, 2016/02/22
- Re: [PATCH 1/2] disk: Add support for device-specific malloc function, Leif Lindholm, 2016/02/22
- Re: [PATCH 1/2] disk: Add support for device-specific malloc function, Andrei Borzenkov, 2016/02/22
- Re: [PATCH 1/2] disk: Add support for device-specific malloc function, Leif Lindholm, 2016/02/24
- Re: [PATCH 1/2] disk: Add support for device-specific malloc function,
Andrei Borzenkov <=
- Re: [PATCH 1/2] disk: Add support for device-specific malloc function, Leif Lindholm, 2016/02/24
- Re: [PATCH 1/2] disk: Add support for device-specific malloc function, Andrei Borzenkov, 2016/02/24
- Re: [PATCH 1/2] disk: Add support for device-specific malloc function, Andrei Borzenkov, 2016/02/24
- Re: [PATCH 1/2] disk: Add support for device-specific malloc function, Vladimir 'phcoder' Serbinenko, 2016/02/26
- Re: [PATCH 1/2] disk: Add support for device-specific malloc function, Andrei Borzenkov, 2016/02/26
- Re: [PATCH 1/2] disk: Add support for device-specific malloc function, Vladimir 'phcoder' Serbinenko, 2016/02/26
Re: [PATCH 1/2] disk: Add support for device-specific malloc function, Andrei Borzenkov, 2016/02/22
[PATCH 2/2] efidisk: respect block_io_protocol minimum buffer alignment, Leif Lindholm, 2016/02/19