diff mbox

[RFC,v2,04/11] pstore: Add compression support to pstore

Message ID 20130816131805.3338.68767.stgit@aruna-ThinkPad-T420 (mailing list archive)
State Superseded
Headers show

Commit Message

Aruna Balakrishnaiah Aug. 16, 2013, 1:18 p.m. UTC
Add compression support to pstore which will help in capturing more data.
Initially, pstore will make a call to kmsg_dump with a bigger buffer
and will pass the size of bigger buffer to kmsg_dump and then compress
the data to registered buffer of registered size.

In case compression fails, pstore will capture the uncompressed
data by making a call again to kmsg_dump with registered_buffer
of registered size.

Pstore will indicate the data is compressed or not with a flag
in the write callback.

Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
---
 fs/pstore/platform.c |  148 +++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 139 insertions(+), 9 deletions(-)

Comments

Seiji Aguchi Aug. 22, 2013, 11:07 p.m. UTC | #1
>   * callback from kmsg_dump. (s2,l2) has the most recently
>   * written bytes, older bytes are in (s1,l1). Save as much
> @@ -148,23 +243,56 @@ static void pstore_dump(struct kmsg_dumper *dumper,
>  		char *dst;
>  		unsigned long size;
>  		int hsize;
> +		int zipped_len = -1;
>  		size_t len;
> -		bool compressed = false;
> +		bool compressed;
> +		size_t total_len;
> 
> -		dst = psinfo->buf;
> -		hsize = sprintf(dst, "%s#%d Part%d\n", why, oopscount, part);
> -		size = psinfo->bufsize - hsize;
> -		dst += hsize;
> +		if (big_oops_buf) {
> +			dst = big_oops_buf;
> +			hsize = sprintf(dst, "%s#%d Part%d\n", why,
> +							oopscount, part);
> +			size = big_oops_buf_sz - hsize;
> 
> -		if (!kmsg_dump_get_buffer(dumper, true, dst, size, &len))
> -			break;
> +			if (!kmsg_dump_get_buffer(dumper, true, dst + hsize,
> +								size, &len))
> +				break;
> +
> +			zipped_len = pstore_compress(dst, psinfo->buf,
> +						hsize + len, psinfo->bufsize);
> +
> +			if (zipped_len > 0) {
> +				compressed = true;
> +				total_len = zipped_len;
> +			} else {
> +				pr_err("pstore: compression failed for Part %d"
> +					" returned %d\n", part, zipped_len);
> +				pr_err("pstore: Capture uncompressed"
> +					" oops/panic report of Part %d\n", part);

Why did you add these messages in pstore_dump()?
In my test case, they are not needed....

<snip>
# cat dmesg-efi-1
Panic#2 Part1
<4>[  383.209057] RBP: ffff88006f551e80 R08: 0000000000000002 R09: 0000000000000000
<4>[  383.209057] R10: 0000000000000382 R11: 0000000000000000 R12: 0000000000000063
<4>[  383.209057] R13: 0000000000000286 R14: 000000000000000f R15: 0000000000000000
<4>[  383.209057] FS:  00007f53317cc740(0000) GS:ffff88007c400000(0000) knlGS:0000000000000000
<4>[  383.209057] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[  383.209057] CR2: 0000000000000000 CR3: 0000000078a73000 CR4: 00000000000006f0
<4>[  383.209057] Stack:
<4>[  383.209057]  ffff88006f551eb8 ffffffff813d40a2 0000000000000002 00007f53317db000
<4>[  383.209057]  ffff88006f551f50 0000000000000002 0000000000000000 ffff88006f551ed8
<4>[  383.209057]  ffffffff813d45aa 00007f53317db000 ffff88003f8c2b00 ffff88006f551ef8
<4>[  383.209057] Call Trace:
<4>[  383.209057]  [<ffffffff813d40a2>] __handle_sysrq+0xa2/0x170
<4>[  383.209057]  [<ffffffff813d45aa>] write_sysrq_trigger+0x4a/0x50
<4>[  383.209057]  [<ffffffff8121981d>] proc_reg_write+0x3d/0x80
<4>[  383.209057]  [<ffffffff811aeb20>] vfs_write+0xc0/0x1f0
<4>[  383.209057]  [<ffffffff811af59c>] SyS_write+0x4c/0xa0
<4>[  383.209057]  [<ffffffff8168be82>] system_call_fastpath+0x16/0x1b
<4>[  383.209057] Code: ef e8 ff f7 ff ff eb d8 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 0f 1f 44 00 00 55 c7 05 cc f3 c9 00 01 00 00 00 48 89 e5 0f ae f8 <c6> 04 25 00 00 00 00 01 5d c3 0f 1f 44 00 00 55 31 c0 c7 05 5e 
<1>[  383.209057] RIP  [<ffffffff813d3946>] sysrq_handle_crash+0x16/0x20
<4>[  383.209057]  RSP <ffff88006f551e80>
<4>[  383.209057] CR2: 0000000000000000
<4>[  383.209057] ---[ end trace 04a1cddad37b4b33 ]---
<3>[  383.209057] pstore: compression failed for Part 2 returned -5
<3>[  383.209057] pstore: Capture uncompressed oops/panic report of Part 2
<3>[  383.209057] pstore: compression failed for Part 5 returned -5
<3>[  383.209057] pstore: Capture uncompressed oops/panic report of Part 5
<3>[  383.209057] pstore: compression failed for Part 12 returned -5
<3>[  383.209057] pstore: Capture uncompressed oops/panic report of Part 12
<0>[  383.209057] Kernel panic - not syncing: Fatal exception
<3>[  383.209057] drm_kms_helper: panic occurred, switching back to text console
<snip>


In efi-pstore case, after rebooting a system,
we are able to know which entry failed to compress with 'C' or 'D' as below.

#ls /sys/firmware/efi/vars/ |grep dump
dump-type0-10-1-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-10-2-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-11-1-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-1-1-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-11-2-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-12-1-1377204734-D-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-1-2-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-12-2-1377204734-D-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-13-1-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-13-2-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-2-1-1377204734-D-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-2-2-1377204734-D-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-3-1-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-3-2-1377204734-D-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-4-1-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-4-2-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-5-1-1377204734-D-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-5-2-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-6-1-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-6-2-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-7-1-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-7-2-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-8-1-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-8-2-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-9-1-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-9-2-1377204734-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0

Seiji
Tony Luck Aug. 22, 2013, 11:17 p.m. UTC | #2
<1>[  383.209057] RIP  [<ffffffff813d3946>] sysrq_handle_crash+0x16/0x20
<4>[  383.209057]  RSP <ffff88006f551e80>
<4>[  383.209057] CR2: 0000000000000000
<4>[  383.209057] ---[ end trace 04a1cddad37b4b33 ]---
<3>[  383.209057] pstore: compression failed for Part 2 returned -5
<3>[  383.209057] pstore: Capture uncompressed oops/panic report of Part 2
<3>[  383.209057] pstore: compression failed for Part 5 returned -5

Interesting.  With ERST backend I didn't see these messages.  Traces in 
pstore recovered files go as far as the line before the "---[ end trace 04a1cddad37b4b33 ]---"

Why the difference depending on which back end is in use?

But I agree that we shouldn't have these messages.  They use up space
in the persistent store that could be better used saving some more lines
from earlier in the console log.

-Tony
Seiji Aguchi Aug. 22, 2013, 11:47 p.m. UTC | #3
> -----Original Message-----
> From: Luck, Tony [mailto:tony.luck@intel.com]
> Sent: Thursday, August 22, 2013 7:17 PM
> To: Seiji Aguchi; Aruna Balakrishnaiah; linuxppc-dev@ozlabs.org; linux-kernel@vger.kernel.org; keescook@chromium.org
> Cc: jkenisto@linux.vnet.ibm.com; ananth@in.ibm.com; benh@kernel.crashing.org; cbouatmailru@gmail.com;
> mahesh@linux.vnet.ibm.com; ccross@android.com
> Subject: RE: [RFC PATCH v2 04/11] pstore: Add compression support to pstore
> 
> <1>[  383.209057] RIP  [<ffffffff813d3946>] sysrq_handle_crash+0x16/0x20
> <4>[  383.209057]  RSP <ffff88006f551e80>
> <4>[  383.209057] CR2: 0000000000000000
> <4>[  383.209057] ---[ end trace 04a1cddad37b4b33 ]---
> <3>[  383.209057] pstore: compression failed for Part 2 returned -5
> <3>[  383.209057] pstore: Capture uncompressed oops/panic report of Part 2
> <3>[  383.209057] pstore: compression failed for Part 5 returned -5
> 
> Interesting.  With ERST backend I didn't see these messages.  Traces in
> pstore recovered files go as far as the line before the "---[ end trace 04a1cddad37b4b33 ]---"
> 
> Why the difference depending on which back end is in use?

I think the difference doesn't depend on the back end.
Rather it depends on the environment.

I tested on a kvm guest with OVMF.

Seiji


> 
> But I agree that we shouldn't have these messages.  They use up space
> in the persistent store that could be better used saving some more lines
> from earlier in the console log.
> 
> -Tony
Aruna Balakrishnaiah Aug. 27, 2013, 5:19 a.m. UTC | #4
On Friday 23 August 2013 04:47 AM, Luck, Tony wrote:
> <1>[  383.209057] RIP  [<ffffffff813d3946>] sysrq_handle_crash+0x16/0x20
> <4>[  383.209057]  RSP <ffff88006f551e80>
> <4>[  383.209057] CR2: 0000000000000000
> <4>[  383.209057] ---[ end trace 04a1cddad37b4b33 ]---
> <3>[  383.209057] pstore: compression failed for Part 2 returned -5
> <3>[  383.209057] pstore: Capture uncompressed oops/panic report of Part 2
> <3>[  383.209057] pstore: compression failed for Part 5 returned -5
>
> Interesting.  With ERST backend I didn't see these messages.  Traces in
> pstore recovered files go as far as the line before the "---[ end trace 04a1cddad37b4b33 ]---"
>
> Why the difference depending on which back end is in use?
>
> But I agree that we shouldn't have these messages.  They use up space
> in the persistent store that could be better used saving some more lines
> from earlier in the console log.

Yeah. We can remove these messages as it will add to the space consumed. But it 
would
be good to know why the compression failed with efivars case.

Seiji,

Could you let us know the efivars buffer size with which the pstore is 
registered when
the failure occurred.

Regards,
Aruna


>
> -Tony
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
Seiji Aguchi Sept. 4, 2013, 1:44 a.m. UTC | #5
Aruna,

Sorry for the late response.

> Seiji,
> 
> Could you let us know the efivars buffer size with which the pstore is
> registered when
> the failure occurred.

I looked into the issue today.

I added some debug message just before pstore_compress().
As you can see below, the buffer size is a fixed value(1024).
Therefore, the size doesn't seem to be related to the failure directly.

Also, in the failure case, zlib_deflate() returns Z_OK and stream.avail_out is zero.
So, I thought big_oops_buf_sz is too big in my environment.
And then, I tuned  big_oops_buf_sz to (psinfo->bufsize * 100) / 53.

At the same time, while looking into this issue, I had two concerns about current cording.

1) In pstore_decompress(), why not use zlib_inflateInit2() instead of zlib_inflateInit()?
     If you use zlib_deflateInit2()  for specifying window_bit at compressing time, 
     zlib_inflateInit2() seems to be appropriate for decompressing.
     (Please see a comment about inflateInit2() in include/linux/zlib.h and source code of crypto/deflate.c)

2) As looking at crypto/deflate.c, stream->workspace is kzalloced at the beginning of 
   compressing/decompressing time.
    So, in pstore case,  stream.workspace must be initialized to 0 with memset() in pstore_compress()/decompress().

I did three things above, tuning big_oops_buf_sz, using zlib_inflateInit2() and initializing stream.workspace , in my environment.
As a result, compressions/decmpressions of all entries succeeded with efivars driver.

<snip>
Panic#2 Part1
<4>[   75.665020]  [<ffffffff811af59c>] SyS_write+0x4c/0xa0
<4>[   75.665020]  [<ffffffff8168be82>] system_call_fastpath+0x16/0x1b
<4>[   75.665020] Code: ef e8 ff f7 ff ff eb d8 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 0f 1f 44 00 00 55 c7 05 cc f3 c9 00 01 00 00 00 48 89 e5 0f ae f8 <c6> 04 25 00 00 00 00 01 5d c3 0f 1f 44 00 00 55 31 c0 c7 05 5e 
<1>[   75.665020] RIP  [<ffffffff813d3946>] sysrq_handle_crash+0x16/0x20
<4>[   75.665020]  RSP <ffff88007852de80>
<4>[   75.665020] CR2: 0000000000000000
<4>[   75.665020] ---[ end trace 97bb4a1f8d3fe7b2 ]---
<3>[   75.665020] pstore_dump hsize=13 len=2155 big_oops_buf_sz=2275 psinfo->bufsize=1024
<3>[   75.665020] pstore_dump hsize=13 len=2246 big_oops_buf_sz=2275 psinfo->bufsize=1024
<3>[   75.665020] pstore: compression failed for Part 2 returned -5
<3>[   75.665020] pstore: Capture uncompressed oops/panic report of Part 2
<3>[   75.665020] pstore_dump hsize=13 len=2239 big_oops_buf_sz=2275 psinfo->bufsize=1024
<3>[   75.665020] pstore_dump hsize=13 len=2231 big_oops_buf_sz=2275 psinfo->bufsize=1024
<3>[   75.665020] pstore_dump hsize=13 len=2185 big_oops_buf_sz=2275 psinfo->bufsize=1024
<3>[   75.665020] pstore: compression failed for Part 5 returned -5
<3>[   75.665020] pstore: Capture uncompressed oops/panic report of Part 5
<3>[   75.665020] pstore_dump hsize=13 len=2234 big_oops_buf_sz=2275 psinfo->bufsize=1024
<3>[   75.665020] pstore_dump hsize=13 len=2208 big_oops_buf_sz=2275 psinfo->bufsize=1024
<3>[   75.665020] pstore_dump hsize=13 len=2218 big_oops_buf_sz=2275 psinfo->bufsize=1024
<3>[   75.665020] pstore_dump hsize=13 len=2222 big_oops_buf_sz=2275 psinfo->bufsize=1024
<3>[   75.665020] pstore: compression failed for Part 9 returned -5
<3>[   75.665020] pstore: Capture uncompressed oops/panic report of Part 9
<3>[   75.665020] pstore_dump hsize=14 len=2256 big_oops_buf_sz=2275 psinfo->bufsize=1024
<3>[   75.665020] pstore_dump hsize=14 len=2219 big_oops_buf_sz=2275 psinfo->bufsize=1024
<3>[   75.665020] pstore_dump hsize=14 len=2226 big_oops_buf_sz=2275 psinfo->bufsize=1024
<0>[   75.665020] Kernel panic - not syncing: Fatal exception
<3>[   75.665020] drm_kms_helper: panic occurred, switching back to text console
<snip>

Seiji
Aruna Balakrishnaiah Sept. 4, 2013, 6:01 a.m. UTC | #6
On Wednesday 04 September 2013 07:14 AM, Seiji Aguchi wrote:
> Aruna,
>
> Sorry for the late response.
>
>> Seiji,
>>
>> Could you let us know the efivars buffer size with which the pstore is
>> registered when
>> the failure occurred.
> I looked into the issue today.
>
> I added some debug message just before pstore_compress().
> As you can see below, the buffer size is a fixed value(1024).
> Therefore, the size doesn't seem to be related to the failure directly.
>
> Also, in the failure case, zlib_deflate() returns Z_OK and stream.avail_out is zero.
> So, I thought big_oops_buf_sz is too big in my environment.
> And then, I tuned  big_oops_buf_sz to (psinfo->bufsize * 100) / 53.
Seiji,

Thank you for the analysis.

The reason behind compression failure is the size of big_oops_buf which is too
big for efivars case. I will do some experiments with different kind of texts
for buffer size 1024 to check if 100/53 suits for all the cases.

>
> At the same time, while looking into this issue, I had two concerns about current cording.
>
> 1) In pstore_decompress(), why not use zlib_inflateInit2() instead of zlib_inflateInit()?
>       If you use zlib_deflateInit2()  for specifying window_bit at compressing time,
>       zlib_inflateInit2() seems to be appropriate for decompressing.
>       (Please see a comment about inflateInit2() in include/linux/zlib.h and source code of crypto/deflate.c)

