9:23 p.m.
On Fri, Apr 5, 2024 at 2:04 PM <devel-request(a)lists.crash-utility.osci.io>
wrote:
Date: Fri, 5 Apr 2024 06:01:33 +0000
From: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
Subject: [Crash-utility] Re: [PATCH v4 ] Adding the zram decompression
algorithm "lzo-rle" to support kernel versions >= 5.1
To: Yulong TANG 汤玉龙 <yulong.tang(a)nio.com>,
"devel(a)lists.crash-utility.osci.io"
<devel(a)lists.crash-utility.osci.io>
Message-ID: <ce7328ee-aab4-4b72-85d5-77e5ef3aff9a(a)nec.com>
Content-Type: multipart/mixed;
boundary="_002_ce7328eeaab44b7285d577e5ef3aff9aneccom_"
Hi, Yulong
I have one question about the following function:
+static uint16_t get_unaligned_le16(const void *p) {
+ uint16_t value;
+ memcpy(&value, p, sizeof(uint16_t));
+ return value;
+}
Is this for handling the data under a little-endian machine? It might have
a different result when calling the above function on a
big-endian/little-endian machine, is that expected behavior? Just confirm
with you.
Thanks
Lianbo
4:30 a.m.
New subject: [PATCH v4 ] Adding the zram decompression algorithm "lzo-rle" to support kernel versions >= 5.1
Hi, Lianbo
Thank you for your review.
Your concerns are valid, there are some bit operations performed here after
get_unaligned_le16(), perhaps we can add a check.
But the crash-utility should match the endianess of the vmcore file and the local machine,
and whether the compression algorithm always compresses data in the same format, so may
need to be tested on a big-endian machine.
Thanks,
Yulong
1:32 a.m.
New subject: [PATCH v4 ] Adding the zram decompression algorithm "lzo-rle" to support kernel versions >= 5.1
On 2024/04/09 18:30, Yulong Tang wrote:
Hi, Lianbo
Thank you for your review.
Your concerns are valid, there are some bit operations performed here after
get_unaligned_le16(), perhaps we can add a check.
But the crash-utility should match the endianess of the vmcore file and the local
machine, and whether the compression algorithm always compresses data in the same format,
so may need to be tested on a big-endian machine.
Good catch, Lianbo.
It seems that in the kenrnel, le16_to_cpu in get_unaligned_le16 does
bswap_16 on a big-endian machine, probably we need to add something like
#if __BYTE_ORDER == __BIG_ENDIAN
return bswap_16(value);
#else
return value;
#endif
Is it possible to test on a big-endian machine?
Thanks,
Kazu
4:57 a.m.
New subject: [PATCH v4 ] Adding the zram decompression algorithm "lzo-rle" to support kernel versions >= 5.1
On Mon, Apr 15, 2024 at 2:32 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
wrote:
On 2024/04/09 18:30, Yulong Tang wrote:
> Hi, Lianbo
>
> Thank you for your review.
>
> Your concerns are valid, there are some bit operations performed here
after get_unaligned_le16(), perhaps we can add a check.
>
> But the crash-utility should match the endianess of the vmcore file and
the local machine, and whether the compression algorithm always compresses
data in the same format, so may need to be tested on a big-endian machine.
Good catch, Lianbo.
It seems that in the kenrnel, le16_to_cpu in get_unaligned_le16 does
bswap_16 on a big-endian machine, probably we need to add something like
#if __BYTE_ORDER == __BIG_ENDIAN
return bswap_16(value);
#else
return value;
#endif
Is it possible to test on a big-endian machine?
Sure. The macro '#if __BYTE_ORDER == __BIG_ENDIAN' takes effect on a
big-endian machine. But it requires to include the header file 'byteswap.h'
as below:
#include <byteswap.h>
In addition, there is a similar implementation, but I haven't tested it
with Yulong's case. Can you help to check if this can work well for you?
Yulong.
static inline uint16_t
get_unaligned_le16 (const uint8_t *p)
{
return p[0] | p[1] << 8;
}
Thank
Lianbo
Thanks,
Kazu
1:49 a.m.
New subject: [PATCH v4 ] Adding the zram decompression algorithm "lzo-rle" to support kernel versions >= 5.1
On Mon, Apr 15, 2024 at 2:32 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com>
wrote:
Sure. The macro '#if __BYTE_ORDER == __BIG_ENDIAN' takes effect on a
big-endian machine. But it requires to include the header file 'byteswap.h'
as below:
#include <byteswap.h>
In addition, there is a similar implementation, but I haven't tested it
with Yulong's case. Can you help to check if this can work well for you?
Yulong.
static inline uint16_t
get_unaligned_le16 (const uint8_t *p)
{
return p[0] | p[1] << 8;
}
Hi, Lianbo
It works well in my case(little-endian). This looks promising, but I don't have a
big-endian machine to test it on.
Thanks,
Yulong
>
> Thank
> Lianbo
3:01 a.m.
New subject: [PATCH v4 ] Adding the zram decompression algorithm "lzo-rle" to support kernel versions >= 5.1
On 2024/04/16 15:49, Yulong Tang wrote:
> On Mon, Apr 15, 2024 at 2:32 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com>
> wrote:
>
>
> Sure. The macro '#if __BYTE_ORDER == __BIG_ENDIAN' takes effect on a
> big-endian machine. But it requires to include the header file 'byteswap.h'
> as below:
> #include <byteswap.h>
>
> In addition, there is a similar implementation, but I haven't tested it
> with Yulong's case. Can you help to check if this can work well for you?
> Yulong.
>
> static inline uint16_t
> get_unaligned_le16 (const uint8_t *p)
> {
> return p[0] | p[1] << 8;
> }
Ah nice, this looks simple and better. If it works well on a big-endian
machine too, let's go with this.
>
Hi, Lianbo
It works well in my case(little-endian). This looks promising, but I don't have a
big-endian machine to test it on.
Thank you for testing, Yulong.
Thanks,
Kazu
3:59 a.m.
New subject: [PATCH v4 ] Adding the zram decompression algorithm "lzo-rle" to support kernel versions >= 5.1
On Tue, Apr 16, 2024 at 4:01 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
wrote:
On 2024/04/16 15:49, Yulong Tang wrote:
>> On Mon, Apr 15, 2024 at 2:32 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)
nec.com>
>> wrote:
>>
>>
>> Sure. The macro '#if __BYTE_ORDER == __BIG_ENDIAN' takes effect on a
>> big-endian machine. But it requires to include the header file
'byteswap.h'
>> as below:
>> #include <byteswap.h>
>>
>> In addition, there is a similar implementation, but I haven't tested it
>> with Yulong's case. Can you help to check if this can work well for you?
>> Yulong.
>>
>> static inline uint16_t
>> get_unaligned_le16 (const uint8_t *p)
>> {
>> return p[0] | p[1] << 8;
>> }
Ah nice, this looks simple and better. If it works well on a big-endian
machine too, let's go with this.
I tested these two functions with a test case, for example:
unsigned int x = 0x76543210;
big-endian machine:
call get_unaligned_le16_v1(): 0x5476
call get_unaligned_le16_v2(): 0x5476
little-endian machine:
call get_unaligned_le16_v1(): 0x3210
call get_unaligned_le16_v2(): 0x3210
---------------------------code---------------------------------------
static uint16_t get_unaligned_le16_v1(const void *p) {
uint16_t value;
memcpy(&value, p, sizeof(uint16_t));
#if __BYTE_ORDER == __BIG_ENDIAN
return bswap_16(value);
#else
return value;
#endif
}
static inline uint16_t
get_unaligned_le16_v2(const uint8_t *p)
{
return p[0] | p[1] << 8;
}
---------------------------code---------------------------------------
They have the same results on the little-endian/big-endian machine from my
side.
>>
>
> Hi, Lianbo
>
> It works well in my case(little-endian). This looks promising, but I
don't have a big-
Thank you for the confirmation, Yulong.
Thanks
Lianbo
endian machine to test it on.
Thank you for testing, Yulong.
Thanks,
Kazu
9:45 p.m.
New subject: [PATCH v4 ] Adding the zram decompression algorithm "lzo-rle" to support kernel versions >= 5.1
Hi, Lianbo
Thanks for the improvement and testing.
So, I'll add this modification into the latest patch by Kazu and submit the patch v5,
right?
Thanks
Yulong
64
days inactive
84
days old
devel@lists.crash-utility.osci.io
7 comments
3 participants
participants (3)
-
HAGIO KAZUHITO(萩尾 一仁) -
lijiang -
Yulong Tang