* [RFC] LZO support
@ 2011-09-14 19:20 Szymon Janc
2011-09-28 21:39 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 1 reply; 4+ messages in thread
From: Szymon Janc @ 2011-09-14 19:20 UTC (permalink / raw)
To: The development of GNU GRUB
Hello,
I've implemented support for LZO in grub, this includes:
- import of minilzo library (from [1])
- support for LZO (de)compression in btrfs (compress=lzo mount option)
- lzopio - for reading lzop compressed files
Some comments:
- with this code one is able to boot from btrfs partition that was mounted
with compress=lzo, also did some tests with grub-fstest (crc, testload) on
some various files, seems to be working fine,
- lzopio works ok with grub-fstest (crc, testload), also able to load modules
compressed with lzop, but compressing all modules and doing
grub-install --modules=lzopio fails to boot, still need to investigate that
- lzopio have room for speed optimisation i.e. decompressing directly to
target buffer when possible, currently there is always memcpy from temp
buffer
- tested on 32bit x86 only, I'll try to do some tests on different platforms as
well (on QEMU) but brave people could test it on some real hardware :)
- there are also some addons which could be used by other grub code as well:
* suport for adler32 checksum (for lzop)
* helper functions for easier unaligned access
Last but not least: code is available at [2].
Comments and testing are welcome :)
[1] http://www.oberhumer.com/opensource/lzo/#minilzo
[2] http://bzr.savannah.gnu.org/r/grub/people/janc/lzo/
--
Szymon K. Janc
szymon@janc.net.pl // GG: 1383435
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] LZO support
2011-09-14 19:20 [RFC] LZO support Szymon Janc
@ 2011-09-28 21:39 ` Vladimir 'φ-coder/phcoder' Serbinenko
2011-10-04 20:11 ` Szymon Janc
0 siblings, 1 reply; 4+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-09-28 21:39 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]
On 14.09.2011 21:20, Szymon Janc wrote:
> Hello,
>
> I've implemented support for LZO in grub, this includes:
> - import of minilzo library (from [1])
> - support for LZO (de)compression in btrfs (compress=lzo mount option)
> - lzopio - for reading lzop compressed files
>
+ common = lib/minilzo/minilzo.c;
@@ -1650,6 +1653,14 @@
+ common = lib/minilzo/minilzo.c;
Please don't include same file twice in different modules it creates a
symbol conflict. It's ok to have btrfs depend on lzopio if necessary, at
least for now.
+#include "minilzo.h"
I'd prefer <minilzo.h> and an explicit -I on cppflags if necessary.
- grub_disk_addr_t *outaddr, grub_size_t *outsize,
+ grub_disk_addr_t * outaddr, grub_size_t * outsize,
this is a stylistic regression. (several such occurences)
- if (desc->data[desc->depth - 1].iter
- < desc->data[desc->depth - 1].maxiter)
+ if (desc->data[desc->depth - 1].iter <
+ desc->data[desc->depth - 1].maxiter)
Likewise.
+ grub_uint8_t context[lzopio->ccheck_fun->contextsize];
This isn't necessarily well-aligned. Use grub_uint64_t
context[(lzopio->ccheck_fun->contextsize + 7) / 8];
(several occurences)
+ file->size = GRUB_FILE_SIZE_UNKNOWN;
Several components have trouble with such files. Is there any way to
retrieve the size of lzio compressed file?
> - lzopio works ok with grub-fstest (crc, testload), also able to load modules
> compressed with lzop, but compressing all modules and doing
> grub-install --modules=lzopio fails to boot, still need to investigate that
That because dependencies on hashes are soft so it's not correctly
tracked by grub-install and so you end up in an infinite recursion.
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] LZO support
2011-09-28 21:39 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2011-10-04 20:11 ` Szymon Janc
2011-10-06 14:58 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 1 reply; 4+ messages in thread
From: Szymon Janc @ 2011-10-04 20:11 UTC (permalink / raw)
To: grub-devel; +Cc: Vladimir 'φ-coder/phcoder' Serbinenko
On Wednesday 28 September 2011 23:39:24 Vladimir 'φ-coder/phcoder' Serbinenko
wrote:
> On 14.09.2011 21:20, Szymon Janc wrote:
> > Hello,
> >
> > I've implemented support for LZO in grub, this includes:
> > - import of minilzo library (from [1])
> > - support for LZO (de)compression in btrfs (compress=lzo mount option)
> > - lzopio - for reading lzop compressed files
>
> + common = lib/minilzo/minilzo.c;
> @@ -1650,6 +1653,14 @@
> + common = lib/minilzo/minilzo.c;
> Please don't include same file twice in different modules it creates a
> symbol conflict. It's ok to have btrfs depend on lzopio if necessary, at
> least for now.
Done.
> +#include "minilzo.h"
> I'd prefer <minilzo.h> and an explicit -I on cppflags if necessary.
Done.
> - grub_disk_addr_t *outaddr, grub_size_t *outsize,
> + grub_disk_addr_t * outaddr, grub_size_t * outsize,
> this is a stylistic regression. (several such occurences)
> - if (desc->data[desc->depth - 1].iter
> - < desc->data[desc->depth - 1].maxiter)
> + if (desc->data[desc->depth - 1].iter <
> + desc->data[desc->depth - 1].maxiter)
> Likewise.
Oops. Fixed.
> + grub_uint8_t context[lzopio->ccheck_fun->contextsize];
> This isn't necessarily well-aligned. Use grub_uint64_t
> context[(lzopio->ccheck_fun->contextsize + 7) / 8];
> (several occurences)
Done.
> + file->size = GRUB_FILE_SIZE_UNKNOWN;
> Several components have trouble with such files. Is there any way to
> retrieve the size of lzio compressed file?
It is possible and it is already implemented, please see
calculate_uncompressed_size(). Actually, uncompressed size is always
calculated. There is TODO to not do it on not-easily-seekable files as done in
other ios. And GRUB_FILE_SIZE_UNKNOWN is set for 'future proof' only.
>
> > - lzopio works ok with grub-fstest (crc, testload), also able to load
> > modules
> >
> > compressed with lzop, but compressing all modules and doing
> > grub-install --modules=lzopio fails to boot, still need to investigate
> > that
>
> That because dependencies on hashes are soft so it's not correctly
> tracked by grub-install and so you end up in an infinite recursion.
Branch on savannah updated.
--
Szymon K. Janc
szymon@janc.net.pl // GG: 1383435
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] LZO support
2011-10-04 20:11 ` Szymon Janc
@ 2011-10-06 14:58 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 0 replies; 4+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-10-06 14:58 UTC (permalink / raw)
To: Szymon Janc; +Cc: grub-devel
[-- Attachment #1: Type: text/plain, Size: 2453 bytes --]
Looks good. You can commit it upstream.
On 04.10.2011 22:11, Szymon Janc wrote:
> On Wednesday 28 September 2011 23:39:24 Vladimir 'φ-coder/phcoder' Serbinenko
> wrote:
>> On 14.09.2011 21:20, Szymon Janc wrote:
>>> Hello,
>>>
>>> I've implemented support for LZO in grub, this includes:
>>> - import of minilzo library (from [1])
>>> - support for LZO (de)compression in btrfs (compress=lzo mount option)
>>> - lzopio - for reading lzop compressed files
>> + common = lib/minilzo/minilzo.c;
>> @@ -1650,6 +1653,14 @@
>> + common = lib/minilzo/minilzo.c;
>> Please don't include same file twice in different modules it creates a
>> symbol conflict. It's ok to have btrfs depend on lzopio if necessary, at
>> least for now.
> Done.
>
>> +#include "minilzo.h"
>> I'd prefer <minilzo.h> and an explicit -I on cppflags if necessary.
> Done.
>
>> - grub_disk_addr_t *outaddr, grub_size_t *outsize,
>> + grub_disk_addr_t * outaddr, grub_size_t * outsize,
>> this is a stylistic regression. (several such occurences)
>> - if (desc->data[desc->depth - 1].iter
>> - < desc->data[desc->depth - 1].maxiter)
>> + if (desc->data[desc->depth - 1].iter <
>> + desc->data[desc->depth - 1].maxiter)
>> Likewise.
> Oops. Fixed.
>
>> + grub_uint8_t context[lzopio->ccheck_fun->contextsize];
>> This isn't necessarily well-aligned. Use grub_uint64_t
>> context[(lzopio->ccheck_fun->contextsize + 7) / 8];
>> (several occurences)
> Done.
>
>> + file->size = GRUB_FILE_SIZE_UNKNOWN;
>> Several components have trouble with such files. Is there any way to
>> retrieve the size of lzio compressed file?
> It is possible and it is already implemented, please see
> calculate_uncompressed_size(). Actually, uncompressed size is always
> calculated. There is TODO to not do it on not-easily-seekable files as done in
> other ios. And GRUB_FILE_SIZE_UNKNOWN is set for 'future proof' only.
>
>>> - lzopio works ok with grub-fstest (crc, testload), also able to load
>>> modules
>>>
>>> compressed with lzop, but compressing all modules and doing
>>> grub-install --modules=lzopio fails to boot, still need to investigate
>>> that
>> That because dependencies on hashes are soft so it's not correctly
>> tracked by grub-install and so you end up in an infinite recursion.
>
> Branch on savannah updated.
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-10-06 14:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-14 19:20 [RFC] LZO support Szymon Janc
2011-09-28 21:39 ` Vladimir 'φ-coder/phcoder' Serbinenko
2011-10-04 20:11 ` Szymon Janc
2011-10-06 14:58 ` Vladimir 'φ-coder/phcoder' Serbinenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.