Yes this can be changed to zlib_inflateInit2().

> 2) As looking at crypto/deflate.c, stream->workspace is kzalloced at the beginning of
>     compressing/decompressing time.
>      So, in pstore case,  stream.workspace must be initialized to 0 with memset() in pstore_compress()/decompress().

Hmm.. I don't think this is a issue. If you see fs/logfs/compr.c from which I 
derived the compression
algorithms for pstore as well, they have not initialized stream.workspace. Since 
the space will be overwritten
during the calculation, I dont think it will matter.

The above 2 things you have suggested are good to have. But the reason behind 
compression failure is
the big_oops_buf_sz which is too big for efivars.

> I did three things above, tuning big_oops_buf_sz, using zlib_inflateInit2() and initializing stream.workspace , in my environment.
> As a result, compressions/decmpressions of all entries succeeded with efivars driver.
>
> <snip>
> Panic#2 Part1
> <4>[   75.665020]  [<ffffffff811af59c>] SyS_write+0x4c/0xa0
> <4>[   75.665020]  [<ffffffff8168be82>] system_call_fastpath+0x16/0x1b
> <4>[   75.665020] Code: ef e8 ff f7 ff ff eb d8 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 0f 1f 44 00 00 55 c7 05 cc f3 c9 00 01 00 00 00 48 89 e5 0f ae f8 <c6> 04 25 00 00 00 00 01 5d c3 0f 1f 44 00 00 55 31 c0 c7 05 5e
> <1>[   75.665020] RIP  [<ffffffff813d3946>] sysrq_handle_crash+0x16/0x20
> <4>[   75.665020]  RSP <ffff88007852de80>
> <4>[   75.665020] CR2: 0000000000000000
> <4>[   75.665020] ---[ end trace 97bb4a1f8d3fe7b2 ]---
> <3>[   75.665020] pstore_dump hsize=13 len=2155 big_oops_buf_sz=2275 psinfo->bufsize=1024
> <3>[   75.665020] pstore_dump hsize=13 len=2246 big_oops_buf_sz=2275 psinfo->bufsize=1024
> <3>[   75.665020] pstore: compression failed for Part 2 returned -5
> <3>[   75.665020] pstore: Capture uncompressed oops/panic report of Part 2
> <3>[   75.665020] pstore_dump hsize=13 len=2239 big_oops_buf_sz=2275 psinfo->bufsize=1024
> <3>[   75.665020] pstore_dump hsize=13 len=2231 big_oops_buf_sz=2275 psinfo->bufsize=1024
> <3>[   75.665020] pstore_dump hsize=13 len=2185 big_oops_buf_sz=2275 psinfo->bufsize=1024
> <3>[   75.665020] pstore: compression failed for Part 5 returned -5
> <3>[   75.665020] pstore: Capture uncompressed oops/panic report of Part 5
> <3>[   75.665020] pstore_dump hsize=13 len=2234 big_oops_buf_sz=2275 psinfo->bufsize=1024
> <3>[   75.665020] pstore_dump hsize=13 len=2208 big_oops_buf_sz=2275 psinfo->bufsize=1024
> <3>[   75.665020] pstore_dump hsize=13 len=2218 big_oops_buf_sz=2275 psinfo->bufsize=1024
> <3>[   75.665020] pstore_dump hsize=13 len=2222 big_oops_buf_sz=2275 psinfo->bufsize=1024
> <3>[   75.665020] pstore: compression failed for Part 9 returned -5
> <3>[   75.665020] pstore: Capture uncompressed oops/panic report of Part 9
> <3>[   75.665020] pstore_dump hsize=14 len=2256 big_oops_buf_sz=2275 psinfo->bufsize=1024
> <3>[   75.665020] pstore_dump hsize=14 len=2219 big_oops_buf_sz=2275 psinfo->bufsize=1024
> <3>[   75.665020] pstore_dump hsize=14 len=2226 big_oops_buf_sz=2275 psinfo->bufsize=1024
> <0>[   75.665020] Kernel panic - not syncing: Fatal exception
> <3>[   75.665020] drm_kms_helper: panic occurred, switching back to text console
> <snip>
>
> Seiji
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
Tony Luck Sept. 4, 2013, 4:11 p.m. UTC | #7
> The reason behind compression failure is the size of big_oops_buf which is too
> big for efivars case. I will do some experiments with different kind of texts
> for buffer size 1024 to check if 100/53 suits for all the cases.
...

