2013-09-12 06:50:46

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: [PATCH v2] pstore: Adjust buffer size for compression for smaller registered buffers

When backends (ex: efivars) have smaller registered buffers, the big_oops_buf
is quite too big for them as number of repeated occurences in the text captured
will be less. Patch takes care of adjusting the buffer size based on the
registered buffer size. cmpr values has been arrived after doing experiments with
plain text for buffers of size 1k - 4k (Smaller the buffer size repeated occurence
will be less) and with sample crash log for buffers ranging from 4k - 10k.

Reported-by: Seiji Aguchi <[email protected]>
Signed-off-by: Aruna Balakrishnaiah <[email protected]>
---
Changes from v1:
Retain the cmpr = 45 for buffers ranging of size 4k - 10k. 45 seems to work.
I added an additional headroom of 3%. Revert it back to 45.

fs/pstore/platform.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 4ffb7ab..57b4219 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -195,8 +195,29 @@ error:
static void allocate_buf_for_compression(void)
{
size_t size;
+ size_t cmpr;
+
+ switch (psinfo->bufsize) {
+ /* buffer range for efivars */
+ case 1000 ... 2000:
+ cmpr = 56;
+ break;
+ case 2001 ... 3000:
+ cmpr = 54;
+ break;
+ case 3001 ... 3999:
+ cmpr = 52;
+ break;
+ /* buffer range for nvram, erst */
+ case 4000 ... 10000:
+ cmpr = 45;
+ break;
+ default:
+ cmpr = 60;
+ break;
+ }

- big_oops_buf_sz = (psinfo->bufsize * 100) / 45;
+ big_oops_buf_sz = (psinfo->bufsize * 100) / cmpr;
big_oops_buf = kmalloc(big_oops_buf_sz, GFP_KERNEL);
if (big_oops_buf) {
size = max(zlib_deflate_workspacesize(WINDOW_BITS, MEM_LEVEL),


2013-09-12 16:23:14

by Seiji Aguchi

[permalink] [raw]
Subject: RE: [PATCH v2] pstore: Adjust buffer size for compression for smaller registered buffers

efivars works fine with this v2 patch.

Tested-by: Seiji Aguchi <[email protected]>

> -----Original Message-----
> From: Aruna Balakrishnaiah [mailto:[email protected]]
> Sent: Thursday, September 12, 2013 2:51 AM
> To: [email protected]; [email protected]; Seiji Aguchi; [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
> Subject: [PATCH v2] pstore: Adjust buffer size for compression for smaller registered buffers
>
> When backends (ex: efivars) have smaller registered buffers, the big_oops_buf
> is quite too big for them as number of repeated occurences in the text captured
> will be less. Patch takes care of adjusting the buffer size based on the
> registered buffer size. cmpr values has been arrived after doing experiments with
> plain text for buffers of size 1k - 4k (Smaller the buffer size repeated occurence
> will be less) and with sample crash log for buffers ranging from 4k - 10k.
>
> Reported-by: Seiji Aguchi <[email protected]>
> Signed-off-by: Aruna Balakrishnaiah <[email protected]>
> ---
> Changes from v1:
> Retain the cmpr = 45 for buffers ranging of size 4k - 10k. 45 seems to work.
> I added an additional headroom of 3%. Revert it back to 45.
>
> fs/pstore/platform.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 4ffb7ab..57b4219 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -195,8 +195,29 @@ error:
> static void allocate_buf_for_compression(void)
> {
> size_t size;
> + size_t cmpr;
> +
> + switch (psinfo->bufsize) {
> + /* buffer range for efivars */
> + case 1000 ... 2000:
> + cmpr = 56;
> + break;
> + case 2001 ... 3000:
> + cmpr = 54;
> + break;
> + case 3001 ... 3999:
> + cmpr = 52;
> + break;
> + /* buffer range for nvram, erst */
> + case 4000 ... 10000:
> + cmpr = 45;
> + break;
> + default:
> + cmpr = 60;
> + break;
> + }
>
> - big_oops_buf_sz = (psinfo->bufsize * 100) / 45;
> + big_oops_buf_sz = (psinfo->bufsize * 100) / cmpr;
> big_oops_buf = kmalloc(big_oops_buf_sz, GFP_KERNEL);
> if (big_oops_buf) {
> size = max(zlib_deflate_workspacesize(WINDOW_BITS, MEM_LEVEL),

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-12 17:43:59

by Luck, Tony

[permalink] [raw]
Subject: RE: [PATCH v2] pstore: Adjust buffer size for compression for smaller registered buffers

+ default:
+ cmpr = 60;
+ break;
+ }

Is this the right "default"? It may be a good choice for a backend with a really
tiny buffer (1 ... 999). But less good for a (theoretical) backend with a larger
buffer (10001 ... infinity and beyond). Which are you trying to catch here?

-Tony

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-12 19:22:15

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: Re: [PATCH v2] pstore: Adjust buffer size for compression for smaller registered buffers

On Thursday 12 September 2013 11:13 PM, Luck, Tony wrote:
> + default:
> + cmpr = 60;
> + break;
> + }
>
> Is this the right "default"? It may be a good choice for a backend with a really
> tiny buffer (1 ... 999). But less good for a (theoretical) backend with a larger
> buffer (10001 ... infinity and beyond). Which are you trying to catch here?

Trying to catch lower buffers and also tried till 23k ( 10k * 100/45) of text with a
sample crash logit achieved a good compression of 25%. Since the upper limit is
not known
and compression behavior is not tested above 10k chose to keep a higher default
of 60.

> -Tony
>
> _______________________________________________
> Linuxppc-dev mailing list
> [email protected]
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>