Patchwork [v2] pstore: Adjust buffer size for compression for smaller registered buffers

login
register
mail settings
Submitter Aruna Balakrishnaiah
Date Sept. 12, 2013, 6:50 a.m.
Message ID <20130912064832.15340.52422.stgit@aruna-ThinkPad-T420>
Download mbox | patch
Permalink /patch/274444/
State Not Applicable
Headers show

Comments

Aruna Balakrishnaiah - Sept. 12, 2013, 6:50 a.m.
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 <seiji.aguchi@hds.com>
Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
---
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(-)
Seiji Aguchi - Sept. 12, 2013, 4:22 p.m.
efivars works fine with this v2 patch.

Tested-by: Seiji Aguchi <seiji.aguchi@hds.com>

> -----Original Message-----
> From: Aruna Balakrishnaiah [mailto:aruna@linux.vnet.ibm.com]
> Sent: Thursday, September 12, 2013 2:51 AM
> To: linuxppc-dev@ozlabs.org; tony.luck@intel.com; Seiji Aguchi; linux-kernel@vger.kernel.org; keescook@chromium.org
> Cc: jkenisto@linux.vnet.ibm.com; mahesh@linux.vnet.ibm.com; cbouatmailru@gmail.com; ananth@in.ibm.com; ccross@android.com
> 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 <seiji.aguchi@hds.com>
> Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
> ---
> 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),
Luck, Tony - Sept. 12, 2013, 5:43 p.m.
+	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
Aruna Balakrishnaiah - Sept. 12, 2013, 7:22 p.m.
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
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

Patch

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),