> Yes this can be changed to zlib_inflateInit2().

Original patch series was just pulled by Linus ... so we'll need a patch on top
of current Linus git tree to fix these issues.  But let's make sure that efivars, erst,
etc. are all happy with the changes we make before I ask Linus to pull another
pstore piece.

Thanks

-Tony
Seiji Aguchi Sept. 4, 2013, 4:39 p.m. UTC | #8
> But let's make sure that efivars, erst,
> etc. are all happy with the changes we make before I ask Linus to pull another
> pstore piece.

I will test efivars when Aruna posts the bugfix patches.

Seiji

> -----Original Message-----
> From: Luck, Tony [mailto:tony.luck@intel.com]
> Sent: Wednesday, September 04, 2013 12:11 PM
> To: Aruna Balakrishnaiah; Seiji Aguchi
> Cc: jkenisto@linux.vnet.ibm.com; keescook@chromium.org; mahesh@linux.vnet.ibm.com; ccross@android.com; linux-
> kernel@vger.kernel.org; linuxppc-dev@ozlabs.org; cbouatmailru@gmail.com
> Subject: RE: [RFC PATCH v2 04/11] pstore: Add compression support to pstore
> 
> > The reason behind compression failure is the size of big_oops_buf which is too
> > big for efivars case. I will do some experiments with different kind of texts
> > for buffer size 1024 to check if 100/53 suits for all the cases.
> ...
> 
> > Yes this can be changed to zlib_inflateInit2().
> 
> Original patch series was just pulled by Linus ... so we'll need a patch on top
> of current Linus git tree to fix these issues.  But let's make sure that efivars, erst,
> etc. are all happy with the changes we make before I ask Linus to pull another
> pstore piece.
> 
> Thanks
> 
> -Tony
diff mbox

Patch

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 20fa686..56218cb 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -26,6 +26,7 @@ 
 #include <linux/console.h>
 #include <linux/module.h>
 #include <linux/pstore.h>
+#include <linux/zlib.h>
 #include <linux/string.h>
 #include <linux/timer.h>
 #include <linux/slab.h>
@@ -65,6 +66,15 @@  struct pstore_info *psinfo;
 
 static char *backend;
 
+/* Compression parameters */
+#define COMPR_LEVEL 6
+#define WINDOW_BITS 12
+#define MEM_LEVEL 4
+static struct z_stream_s stream;
+
+static char *big_oops_buf;
+static size_t big_oops_buf_sz;
+
 /* How much of the console log to snapshot */
 static unsigned long kmsg_bytes = 10240;
 
@@ -117,6 +127,91 @@  bool pstore_cannot_block_path(enum kmsg_dump_reason reason)
 }
 EXPORT_SYMBOL_GPL(pstore_cannot_block_path);
 
+/* Derived from logfs_compress() */
+static int pstore_compress(const void *in, void *out, size_t inlen,
+							size_t outlen)
+{
+	int err, ret;
+
+	ret = -EIO;
+	err = zlib_deflateInit2(&stream, COMPR_LEVEL, Z_DEFLATED, WINDOW_BITS,
+						MEM_LEVEL, Z_DEFAULT_STRATEGY);
+	if (err != Z_OK)
+		goto error;
+
+	stream.next_in = in;
+	stream.avail_in = inlen;
+	stream.total_in = 0;
+	stream.next_out = out;
+	stream.avail_out = outlen;
+	stream.total_out = 0;
+
+	err = zlib_deflate(&stream, Z_FINISH);
+	if (err != Z_STREAM_END)
+		goto error;
+
+	err = zlib_deflateEnd(&stream);
+	if (err != Z_OK)
+		goto error;
+
+	if (stream.total_out >= stream.total_in)
+		goto error;
+
+	ret = stream.total_out;
+error:
+	return ret;
+}
+
+static void allocate_buf_for_compression(void)
+{
+	size_t size;
+
+	big_oops_buf_sz = (psinfo->bufsize * 100) / 45;
+	big_oops_buf = kmalloc(big_oops_buf_sz, GFP_KERNEL);
+	if (big_oops_buf) {
+		size = max(zlib_deflate_workspacesize(WINDOW_BITS, MEM_LEVEL),
+			zlib_inflate_workspacesize());
+		stream.workspace = kmalloc(size, GFP_KERNEL);
+		if (!stream.workspace) {
+			pr_err("pstore: No memory for compression workspace; "
+				"skipping compression\n");
+			kfree(big_oops_buf);
+			big_oops_buf = NULL;
+		}
+	} else {
+		pr_err("No memory for uncompressed data; "
+			"skipping compression\n");
+		stream.workspace = NULL;
+	}
+
+}
+
+/*
+ * Called when compression fails, since the printk buffer
+ * would be fetched for compression calling it again when
+ * compression fails would have moved the iterator of
+ * printk buffer which results in fetching old contents.
+ * Copy the recent messages from big_oops_buf to psinfo->buf
+ */
+static size_t copy_kmsg_to_buffer(int hsize, size_t len)
+{
+	size_t total_len;
+	size_t diff;
+
+	total_len = hsize + len;
+
+	if (total_len > psinfo->bufsize) {
+		diff = total_len - psinfo->bufsize + hsize;
+		memcpy(psinfo->buf, big_oops_buf, hsize);
+		memcpy(psinfo->buf + hsize, big_oops_buf + diff,
+					psinfo->bufsize - hsize);
+		total_len = psinfo->bufsize;
+	} else
+		memcpy(psinfo->buf, big_oops_buf, total_len);
+
+	return total_len;
+}
+
 /*
  * callback from kmsg_dump. (s2,l2) has the most recently
  * written bytes, older bytes are in (s1,l1). Save as much
@@ -148,23 +243,56 @@  static void pstore_dump(struct kmsg_dumper *dumper,
 		char *dst;
 		unsigned long size;
 		int hsize;
+		int zipped_len = -1;
 		size_t len;
-		bool compressed = false;
+		bool compressed;
+		size_t total_len;
 
-		dst = psinfo->buf;
-		hsize = sprintf(dst, "%s#%d Part%d\n", why, oopscount, part);
-		size = psinfo->bufsize - hsize;
-		dst += hsize;
+		if (big_oops_buf) {
+			dst = big_oops_buf;
+			hsize = sprintf(dst, "%s#%d Part%d\n", why,
+							oopscount, part);
+			size = big_oops_buf_sz - hsize;
 
-		if (!kmsg_dump_get_buffer(dumper, true, dst, size, &len))
-			break;
+			if (!kmsg_dump_get_buffer(dumper, true, dst + hsize,
+								size, &len))
+				break;
+
+			zipped_len = pstore_compress(dst, psinfo->buf,
+						hsize + len, psinfo->bufsize);
+
+			if (zipped_len > 0) {
+				compressed = true;
+				total_len = zipped_len;
+			} else {
+				pr_err("pstore: compression failed for Part %d"
+					" returned %d\n", part, zipped_len);
+				pr_err("pstore: Capture uncompressed"
+					" oops/panic report of Part %d\n", part);
+				compressed = false;
+				total_len = copy_kmsg_to_buffer(hsize, len);
+			}
+		} else {
+			dst = psinfo->buf;
+			hsize = sprintf(dst, "%s#%d Part%d\n", why, oopscount,
+									part);
+			size = psinfo->bufsize - hsize;
+			dst += hsize;
+
+			if (!kmsg_dump_get_buffer(dumper, true, dst,
+								size, &len))
+				break;
+
+			compressed = false;
+			total_len = hsize + len;
+		}
 
 		ret = psinfo->write(PSTORE_TYPE_DMESG, reason, &id, part,
-				    oopscount, compressed, hsize + len, psinfo);
+				    oopscount, compressed, total_len, psinfo);
 		if (ret == 0 && reason == KMSG_DUMP_OOPS && pstore_is_mounted())
 			pstore_new_entry = 1;
 
-		total += hsize + len;
+		total += total_len;
 		part++;
 	}
 	if (pstore_cannot_block_path(reason)) {
@@ -262,6 +390,8 @@  int pstore_register(struct pstore_info *psi)
 		return -EINVAL;
 	}
 
+	allocate_buf_for_compression();
+
 	if (pstore_is_mounted())
 		pstore_get_records(0);