[RFC PATCH 0/3] Nvram-to-pstore: compression support for oops data

[RFC PATCH 0/3] Nvram-to-pstore: compression support for oops data

[Aruna Balakrishnaiah]

The patchset takes care of compressing oops messages while writing to NVRAM,
so that more oops data can be captured in the given space.

big_oops_buf (2.22 * oops_data_sz) is allocated for compression.
oops_data_sz is oops header size less of oops partition size.

Pstore will internally call kmsg_dump to capture messages from printk
buffer. While returning the data to nvram it adds is own header.

For compression:
	Register pstore with big_oops_buf. 

In case compression fails, copy header added by pstore and
last oops_data_sz bytes (recent messages) of big_oops_buf to
nvram for which we need to know header size. 
	
patch 01/03 will add a function in pstore to return the header size.

pstore read callback of nvram will read the compressed data and return the
decompressed data so that dmesg file (under /dev/pstore) is readable.

In case decompression fails, instead of having the compressed data (junk) in the
dmesg file it will skip and continue reading other partitions. This results in
absence of dmesg file but will still have files relating to other parititons.


---

Aruna Balakrishnaiah (3):
      Retreive header size from pstore.
      powerpc/pseries: Re-organise the oops compression code
      powerpc/pseries: Support compression of oops text via pstore


 arch/powerpc/platforms/pseries/nvram.c |  236 +++++++++++++++++++++++---------
 fs/pstore/platform.c                   |    7 +
 include/linux/pstore.h                 |    6 +
 3 files changed, 182 insertions(+), 67 deletions(-)

-- 

[PATCH 1/3] Retreive header size from pstore.

[Aruna Balakrishnaiah]

pstore_get_header_size will return the size of the header added by pstore
while logging messages to the registered buffer.

Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
---
 fs/pstore/platform.c   |    7 ++++++-
 include/linux/pstore.h |    6 ++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 86d1038..e8260ea 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -49,6 +49,7 @@ MODULE_PARM_DESC(update_ms, "milliseconds before pstore updates its content "
 		 "corruption on Oopses)");
 
 static int pstore_new_entry;
+static int hsize;
 
 static void pstore_timefunc(unsigned long);
 static DEFINE_TIMER(pstore_timer, pstore_timefunc, 0, 0);
@@ -68,6 +69,11 @@ static char *backend;
 /* How much of the console log to snapshot */
 static unsigned long kmsg_bytes = 10240;
 
+int pstore_get_header_size(void)
+{
+	return hsize;
+}
+
 void pstore_set_kmsg_bytes(int bytes)
 {
 	kmsg_bytes = bytes;
@@ -147,7 +153,6 @@ static void pstore_dump(struct kmsg_dumper *dumper,
 	while (total < kmsg_bytes) {
 		char *dst;
 		unsigned long size;
-		int hsize;
 		size_t len;
 
 		dst = psinfo->buf;
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index 656699f..f43b64f 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -73,6 +73,7 @@ struct pstore_info {
 #ifdef CONFIG_PSTORE
 extern int pstore_register(struct pstore_info *);
 extern bool pstore_cannot_block_path(enum kmsg_dump_reason reason);
+extern int pstore_get_header_size(void);
 #else
 static inline int
 pstore_register(struct pstore_info *psi)
@@ -84,6 +85,11 @@ pstore_cannot_block_path(enum kmsg_dump_reason reason)
 {
 	return false;
 }
+static inline int
+pstore_get_header_size(void)
+{
+	return 0;
+}
 #endif
 
 #endif /*_LINUX_PSTORE_H*/

[PATCH 2/3] powerpc/pseries: Re-organise the oops compression code

[Aruna Balakrishnaiah]

nvram_compress() and zip_oops() is used by the nvram_pstore_write
API to compress oops messages hence re-organise the functions
accordingly to avoid forward declarations.

Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/nvram.c |  104 ++++++++++++++++----------------
 1 file changed, 52 insertions(+), 52 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
index 14cc486..0159d74 100644
--- a/arch/powerpc/platforms/pseries/nvram.c
+++ b/arch/powerpc/platforms/pseries/nvram.c
@@ -486,6 +486,58 @@ static int clobbering_unread_rtas_event(void)
 						NVRAM_RTAS_READ_TIMEOUT);
 }
 
+/* Derived from logfs_compress() */
+static int nvram_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;
+}
+
+/* Compress the text from big_oops_buf into oops_buf. */
+static int zip_oops(size_t text_len)
+{
+	struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
+	int zipped_len = nvram_compress(big_oops_buf, oops_data, text_len,
+								oops_data_sz);
+	if (zipped_len < 0) {
+		pr_err("nvram: compression failed; returned %d\n", zipped_len);
+		pr_err("nvram: logging uncompressed oops/panic report\n");
+		return -1;
+	}
+	oops_hdr->version = OOPS_HDR_VERSION;
+	oops_hdr->report_length = (u16) zipped_len;
+	oops_hdr->timestamp = get_seconds();
+	return 0;
+}
+
 #ifdef CONFIG_PSTORE
 static int nvram_pstore_open(struct pstore_info *psi)
 {
@@ -757,58 +809,6 @@ int __init pSeries_nvram_init(void)
 }
 
 
-/* Derived from logfs_compress() */
-static int nvram_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;
-}
-
-/* Compress the text from big_oops_buf into oops_buf. */
-static int zip_oops(size_t text_len)
-{
-	struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
-	int zipped_len = nvram_compress(big_oops_buf, oops_data, text_len,
-								oops_data_sz);
-	if (zipped_len < 0) {
-		pr_err("nvram: compression failed; returned %d\n", zipped_len);
-		pr_err("nvram: logging uncompressed oops/panic report\n");
-		return -1;
-	}
-	oops_hdr->version = OOPS_HDR_VERSION;
-	oops_hdr->report_length = (u16) zipped_len;
-	oops_hdr->timestamp = get_seconds();
-	return 0;
-}
-
 /*
  * This is our kmsg_dump callback, called after an oops or panic report
  * has been written to the printk buffer.  We want to capture as much

[PATCH 3/3] powerpc/pseries: Support compression of oops text via pstore

[Aruna Balakrishnaiah]

The patch set supports compression of oops messages while writing to NVRAM,
this helps in capturing more of oops data to lnx,oops-log. The pstore file
for oops messages will be in decompressed format making it readable.

In case compression fails, the patch takes care of copying the header added
by pstore and last oops_data_sz bytes of big_oops_buf to NVRAM so that we
have recent oops messages in lnx,oops-log.

In case decompression fails, it will result in absence of oops file but still
have files (in /dev/pstore) for other partitions.

Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/nvram.c |  132 +++++++++++++++++++++++++++++---
 1 file changed, 118 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
index 0159d74..b5ba5e2 100644
--- a/arch/powerpc/platforms/pseries/nvram.c
+++ b/arch/powerpc/platforms/pseries/nvram.c
@@ -539,6 +539,65 @@ static int zip_oops(size_t text_len)
 }
 
 #ifdef CONFIG_PSTORE
+/* Derived from logfs_uncompress */
+int nvram_decompress(void *in, void *out, size_t inlen, size_t outlen)
+{
+	int err, ret;
+
+	ret = -EIO;
+	err = zlib_inflateInit(&stream);
+	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_inflate(&stream, Z_FINISH);
+	if (err != Z_STREAM_END)
+		goto error;
+
+	err = zlib_inflateEnd(&stream);
+	if (err != Z_OK)
+		goto error;
+
+	ret = stream.total_out;
+error:
+	return ret;
+}
+
+static int unzip_oops(char *oops_buf, char *big_buf)
+{
+	struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
+	u64 timestamp = oops_hdr->timestamp;
+	char *big_oops_data = NULL;
+	char *oops_data_buf = NULL;
+	size_t big_oops_data_sz;
+	int unzipped_len;
+
+	big_oops_data = big_buf + sizeof(struct oops_log_info);
+	big_oops_data_sz = big_oops_buf_sz - sizeof(struct oops_log_info);
+	oops_data_buf = oops_buf + sizeof(struct oops_log_info);
+
+	unzipped_len = nvram_decompress(oops_data_buf, big_oops_data,
+					oops_hdr->report_length,
+					big_oops_data_sz);
+
+	if (unzipped_len < 0) {
+		pr_err("nvram: decompression failed; returned %d\n",
+								unzipped_len);
+		return -1;
+	}
+	oops_hdr = (struct oops_log_info *)big_buf;
+	oops_hdr->version = OOPS_HDR_VERSION;
+	oops_hdr->report_length = (u16) unzipped_len;
+	oops_hdr->timestamp = timestamp;
+	return 0;
+}
+
 static int nvram_pstore_open(struct pstore_info *psi)
 {
 	/* Reset the iterator to start reading partitions again */
@@ -567,6 +626,7 @@ static int nvram_pstore_write(enum pstore_type_id type,
 				size_t size, struct pstore_info *psi)
 {
 	int rc;
+	unsigned int err_type = ERR_TYPE_KERNEL_PANIC;
 	struct oops_log_info *oops_hdr = (struct oops_log_info *) oops_buf;
 
 	/* part 1 has the recent messages from printk buffer */
@@ -577,8 +637,31 @@ static int nvram_pstore_write(enum pstore_type_id type,
 	oops_hdr->version = OOPS_HDR_VERSION;
 	oops_hdr->report_length = (u16) size;
 	oops_hdr->timestamp = get_seconds();
+
+	if (big_oops_buf) {
+		rc = zip_oops(size);
+		/*
+		 * If compression fails copy recent log messages from
+		 * big_oops_buf to oops_data.
+		 */
+		if (rc != 0) {
+			int hsize = pstore_get_header_size();
+			size_t diff = size - oops_data_sz + hsize;
+
+			if (size > oops_data_sz) {
+				memcpy(oops_data, big_oops_buf, hsize);
+				memcpy(oops_data + hsize, big_oops_buf + diff,
+					oops_data_sz - hsize);
+
+				oops_hdr->report_length = (u16) oops_data_sz;
+			} else
+				memcpy(oops_data, big_oops_buf, size);
+		} else
+			err_type = ERR_TYPE_KERNEL_PANIC_GZ;
+	}
+
 	rc = nvram_write_os_partition(&oops_log_partition, oops_buf,
-		(int) (sizeof(*oops_hdr) + size), ERR_TYPE_KERNEL_PANIC,
+		(int) (sizeof(*oops_hdr) + oops_hdr->report_length), err_type,
 		count);
 
 	if (rc != 0)
@@ -600,10 +683,11 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
 	struct oops_log_info *oops_hdr;
 	unsigned int err_type, id_no, size = 0;
 	struct nvram_os_partition *part = NULL;
-	char *buff = NULL;
-	int sig = 0;
+	char *buff = NULL, *big_buff = NULL;
+	int rc, sig = 0;
 	loff_t p;
 
+read_partition:
 	read_type++;
 
 	switch (nvram_type_ids[read_type]) {
@@ -666,6 +750,25 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
 	if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) {
 		oops_hdr = (struct oops_log_info *)buff;
 		*buf = buff + sizeof(*oops_hdr);
+
+		if (err_type == ERR_TYPE_KERNEL_PANIC_GZ) {
+			big_buff = kmalloc(big_oops_buf_sz, GFP_KERNEL);
+			if (!big_buff)
+				return -ENOMEM;
+
+			rc = unzip_oops(buff, big_buff);
+
+			if (rc != 0) {
+				kfree(buff);
+				kfree(big_buff);
+				goto read_partition;
+			}
+
+			oops_hdr = (struct oops_log_info *)big_buff;
+			*buf = big_buff + sizeof(*oops_hdr);
+			kfree(buff);
+		}
+
 		time->tv_sec = oops_hdr->timestamp;
 		time->tv_nsec = 0;
 		return oops_hdr->report_length;
@@ -687,17 +790,18 @@ static int nvram_pstore_init(void)
 {
 	int rc = 0;
 
-	nvram_pstore_info.buf = oops_data;
-	nvram_pstore_info.bufsize = oops_data_sz;
+	if (big_oops_buf) {
+		nvram_pstore_info.buf = big_oops_buf;
+		nvram_pstore_info.bufsize = big_oops_buf_sz;
+	} else {
+		nvram_pstore_info.buf = oops_data;
+		nvram_pstore_info.bufsize = oops_data_sz;
+	}
 
 	rc = pstore_register(&nvram_pstore_info);
 	if (rc != 0)
 		pr_err("nvram: pstore_register() failed, defaults to "
 				"kmsg_dump; returned %d\n", rc);
-	else
-		/*TODO: Support compression when pstore is configured */
-		pr_info("nvram: Compression of oops text supported only when "
-				"pstore is not configured");
 
 	return rc;
 }
@@ -731,11 +835,6 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
 	oops_data = oops_buf + sizeof(struct oops_log_info);
 	oops_data_sz = oops_log_partition.size - sizeof(struct oops_log_info);
 
-	rc = nvram_pstore_init();
-
-	if (!rc)
-		return;
-
 	/*
 	 * Figure compression (preceded by elimination of each line's <n>
 	 * severity prefix) will reduce the oops/panic report to at most
@@ -759,6 +858,11 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
 		stream.workspace = NULL;
 	}
 
+	rc = nvram_pstore_init();
+
+	if (!rc)
+		return;
+
 	rc = kmsg_dump_register(&nvram_kmsg_dumper);
 	if (rc != 0) {
 		pr_err("nvram: kmsg_dump_register() failed; returned %d\n", rc);

Re: [PATCH 3/3] powerpc/pseries: Support compression of oops text via pstore

[Benjamin Herrenschmidt]

On Fri, 2013-04-26 at 15:26 +0530, Aruna Balakrishnaiah wrote:
> The patch set supports compression of oops messages while writing to NVRAM,
> this helps in capturing more of oops data to lnx,oops-log. The pstore file
> for oops messages will be in decompressed format making it readable.
> 
> In case compression fails, the patch takes care of copying the header added
> by pstore and last oops_data_sz bytes of big_oops_buf to NVRAM so that we
> have recent oops messages in lnx,oops-log.
> 
> In case decompression fails, it will result in absence of oops file but still
> have files (in /dev/pstore) for other partitions.

Any reason that isn't handled by pstore itself rather than here ? Ie
make a flag indicating that the partition supports compression and have
pstore do it (so we don't compress everything such as ofw common etc...)

Cheers,
Ben.

> 
> Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/nvram.c |  132 +++++++++++++++++++++++++++++---
>  1 file changed, 118 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
> index 0159d74..b5ba5e2 100644
> --- a/arch/powerpc/platforms/pseries/nvram.c
> +++ b/arch/powerpc/platforms/pseries/nvram.c
> @@ -539,6 +539,65 @@ static int zip_oops(size_t text_len)
>  }
>  
>  #ifdef CONFIG_PSTORE
> +/* Derived from logfs_uncompress */
> +int nvram_decompress(void *in, void *out, size_t inlen, size_t outlen)
> +{
> +	int err, ret;
> +
> +	ret = -EIO;
> +	err = zlib_inflateInit(&stream);
> +	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_inflate(&stream, Z_FINISH);
> +	if (err != Z_STREAM_END)
> +		goto error;
> +
> +	err = zlib_inflateEnd(&stream);
> +	if (err != Z_OK)
> +		goto error;
> +
> +	ret = stream.total_out;
> +error:
> +	return ret;
> +}
> +
> +static int unzip_oops(char *oops_buf, char *big_buf)
> +{
> +	struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
> +	u64 timestamp = oops_hdr->timestamp;
> +	char *big_oops_data = NULL;
> +	char *oops_data_buf = NULL;
> +	size_t big_oops_data_sz;
> +	int unzipped_len;
> +
> +	big_oops_data = big_buf + sizeof(struct oops_log_info);
> +	big_oops_data_sz = big_oops_buf_sz - sizeof(struct oops_log_info);
> +	oops_data_buf = oops_buf + sizeof(struct oops_log_info);
> +
> +	unzipped_len = nvram_decompress(oops_data_buf, big_oops_data,
> +					oops_hdr->report_length,
> +					big_oops_data_sz);
> +
> +	if (unzipped_len < 0) {
> +		pr_err("nvram: decompression failed; returned %d\n",
> +								unzipped_len);
> +		return -1;
> +	}
> +	oops_hdr = (struct oops_log_info *)big_buf;
> +	oops_hdr->version = OOPS_HDR_VERSION;
> +	oops_hdr->report_length = (u16) unzipped_len;
> +	oops_hdr->timestamp = timestamp;
> +	return 0;
> +}
> +
>  static int nvram_pstore_open(struct pstore_info *psi)
>  {
>  	/* Reset the iterator to start reading partitions again */
> @@ -567,6 +626,7 @@ static int nvram_pstore_write(enum pstore_type_id type,
>  				size_t size, struct pstore_info *psi)
>  {
>  	int rc;
> +	unsigned int err_type = ERR_TYPE_KERNEL_PANIC;
>  	struct oops_log_info *oops_hdr = (struct oops_log_info *) oops_buf;
>  
>  	/* part 1 has the recent messages from printk buffer */
> @@ -577,8 +637,31 @@ static int nvram_pstore_write(enum pstore_type_id type,
>  	oops_hdr->version = OOPS_HDR_VERSION;
>  	oops_hdr->report_length = (u16) size;
>  	oops_hdr->timestamp = get_seconds();
> +
> +	if (big_oops_buf) {
> +		rc = zip_oops(size);
> +		/*
> +		 * If compression fails copy recent log messages from
> +		 * big_oops_buf to oops_data.
> +		 */
> +		if (rc != 0) {
> +			int hsize = pstore_get_header_size();
> +			size_t diff = size - oops_data_sz + hsize;
> +
> +			if (size > oops_data_sz) {
> +				memcpy(oops_data, big_oops_buf, hsize);
> +				memcpy(oops_data + hsize, big_oops_buf + diff,
> +					oops_data_sz - hsize);
> +
> +				oops_hdr->report_length = (u16) oops_data_sz;
> +			} else
> +				memcpy(oops_data, big_oops_buf, size);
> +		} else
> +			err_type = ERR_TYPE_KERNEL_PANIC_GZ;
> +	}
> +
>  	rc = nvram_write_os_partition(&oops_log_partition, oops_buf,
> -		(int) (sizeof(*oops_hdr) + size), ERR_TYPE_KERNEL_PANIC,
> +		(int) (sizeof(*oops_hdr) + oops_hdr->report_length), err_type,
>  		count);
>  
>  	if (rc != 0)
> @@ -600,10 +683,11 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
>  	struct oops_log_info *oops_hdr;
>  	unsigned int err_type, id_no, size = 0;
>  	struct nvram_os_partition *part = NULL;
> -	char *buff = NULL;
> -	int sig = 0;
> +	char *buff = NULL, *big_buff = NULL;
> +	int rc, sig = 0;
>  	loff_t p;
>  
> +read_partition:
>  	read_type++;
>  
>  	switch (nvram_type_ids[read_type]) {
> @@ -666,6 +750,25 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
>  	if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) {
>  		oops_hdr = (struct oops_log_info *)buff;
>  		*buf = buff + sizeof(*oops_hdr);
> +
> +		if (err_type == ERR_TYPE_KERNEL_PANIC_GZ) {
> +			big_buff = kmalloc(big_oops_buf_sz, GFP_KERNEL);
> +			if (!big_buff)
> +				return -ENOMEM;
> +
> +			rc = unzip_oops(buff, big_buff);
> +
> +			if (rc != 0) {
> +				kfree(buff);
> +				kfree(big_buff);
> +				goto read_partition;
> +			}
> +
> +			oops_hdr = (struct oops_log_info *)big_buff;
> +			*buf = big_buff + sizeof(*oops_hdr);
> +			kfree(buff);
> +		}
> +
>  		time->tv_sec = oops_hdr->timestamp;
>  		time->tv_nsec = 0;
>  		return oops_hdr->report_length;
> @@ -687,17 +790,18 @@ static int nvram_pstore_init(void)
>  {
>  	int rc = 0;
>  
> -	nvram_pstore_info.buf = oops_data;
> -	nvram_pstore_info.bufsize = oops_data_sz;
> +	if (big_oops_buf) {
> +		nvram_pstore_info.buf = big_oops_buf;
> +		nvram_pstore_info.bufsize = big_oops_buf_sz;
> +	} else {
> +		nvram_pstore_info.buf = oops_data;
> +		nvram_pstore_info.bufsize = oops_data_sz;
> +	}
>  
>  	rc = pstore_register(&nvram_pstore_info);
>  	if (rc != 0)
>  		pr_err("nvram: pstore_register() failed, defaults to "
>  				"kmsg_dump; returned %d\n", rc);
> -	else
> -		/*TODO: Support compression when pstore is configured */
> -		pr_info("nvram: Compression of oops text supported only when "
> -				"pstore is not configured");
>  
>  	return rc;
>  }
> @@ -731,11 +835,6 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
>  	oops_data = oops_buf + sizeof(struct oops_log_info);
>  	oops_data_sz = oops_log_partition.size - sizeof(struct oops_log_info);
>  
> -	rc = nvram_pstore_init();
> -
> -	if (!rc)
> -		return;
> -
>  	/*
>  	 * Figure compression (preceded by elimination of each line's <n>
>  	 * severity prefix) will reduce the oops/panic report to at most
> @@ -759,6 +858,11 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
>  		stream.workspace = NULL;
>  	}
>  
> +	rc = nvram_pstore_init();
> +
> +	if (!rc)
> +		return;
> +
>  	rc = kmsg_dump_register(&nvram_kmsg_dumper);
>  	if (rc != 0) {
>  		pr_err("nvram: kmsg_dump_register() failed; returned %d\n", rc);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] powerpc/pseries: Support compression of oops text via pstore

[Aruna Balakrishnaiah]

Hi Ben,

On Saturday 01 June 2013 10:24 AM, Benjamin Herrenschmidt wrote:
> On Fri, 2013-04-26 at 15:26 +0530, Aruna Balakrishnaiah wrote:
>> The patch set supports compression of oops messages while writing to NVRAM,
>> this helps in capturing more of oops data to lnx,oops-log. The pstore file
>> for oops messages will be in decompressed format making it readable.
>>
>> In case compression fails, the patch takes care of copying the header added
>> by pstore and last oops_data_sz bytes of big_oops_buf to NVRAM so that we
>> have recent oops messages in lnx,oops-log.
>>
>> In case decompression fails, it will result in absence of oops file but still
>> have files (in /dev/pstore) for other partitions.
> Any reason that isn't handled by pstore itself rather than here ? Ie
> make a flag indicating that the partition supports compression and have
> pstore do it (so we don't compress everything such as ofw common etc...)
>
> Cheers,
> Ben.
>

Since pstore does not have the compression support, I planned to reuse the
existing compression code in nvram but later we can add compression support
to pstore so that other platforms can make use of it.

Regards,
Aruna

>> Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
>> ---
>>   arch/powerpc/platforms/pseries/nvram.c |  132 +++++++++++++++++++++++++++++---
>>   1 file changed, 118 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
>> index 0159d74..b5ba5e2 100644
>> --- a/arch/powerpc/platforms/pseries/nvram.c
>> +++ b/arch/powerpc/platforms/pseries/nvram.c
>> @@ -539,6 +539,65 @@ static int zip_oops(size_t text_len)
>>   }
>>   
>>   #ifdef CONFIG_PSTORE
>> +/* Derived from logfs_uncompress */
>> +int nvram_decompress(void *in, void *out, size_t inlen, size_t outlen)
>> +{
>> +	int err, ret;
>> +
>> +	ret = -EIO;
>> +	err = zlib_inflateInit(&stream);
>> +	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_inflate(&stream, Z_FINISH);
>> +	if (err != Z_STREAM_END)
>> +		goto error;
>> +
>> +	err = zlib_inflateEnd(&stream);
>> +	if (err != Z_OK)
>> +		goto error;
>> +
>> +	ret = stream.total_out;
>> +error:
>> +	return ret;
>> +}
>> +
>> +static int unzip_oops(char *oops_buf, char *big_buf)
>> +{
>> +	struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
>> +	u64 timestamp = oops_hdr->timestamp;
>> +	char *big_oops_data = NULL;
>> +	char *oops_data_buf = NULL;
>> +	size_t big_oops_data_sz;
>> +	int unzipped_len;
>> +
>> +	big_oops_data = big_buf + sizeof(struct oops_log_info);
>> +	big_oops_data_sz = big_oops_buf_sz - sizeof(struct oops_log_info);
>> +	oops_data_buf = oops_buf + sizeof(struct oops_log_info);
>> +
>> +	unzipped_len = nvram_decompress(oops_data_buf, big_oops_data,
>> +					oops_hdr->report_length,
>> +					big_oops_data_sz);
>> +
>> +	if (unzipped_len < 0) {
>> +		pr_err("nvram: decompression failed; returned %d\n",
>> +								unzipped_len);
>> +		return -1;
>> +	}
>> +	oops_hdr = (struct oops_log_info *)big_buf;
>> +	oops_hdr->version = OOPS_HDR_VERSION;
>> +	oops_hdr->report_length = (u16) unzipped_len;
>> +	oops_hdr->timestamp = timestamp;
>> +	return 0;
>> +}
>> +
>>   static int nvram_pstore_open(struct pstore_info *psi)
>>   {
>>   	/* Reset the iterator to start reading partitions again */
>> @@ -567,6 +626,7 @@ static int nvram_pstore_write(enum pstore_type_id type,
>>   				size_t size, struct pstore_info *psi)
>>   {
>>   	int rc;
>> +	unsigned int err_type = ERR_TYPE_KERNEL_PANIC;
>>   	struct oops_log_info *oops_hdr = (struct oops_log_info *) oops_buf;
>>   
>>   	/* part 1 has the recent messages from printk buffer */
>> @@ -577,8 +637,31 @@ static int nvram_pstore_write(enum pstore_type_id type,
>>   	oops_hdr->version = OOPS_HDR_VERSION;
>>   	oops_hdr->report_length = (u16) size;
>>   	oops_hdr->timestamp = get_seconds();
>> +
>> +	if (big_oops_buf) {
>> +		rc = zip_oops(size);
>> +		/*
>> +		 * If compression fails copy recent log messages from
>> +		 * big_oops_buf to oops_data.
>> +		 */
>> +		if (rc != 0) {
>> +			int hsize = pstore_get_header_size();
>> +			size_t diff = size - oops_data_sz + hsize;
>> +
>> +			if (size > oops_data_sz) {
>> +				memcpy(oops_data, big_oops_buf, hsize);
>> +				memcpy(oops_data + hsize, big_oops_buf + diff,
>> +					oops_data_sz - hsize);
>> +
>> +				oops_hdr->report_length = (u16) oops_data_sz;
>> +			} else
>> +				memcpy(oops_data, big_oops_buf, size);
>> +		} else
>> +			err_type = ERR_TYPE_KERNEL_PANIC_GZ;
>> +	}
>> +
>>   	rc = nvram_write_os_partition(&oops_log_partition, oops_buf,
>> -		(int) (sizeof(*oops_hdr) + size), ERR_TYPE_KERNEL_PANIC,
>> +		(int) (sizeof(*oops_hdr) + oops_hdr->report_length), err_type,
>>   		count);
>>   
>>   	if (rc != 0)
>> @@ -600,10 +683,11 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
>>   	struct oops_log_info *oops_hdr;
>>   	unsigned int err_type, id_no, size = 0;
>>   	struct nvram_os_partition *part = NULL;
>> -	char *buff = NULL;
>> -	int sig = 0;
>> +	char *buff = NULL, *big_buff = NULL;
>> +	int rc, sig = 0;
>>   	loff_t p;
>>   
>> +read_partition:
>>   	read_type++;
>>   
>>   	switch (nvram_type_ids[read_type]) {
>> @@ -666,6 +750,25 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
>>   	if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) {
>>   		oops_hdr = (struct oops_log_info *)buff;
>>   		*buf = buff + sizeof(*oops_hdr);
>> +
>> +		if (err_type == ERR_TYPE_KERNEL_PANIC_GZ) {
>> +			big_buff = kmalloc(big_oops_buf_sz, GFP_KERNEL);
>> +			if (!big_buff)
>> +				return -ENOMEM;
>> +
>> +			rc = unzip_oops(buff, big_buff);
>> +
>> +			if (rc != 0) {
>> +				kfree(buff);
>> +				kfree(big_buff);
>> +				goto read_partition;
>> +			}
>> +
>> +			oops_hdr = (struct oops_log_info *)big_buff;
>> +			*buf = big_buff + sizeof(*oops_hdr);
>> +			kfree(buff);
>> +		}
>> +
>>   		time->tv_sec = oops_hdr->timestamp;
>>   		time->tv_nsec = 0;
>>   		return oops_hdr->report_length;
>> @@ -687,17 +790,18 @@ static int nvram_pstore_init(void)
>>   {
>>   	int rc = 0;
>>   
>> -	nvram_pstore_info.buf = oops_data;
>> -	nvram_pstore_info.bufsize = oops_data_sz;
>> +	if (big_oops_buf) {
>> +		nvram_pstore_info.buf = big_oops_buf;
>> +		nvram_pstore_info.bufsize = big_oops_buf_sz;
>> +	} else {
>> +		nvram_pstore_info.buf = oops_data;
>> +		nvram_pstore_info.bufsize = oops_data_sz;
>> +	}
>>   
>>   	rc = pstore_register(&nvram_pstore_info);
>>   	if (rc != 0)
>>   		pr_err("nvram: pstore_register() failed, defaults to "
>>   				"kmsg_dump; returned %d\n", rc);
>> -	else
>> -		/*TODO: Support compression when pstore is configured */
>> -		pr_info("nvram: Compression of oops text supported only when "
>> -				"pstore is not configured");
>>   
>>   	return rc;
>>   }
>> @@ -731,11 +835,6 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
>>   	oops_data = oops_buf + sizeof(struct oops_log_info);
>>   	oops_data_sz = oops_log_partition.size - sizeof(struct oops_log_info);
>>   
>> -	rc = nvram_pstore_init();
>> -
>> -	if (!rc)
>> -		return;
>> -
>>   	/*
>>   	 * Figure compression (preceded by elimination of each line's <n>
>>   	 * severity prefix) will reduce the oops/panic report to at most
>> @@ -759,6 +858,11 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
>>   		stream.workspace = NULL;
>>   	}
>>   
>> +	rc = nvram_pstore_init();
>> +
>> +	if (!rc)
>> +		return;
>> +
>>   	rc = kmsg_dump_register(&nvram_kmsg_dumper);
>>   	if (rc != 0) {
>>   		pr_err("nvram: kmsg_dump_register() failed; returned %d\n", rc);
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

Re: [RFC PATCH 0/3] Nvram-to-pstore: compression support for oops data

[Aruna Balakrishnaiah]

Hi Ben,

There is no change in this patchset and it applies cleanly on top of
v4 of Nvram-to-pstorepatches.

> The patchset takes care of compressing oops messages while writing to NVRAM,
> so that more oops data can be captured in the given space.
>
> big_oops_buf (2.22 * oops_data_sz) is allocated for compression.
> oops_data_sz is oops header size less of oops partition size.
>
> Pstore will internally call kmsg_dump to capture messages from printk
> buffer. While returning the data to nvram it adds is own header.
>
> For compression:
> 	Register pstore with big_oops_buf.
>
> In case compression fails, copy header added by pstore and
> last oops_data_sz bytes (recent messages) of big_oops_buf to
> nvram for which we need to know header size.
> 	
> patch 01/03 will add a function in pstore to return the header size.
>
> pstore read callback of nvram will read the compressed data and return the
> decompressed data so that dmesg file (under /dev/pstore) is readable.
>
> In case decompression fails, instead of having the compressed data (junk) in the
> dmesg file it will skip and continue reading other partitions. This results in
> absence of dmesg file but will still have files relating to other parititons.
>
>
> ---
>
> Aruna Balakrishnaiah (3):
>        Retreive header size from pstore.
>        powerpc/pseries: Re-organise the oops compression code
>        powerpc/pseries: Support compression of oops text via pstore
>
>
>   arch/powerpc/platforms/pseries/nvram.c |  236 +++++++++++++++++++++++---------
>   fs/pstore/platform.c                   |    7 +
>   include/linux/pstore.h                 |    6 +
>   3 files changed, 182 insertions(+), 67 deletions(-)
>

[RESEND PATCH 0/3] Nvram-to-pstore: compression support for oops data

[Aruna Balakrishnaiah]

The patchset takes care of compressing oops messages while writing to NVRAM,
so that more oops data can be captured in the given space.

big_oops_buf (2.22 * oops_data_sz) is allocated for compression.
oops_data_sz is oops header size less of oops partition size.

Pstore will internally call kmsg_dump to capture messages from printk
buffer. While returning the data to nvram it adds is own header.

For compression:
	Register pstore with big_oops_buf. 

In case compression fails, copy header added by pstore and
last oops_data_sz bytes (recent messages) of big_oops_buf to
nvram for which we need to know header size. 
	
patch 01/03 will add a function in pstore to return the header size.

pstore read callback of nvram will read the compressed data and return the
decompressed data so that dmesg file (under /dev/pstore) is readable.

In case decompression fails, instead of having the compressed data (junk) in the
dmesg file it will skip and continue reading other partitions. This results in
absence of dmesg file but will still have files relating to other parititons.

---

Aruna Balakrishnaiah (3):
      Retreive header size from pstore
      powerpc/pseries: Re-organise the oops compression code
      powerpc/pseries: Support compression of oops text via pstore


 arch/powerpc/platforms/pseries/nvram.c |  236 +++++++++++++++++++++++---------
 fs/pstore/platform.c                   |    7 +
 include/linux/pstore.h                 |    6 +
 3 files changed, 182 insertions(+), 67 deletions(-)

-- 

[PATCH 3/3] powerpc/pseries: Support compression of oops text via pstore

[Aruna Balakrishnaiah]

The patch set supports compression of oops messages while writing to NVRAM,
this helps in capturing more of oops data to lnx,oops-log. The pstore file
for oops messages will be in decompressed format making it readable.

In case compression fails, the patch takes care of copying the header added
by pstore and last oops_data_sz bytes of big_oops_buf to NVRAM so that we
have recent oops messages in lnx,oops-log.

In case decompression fails, it will result in absence of oops file but still
have files (in /dev/pstore) for other partitions.

Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/nvram.c |  132 +++++++++++++++++++++++++++++---
 1 file changed, 118 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
index 0159d74..b5ba5e2 100644
--- a/arch/powerpc/platforms/pseries/nvram.c
+++ b/arch/powerpc/platforms/pseries/nvram.c
@@ -539,6 +539,65 @@ static int zip_oops(size_t text_len)
 }
 
 #ifdef CONFIG_PSTORE
+/* Derived from logfs_uncompress */
+int nvram_decompress(void *in, void *out, size_t inlen, size_t outlen)
+{
+	int err, ret;
+
+	ret = -EIO;
+	err = zlib_inflateInit(&stream);
+	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_inflate(&stream, Z_FINISH);
+	if (err != Z_STREAM_END)
+		goto error;
+
+	err = zlib_inflateEnd(&stream);
+	if (err != Z_OK)
+		goto error;
+
+	ret = stream.total_out;
+error:
+	return ret;
+}
+
+static int unzip_oops(char *oops_buf, char *big_buf)
+{
+	struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
+	u64 timestamp = oops_hdr->timestamp;
+	char *big_oops_data = NULL;
+	char *oops_data_buf = NULL;
+	size_t big_oops_data_sz;
+	int unzipped_len;
+
+	big_oops_data = big_buf + sizeof(struct oops_log_info);
+	big_oops_data_sz = big_oops_buf_sz - sizeof(struct oops_log_info);
+	oops_data_buf = oops_buf + sizeof(struct oops_log_info);
+
+	unzipped_len = nvram_decompress(oops_data_buf, big_oops_data,
+					oops_hdr->report_length,
+					big_oops_data_sz);
+
+	if (unzipped_len < 0) {
+		pr_err("nvram: decompression failed; returned %d\n",
+								unzipped_len);
+		return -1;
+	}
+	oops_hdr = (struct oops_log_info *)big_buf;
+	oops_hdr->version = OOPS_HDR_VERSION;
+	oops_hdr->report_length = (u16) unzipped_len;
+	oops_hdr->timestamp = timestamp;
+	return 0;
+}
+
 static int nvram_pstore_open(struct pstore_info *psi)
 {
 	/* Reset the iterator to start reading partitions again */
@@ -567,6 +626,7 @@ static int nvram_pstore_write(enum pstore_type_id type,
 				size_t size, struct pstore_info *psi)
 {
 	int rc;
+	unsigned int err_type = ERR_TYPE_KERNEL_PANIC;
 	struct oops_log_info *oops_hdr = (struct oops_log_info *) oops_buf;
 
 	/* part 1 has the recent messages from printk buffer */
@@ -577,8 +637,31 @@ static int nvram_pstore_write(enum pstore_type_id type,
 	oops_hdr->version = OOPS_HDR_VERSION;
 	oops_hdr->report_length = (u16) size;
 	oops_hdr->timestamp = get_seconds();
+
+	if (big_oops_buf) {
+		rc = zip_oops(size);
+		/*
+		 * If compression fails copy recent log messages from
+		 * big_oops_buf to oops_data.
+		 */
+		if (rc != 0) {
+			int hsize = pstore_get_header_size();
+			size_t diff = size - oops_data_sz + hsize;
+
+			if (size > oops_data_sz) {
+				memcpy(oops_data, big_oops_buf, hsize);
+				memcpy(oops_data + hsize, big_oops_buf + diff,
+					oops_data_sz - hsize);
+
+				oops_hdr->report_length = (u16) oops_data_sz;
+			} else
+				memcpy(oops_data, big_oops_buf, size);
+		} else
+			err_type = ERR_TYPE_KERNEL_PANIC_GZ;
+	}
+
 	rc = nvram_write_os_partition(&oops_log_partition, oops_buf,
-		(int) (sizeof(*oops_hdr) + size), ERR_TYPE_KERNEL_PANIC,
+		(int) (sizeof(*oops_hdr) + oops_hdr->report_length), err_type,
 		count);
 
 	if (rc != 0)
@@ -600,10 +683,11 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
 	struct oops_log_info *oops_hdr;
 	unsigned int err_type, id_no, size = 0;
 	struct nvram_os_partition *part = NULL;
-	char *buff = NULL;
-	int sig = 0;
+	char *buff = NULL, *big_buff = NULL;
+	int rc, sig = 0;
 	loff_t p;
 
+read_partition:
 	read_type++;
 
 	switch (nvram_type_ids[read_type]) {
@@ -666,6 +750,25 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
 	if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) {
 		oops_hdr = (struct oops_log_info *)buff;
 		*buf = buff + sizeof(*oops_hdr);
+
+		if (err_type == ERR_TYPE_KERNEL_PANIC_GZ) {
+			big_buff = kmalloc(big_oops_buf_sz, GFP_KERNEL);
+			if (!big_buff)
+				return -ENOMEM;
+
+			rc = unzip_oops(buff, big_buff);
+
+			if (rc != 0) {
+				kfree(buff);
+				kfree(big_buff);
+				goto read_partition;
+			}
+
+			oops_hdr = (struct oops_log_info *)big_buff;
+			*buf = big_buff + sizeof(*oops_hdr);
+			kfree(buff);
+		}
+
 		time->tv_sec = oops_hdr->timestamp;
 		time->tv_nsec = 0;
 		return oops_hdr->report_length;
@@ -687,17 +790,18 @@ static int nvram_pstore_init(void)
 {
 	int rc = 0;
 
-	nvram_pstore_info.buf = oops_data;
-	nvram_pstore_info.bufsize = oops_data_sz;
+	if (big_oops_buf) {
+		nvram_pstore_info.buf = big_oops_buf;
+		nvram_pstore_info.bufsize = big_oops_buf_sz;
+	} else {
+		nvram_pstore_info.buf = oops_data;
+		nvram_pstore_info.bufsize = oops_data_sz;
+	}
 
 	rc = pstore_register(&nvram_pstore_info);
 	if (rc != 0)
 		pr_err("nvram: pstore_register() failed, defaults to "
 				"kmsg_dump; returned %d\n", rc);
-	else
-		/*TODO: Support compression when pstore is configured */
-		pr_info("nvram: Compression of oops text supported only when "
-				"pstore is not configured");
 
 	return rc;
 }
@@ -731,11 +835,6 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
 	oops_data = oops_buf + sizeof(struct oops_log_info);
 	oops_data_sz = oops_log_partition.size - sizeof(struct oops_log_info);
 
-	rc = nvram_pstore_init();
-
-	if (!rc)
-		return;
-
 	/*
 	 * Figure compression (preceded by elimination of each line's <n>
 	 * severity prefix) will reduce the oops/panic report to at most
@@ -759,6 +858,11 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
 		stream.workspace = NULL;
 	}
 
+	rc = nvram_pstore_init();
+
+	if (!rc)
+		return;
+
 	rc = kmsg_dump_register(&nvram_kmsg_dumper);
 	if (rc != 0) {
 		pr_err("nvram: kmsg_dump_register() failed; returned %d\n", rc);

Re: [PATCH 3/3] powerpc/pseries: Support compression of oops text via pstore

[Kees Cook]

On Sun, Jun 23, 2013 at 11:23 PM, Aruna Balakrishnaiah
<aruna@linux.vnet.ibm.com> wrote:
> The patch set supports compression of oops messages while writing to NVRAM,
> this helps in capturing more of oops data to lnx,oops-log. The pstore file
> for oops messages will be in decompressed format making it readable.
>
> In case compression fails, the patch takes care of copying the header added
> by pstore and last oops_data_sz bytes of big_oops_buf to NVRAM so that we
> have recent oops messages in lnx,oops-log.
>
> In case decompression fails, it will result in absence of oops file but still
> have files (in /dev/pstore) for other partitions.
>
> Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/nvram.c |  132 +++++++++++++++++++++++++++++---
>  1 file changed, 118 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
> index 0159d74..b5ba5e2 100644
> --- a/arch/powerpc/platforms/pseries/nvram.c
> +++ b/arch/powerpc/platforms/pseries/nvram.c
> @@ -539,6 +539,65 @@ static int zip_oops(size_t text_len)
>  }
>
>  #ifdef CONFIG_PSTORE
> +/* Derived from logfs_uncompress */
> +int nvram_decompress(void *in, void *out, size_t inlen, size_t outlen)
> +{
> +       int err, ret;
> +
> +       ret = -EIO;
> +       err = zlib_inflateInit(&stream);
> +       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_inflate(&stream, Z_FINISH);
> +       if (err != Z_STREAM_END)
> +               goto error;
> +
> +       err = zlib_inflateEnd(&stream);
> +       if (err != Z_OK)
> +               goto error;
> +
> +       ret = stream.total_out;
> +error:
> +       return ret;
> +}
> +
> +static int unzip_oops(char *oops_buf, char *big_buf)
> +{
> +       struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
> +       u64 timestamp = oops_hdr->timestamp;
> +       char *big_oops_data = NULL;
> +       char *oops_data_buf = NULL;
> +       size_t big_oops_data_sz;
> +       int unzipped_len;
> +
> +       big_oops_data = big_buf + sizeof(struct oops_log_info);
> +       big_oops_data_sz = big_oops_buf_sz - sizeof(struct oops_log_info);
> +       oops_data_buf = oops_buf + sizeof(struct oops_log_info);
> +
> +       unzipped_len = nvram_decompress(oops_data_buf, big_oops_data,
> +                                       oops_hdr->report_length,
> +                                       big_oops_data_sz);
> +
> +       if (unzipped_len < 0) {
> +               pr_err("nvram: decompression failed; returned %d\n",
> +                                                               unzipped_len);
> +               return -1;
> +       }
> +       oops_hdr = (struct oops_log_info *)big_buf;
> +       oops_hdr->version = OOPS_HDR_VERSION;
> +       oops_hdr->report_length = (u16) unzipped_len;
> +       oops_hdr->timestamp = timestamp;
> +       return 0;
> +}
> +
>  static int nvram_pstore_open(struct pstore_info *psi)
>  {
>         /* Reset the iterator to start reading partitions again */
> @@ -567,6 +626,7 @@ static int nvram_pstore_write(enum pstore_type_id type,
>                                 size_t size, struct pstore_info *psi)
>  {
>         int rc;
> +       unsigned int err_type = ERR_TYPE_KERNEL_PANIC;
>         struct oops_log_info *oops_hdr = (struct oops_log_info *) oops_buf;
>
>         /* part 1 has the recent messages from printk buffer */
> @@ -577,8 +637,31 @@ static int nvram_pstore_write(enum pstore_type_id type,
>         oops_hdr->version = OOPS_HDR_VERSION;
>         oops_hdr->report_length = (u16) size;
>         oops_hdr->timestamp = get_seconds();
> +
> +       if (big_oops_buf) {
> +               rc = zip_oops(size);
> +               /*
> +                * If compression fails copy recent log messages from
> +                * big_oops_buf to oops_data.
> +                */
> +               if (rc != 0) {
> +                       int hsize = pstore_get_header_size();

I think I would rather see the API to pstore_write() changed to
include explicit details about header sizes. Mkaing hsize a global
seems unwise, since it's not strictly going to be a constant value. It
could change between calls to the writer, for example.

Beyond that, this all seems sensible, though it would be kind of cool
to move this compression logic into the pstore core so it would get
used by default (or through a module parameter).

-Kees

> +                       size_t diff = size - oops_data_sz + hsize;
> +
> +                       if (size > oops_data_sz) {
> +                               memcpy(oops_data, big_oops_buf, hsize);
> +                               memcpy(oops_data + hsize, big_oops_buf + diff,
> +                                       oops_data_sz - hsize);
> +
> +                               oops_hdr->report_length = (u16) oops_data_sz;
> +                       } else
> +                               memcpy(oops_data, big_oops_buf, size);
> +               } else
> +                       err_type = ERR_TYPE_KERNEL_PANIC_GZ;
> +       }
> +
>         rc = nvram_write_os_partition(&oops_log_partition, oops_buf,
> -               (int) (sizeof(*oops_hdr) + size), ERR_TYPE_KERNEL_PANIC,
> +               (int) (sizeof(*oops_hdr) + oops_hdr->report_length), err_type,
>                 count);
>
>         if (rc != 0)
> @@ -600,10 +683,11 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
>         struct oops_log_info *oops_hdr;
>         unsigned int err_type, id_no, size = 0;
>         struct nvram_os_partition *part = NULL;
> -       char *buff = NULL;
> -       int sig = 0;
> +       char *buff = NULL, *big_buff = NULL;
> +       int rc, sig = 0;
>         loff_t p;
>
> +read_partition:
>         read_type++;
>
>         switch (nvram_type_ids[read_type]) {
> @@ -666,6 +750,25 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
>         if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) {
>                 oops_hdr = (struct oops_log_info *)buff;
>                 *buf = buff + sizeof(*oops_hdr);
> +
> +               if (err_type == ERR_TYPE_KERNEL_PANIC_GZ) {
> +                       big_buff = kmalloc(big_oops_buf_sz, GFP_KERNEL);
> +                       if (!big_buff)
> +                               return -ENOMEM;
> +
> +                       rc = unzip_oops(buff, big_buff);
> +
> +                       if (rc != 0) {
> +                               kfree(buff);
> +                               kfree(big_buff);
> +                               goto read_partition;
> +                       }
> +
> +                       oops_hdr = (struct oops_log_info *)big_buff;
> +                       *buf = big_buff + sizeof(*oops_hdr);
> +                       kfree(buff);
> +               }
> +
>                 time->tv_sec = oops_hdr->timestamp;
>                 time->tv_nsec = 0;
>                 return oops_hdr->report_length;
> @@ -687,17 +790,18 @@ static int nvram_pstore_init(void)
>  {
>         int rc = 0;
>
> -       nvram_pstore_info.buf = oops_data;
> -       nvram_pstore_info.bufsize = oops_data_sz;
> +       if (big_oops_buf) {
> +               nvram_pstore_info.buf = big_oops_buf;
> +               nvram_pstore_info.bufsize = big_oops_buf_sz;
> +       } else {
> +               nvram_pstore_info.buf = oops_data;
> +               nvram_pstore_info.bufsize = oops_data_sz;
> +       }
>
>         rc = pstore_register(&nvram_pstore_info);
>         if (rc != 0)
>                 pr_err("nvram: pstore_register() failed, defaults to "
>                                 "kmsg_dump; returned %d\n", rc);
> -       else
> -               /*TODO: Support compression when pstore is configured */
> -               pr_info("nvram: Compression of oops text supported only when "
> -                               "pstore is not configured");
>
>         return rc;
>  }
> @@ -731,11 +835,6 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
>         oops_data = oops_buf + sizeof(struct oops_log_info);
>         oops_data_sz = oops_log_partition.size - sizeof(struct oops_log_info);
>
> -       rc = nvram_pstore_init();
> -
> -       if (!rc)
> -               return;
> -
>         /*
>          * Figure compression (preceded by elimination of each line's <n>
>          * severity prefix) will reduce the oops/panic report to at most
> @@ -759,6 +858,11 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
>                 stream.workspace = NULL;
>         }
>
> +       rc = nvram_pstore_init();
> +
> +       if (!rc)
> +               return;
> +
>         rc = kmsg_dump_register(&nvram_kmsg_dumper);
>         if (rc != 0) {
>                 pr_err("nvram: kmsg_dump_register() failed; returned %d\n", rc);
>



--
Kees Cook
Chrome OS Security

Re: [PATCH 3/3] powerpc/pseries: Support compression of oops text via pstore

[Aruna Balakrishnaiah]

Hi Kees,

On Monday 24 June 2013 11:27 PM, Kees Cook wrote:
> On Sun, Jun 23, 2013 at 11:23 PM, Aruna Balakrishnaiah
> <aruna@linux.vnet.ibm.com> wrote:
>> The patch set supports compression of oops messages while writing to NVRAM,
>> this helps in capturing more of oops data to lnx,oops-log. The pstore file
>> for oops messages will be in decompressed format making it readable.
>>
>> In case compression fails, the patch takes care of copying the header added
>> by pstore and last oops_data_sz bytes of big_oops_buf to NVRAM so that we
>> have recent oops messages in lnx,oops-log.
>>
>> In case decompression fails, it will result in absence of oops file but still
>> have files (in /dev/pstore) for other partitions.
>>
>> Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
>> ---
>>   arch/powerpc/platforms/pseries/nvram.c |  132 +++++++++++++++++++++++++++++---
>>   1 file changed, 118 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
>> index 0159d74..b5ba5e2 100644
>> --- a/arch/powerpc/platforms/pseries/nvram.c
>> +++ b/arch/powerpc/platforms/pseries/nvram.c
>> @@ -539,6 +539,65 @@ static int zip_oops(size_t text_len)
>>   }
>>
>>   #ifdef CONFIG_PSTORE
>> +/* Derived from logfs_uncompress */
>> +int nvram_decompress(void *in, void *out, size_t inlen, size_t outlen)
>> +{
>> +       int err, ret;
>> +
>> +       ret = -EIO;
>> +       err = zlib_inflateInit(&stream);
>> +       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_inflate(&stream, Z_FINISH);
>> +       if (err != Z_STREAM_END)
>> +               goto error;
>> +
>> +       err = zlib_inflateEnd(&stream);
>> +       if (err != Z_OK)
>> +               goto error;
>> +
>> +       ret = stream.total_out;
>> +error:
>> +       return ret;
>> +}
>> +
>> +static int unzip_oops(char *oops_buf, char *big_buf)
>> +{
>> +       struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
>> +       u64 timestamp = oops_hdr->timestamp;
>> +       char *big_oops_data = NULL;
>> +       char *oops_data_buf = NULL;
>> +       size_t big_oops_data_sz;
>> +       int unzipped_len;
>> +
>> +       big_oops_data = big_buf + sizeof(struct oops_log_info);
>> +       big_oops_data_sz = big_oops_buf_sz - sizeof(struct oops_log_info);
>> +       oops_data_buf = oops_buf + sizeof(struct oops_log_info);
>> +
>> +       unzipped_len = nvram_decompress(oops_data_buf, big_oops_data,
>> +                                       oops_hdr->report_length,
>> +                                       big_oops_data_sz);
>> +
>> +       if (unzipped_len < 0) {
>> +               pr_err("nvram: decompression failed; returned %d\n",
>> +                                                               unzipped_len);
>> +               return -1;
>> +       }
>> +       oops_hdr = (struct oops_log_info *)big_buf;
>> +       oops_hdr->version = OOPS_HDR_VERSION;
>> +       oops_hdr->report_length = (u16) unzipped_len;
>> +       oops_hdr->timestamp = timestamp;
>> +       return 0;
>> +}
>> +
>>   static int nvram_pstore_open(struct pstore_info *psi)
>>   {
>>          /* Reset the iterator to start reading partitions again */
>> @@ -567,6 +626,7 @@ static int nvram_pstore_write(enum pstore_type_id type,
>>                                  size_t size, struct pstore_info *psi)
>>   {
>>          int rc;
>> +       unsigned int err_type = ERR_TYPE_KERNEL_PANIC;
>>          struct oops_log_info *oops_hdr = (struct oops_log_info *) oops_buf;
>>
>>          /* part 1 has the recent messages from printk buffer */
>> @@ -577,8 +637,31 @@ static int nvram_pstore_write(enum pstore_type_id type,
>>          oops_hdr->version = OOPS_HDR_VERSION;
>>          oops_hdr->report_length = (u16) size;
>>          oops_hdr->timestamp = get_seconds();
>> +
>> +       if (big_oops_buf) {
>> +               rc = zip_oops(size);
>> +               /*
>> +                * If compression fails copy recent log messages from
>> +                * big_oops_buf to oops_data.
>> +                */
>> +               if (rc != 0) {
>> +                       int hsize = pstore_get_header_size();
> I think I would rather see the API to pstore_write() changed to
> include explicit details about header sizes. Mkaing hsize a global
> seems unwise, since it's not strictly going to be a constant value. It
> could change between calls to the writer, for example.

Introducing headersize in pstore_write() API would need changes at
multiple places whereits being called. The idea is to move the
compression support to pstore infrastructure so that other platforms
could also make use of it. Once the compression support gets in,
header size argument in pstore_write() will have to be deprecated.

Till the time compression support for pstore goes in, can't we call
pstore_header_size before every write call to knowthe header size.

> Beyond that, this all seems sensible, though it would be kind of cool
> to move this compression logic into the pstore core so it would get
> used by default (or through a module parameter).
> -Kees
>
>> +                       size_t diff = size - oops_data_sz + hsize;
>> +
>> +                       if (size > oops_data_sz) {
>> +                               memcpy(oops_data, big_oops_buf, hsize);
>> +                               memcpy(oops_data + hsize, big_oops_buf + diff,
>> +                                       oops_data_sz - hsize);
>> +
>> +                               oops_hdr->report_length = (u16) oops_data_sz;
>> +                       } else
>> +                               memcpy(oops_data, big_oops_buf, size);
>> +               } else
>> +                       err_type = ERR_TYPE_KERNEL_PANIC_GZ;
>> +       }
>> +
>>          rc = nvram_write_os_partition(&oops_log_partition, oops_buf,
>> -               (int) (sizeof(*oops_hdr) + size), ERR_TYPE_KERNEL_PANIC,
>> +               (int) (sizeof(*oops_hdr) + oops_hdr->report_length), err_type,
>>                  count);
>>
>>          if (rc != 0)
>> @@ -600,10 +683,11 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
>>          struct oops_log_info *oops_hdr;
>>          unsigned int err_type, id_no, size = 0;
>>          struct nvram_os_partition *part = NULL;
>> -       char *buff = NULL;
>> -       int sig = 0;
>> +       char *buff = NULL, *big_buff = NULL;
>> +       int rc, sig = 0;
>>          loff_t p;
>>
>> +read_partition:
>>          read_type++;
>>
>>          switch (nvram_type_ids[read_type]) {
>> @@ -666,6 +750,25 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
>>          if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) {
>>                  oops_hdr = (struct oops_log_info *)buff;
>>                  *buf = buff + sizeof(*oops_hdr);
>> +
>> +               if (err_type == ERR_TYPE_KERNEL_PANIC_GZ) {
>> +                       big_buff = kmalloc(big_oops_buf_sz, GFP_KERNEL);
>> +                       if (!big_buff)
>> +                               return -ENOMEM;
>> +
>> +                       rc = unzip_oops(buff, big_buff);
>> +
>> +                       if (rc != 0) {
>> +                               kfree(buff);
>> +                               kfree(big_buff);
>> +                               goto read_partition;
>> +                       }
>> +
>> +                       oops_hdr = (struct oops_log_info *)big_buff;
>> +                       *buf = big_buff + sizeof(*oops_hdr);
>> +                       kfree(buff);
>> +               }
>> +
>>                  time->tv_sec = oops_hdr->timestamp;
>>                  time->tv_nsec = 0;
>>                  return oops_hdr->report_length;
>> @@ -687,17 +790,18 @@ static int nvram_pstore_init(void)
>>   {
>>          int rc = 0;
>>
>> -       nvram_pstore_info.buf = oops_data;
>> -       nvram_pstore_info.bufsize = oops_data_sz;
>> +       if (big_oops_buf) {
>> +               nvram_pstore_info.buf = big_oops_buf;
>> +               nvram_pstore_info.bufsize = big_oops_buf_sz;
>> +       } else {
>> +               nvram_pstore_info.buf = oops_data;
>> +               nvram_pstore_info.bufsize = oops_data_sz;
>> +       }
>>
>>          rc = pstore_register(&nvram_pstore_info);
>>          if (rc != 0)
>>                  pr_err("nvram: pstore_register() failed, defaults to "
>>                                  "kmsg_dump; returned %d\n", rc);
>> -       else
>> -               /*TODO: Support compression when pstore is configured */
>> -               pr_info("nvram: Compression of oops text supported only when "
>> -                               "pstore is not configured");
>>
>>          return rc;
>>   }
>> @@ -731,11 +835,6 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
>>          oops_data = oops_buf + sizeof(struct oops_log_info);
>>          oops_data_sz = oops_log_partition.size - sizeof(struct oops_log_info);
>>
>> -       rc = nvram_pstore_init();
>> -
>> -       if (!rc)
>> -               return;
>> -
>>          /*
>>           * Figure compression (preceded by elimination of each line's <n>
>>           * severity prefix) will reduce the oops/panic report to at most
>> @@ -759,6 +858,11 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
>>                  stream.workspace = NULL;
>>          }
>>
>> +       rc = nvram_pstore_init();
>> +
>> +       if (!rc)
>> +               return;
>> +
>>          rc = kmsg_dump_register(&nvram_kmsg_dumper);
>>          if (rc != 0) {
>>                  pr_err("nvram: kmsg_dump_register() failed; returned %d\n", rc);
>>
>
>
> --
> Kees Cook
> Chrome OS Security
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

RE: [PATCH 3/3] powerpc/pseries: Support compression of oops text via pstore

[Luck, Tony]

> Introducing headersize in pstore_write() API would need changes at
> multiple places whereits being called. The idea is to move the
> compression support to pstore infrastructure so that other platforms
> could also make use of it.

Any thoughts on the back/forward compatibility as we switch to compressed
pstore data?   E.g. imagine I have a system installed with some Linux distr=
ibution
with a kernel too old to know about compressed pstore. I use that machine t=
o
run the latest kernels that do compression ... and one fine day one of them=
 crashes
hard - logging in compressed form to pstore. Now I boot my distro kernel to=
 pick
up the pieces ... what do I see in /sys/fs/pstore/*? Some compressed files?=
 Can I
read them with some tool?

This somewhat of a corner case - but not completely unrealistic ... I'd at =
least
like to be reassured that the old kernel won't choke when it sees the compr=
essed
blobs.

-Tony

Re: [PATCH 3/3] powerpc/pseries: Support compression of oops text via pstore

[Aruna Balakrishnaiah]

Hi Tony,

On Tuesday 25 June 2013 09:32 PM, Luck, Tony wrote:
>> Introducing headersize in pstore_write() API would need changes at
>> multiple places whereits being called. The idea is to move the
>> compression support to pstore infrastructure so that other platforms
>> could also make use of it.
> Any thoughts on the back/forward compatibility as we switch to compressed
> pstore data?   E.g. imagine I have a system installed with some Linux distribution
> with a kernel too old to know about compressed pstore. I use that machine to
> run the latest kernels that do compression ... and one fine day one of them crashes
> hard - logging in compressed form to pstore. Now I boot my distro kernel to pick
> up the pieces ... what do I see in /sys/fs/pstore/*? Some compressed files? Can I
> read them with some tool?
>
> This somewhat of a corner case - but not completely unrealistic ... I'd at least
> like to be reassured that the old kernel won't choke when it sees the compressed
> blobs.

openssl command line tool can be used to decompress the compressed data of
the pstore file in the above scenario.

Usage:

cat <file> | openssl zlib -d

> -Tony
>

Re: [PATCH 3/3] powerpc/pseries: Support compression of oops text via pstore

[Kees Cook]

On Tue, Jun 25, 2013 at 12:04 AM, Aruna Balakrishnaiah
<aruna@linux.vnet.ibm.com> wrote:
> Hi Kees,
>
>
> On Monday 24 June 2013 11:27 PM, Kees Cook wrote:
>>
>> On Sun, Jun 23, 2013 at 11:23 PM, Aruna Balakrishnaiah
>> <aruna@linux.vnet.ibm.com> wrote:
>>>
>>> The patch set supports compression of oops messages while writing to
>>> NVRAM,
>>> this helps in capturing more of oops data to lnx,oops-log. The pstore
>>> file
>>> for oops messages will be in decompressed format making it readable.
>>>
>>> In case compression fails, the patch takes care of copying the header
>>> added
>>> by pstore and last oops_data_sz bytes of big_oops_buf to NVRAM so that we
>>> have recent oops messages in lnx,oops-log.
>>>
>>> In case decompression fails, it will result in absence of oops file but
>>> still
>>> have files (in /dev/pstore) for other partitions.
>>>
>>> Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
>>> ---
>>>   arch/powerpc/platforms/pseries/nvram.c |  132
>>> +++++++++++++++++++++++++++++---
>>>   1 file changed, 118 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/nvram.c
>>> b/arch/powerpc/platforms/pseries/nvram.c
>>> index 0159d74..b5ba5e2 100644
>>> --- a/arch/powerpc/platforms/pseries/nvram.c
>>> +++ b/arch/powerpc/platforms/pseries/nvram.c
>>> @@ -539,6 +539,65 @@ static int zip_oops(size_t text_len)
>>>   }
>>>
>>>   #ifdef CONFIG_PSTORE
>>> +/* Derived from logfs_uncompress */
>>> +int nvram_decompress(void *in, void *out, size_t inlen, size_t outlen)
>>> +{
>>> +       int err, ret;
>>> +
>>> +       ret = -EIO;
>>> +       err = zlib_inflateInit(&stream);
>>> +       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_inflate(&stream, Z_FINISH);
>>> +       if (err != Z_STREAM_END)
>>> +               goto error;
>>> +
>>> +       err = zlib_inflateEnd(&stream);
>>> +       if (err != Z_OK)
>>> +               goto error;
>>> +
>>> +       ret = stream.total_out;
>>> +error:
>>> +       return ret;
>>> +}
>>> +
>>> +static int unzip_oops(char *oops_buf, char *big_buf)
>>> +{
>>> +       struct oops_log_info *oops_hdr = (struct oops_log_info
>>> *)oops_buf;
>>> +       u64 timestamp = oops_hdr->timestamp;
>>> +       char *big_oops_data = NULL;
>>> +       char *oops_data_buf = NULL;
>>> +       size_t big_oops_data_sz;
>>> +       int unzipped_len;
>>> +
>>> +       big_oops_data = big_buf + sizeof(struct oops_log_info);
>>> +       big_oops_data_sz = big_oops_buf_sz - sizeof(struct
>>> oops_log_info);
>>> +       oops_data_buf = oops_buf + sizeof(struct oops_log_info);
>>> +
>>> +       unzipped_len = nvram_decompress(oops_data_buf, big_oops_data,
>>> +                                       oops_hdr->report_length,
>>> +                                       big_oops_data_sz);
>>> +
>>> +       if (unzipped_len < 0) {
>>> +               pr_err("nvram: decompression failed; returned %d\n",
>>> +
>>> unzipped_len);
>>> +               return -1;
>>> +       }
>>> +       oops_hdr = (struct oops_log_info *)big_buf;
>>> +       oops_hdr->version = OOPS_HDR_VERSION;
>>> +       oops_hdr->report_length = (u16) unzipped_len;
>>> +       oops_hdr->timestamp = timestamp;
>>> +       return 0;
>>> +}
>>> +
>>>   static int nvram_pstore_open(struct pstore_info *psi)
>>>   {
>>>          /* Reset the iterator to start reading partitions again */
>>> @@ -567,6 +626,7 @@ static int nvram_pstore_write(enum pstore_type_id
>>> type,
>>>                                  size_t size, struct pstore_info *psi)
>>>   {
>>>          int rc;
>>> +       unsigned int err_type = ERR_TYPE_KERNEL_PANIC;
>>>          struct oops_log_info *oops_hdr = (struct oops_log_info *)
>>> oops_buf;
>>>
>>>          /* part 1 has the recent messages from printk buffer */
>>> @@ -577,8 +637,31 @@ static int nvram_pstore_write(enum pstore_type_id
>>> type,
>>>          oops_hdr->version = OOPS_HDR_VERSION;
>>>          oops_hdr->report_length = (u16) size;
>>>          oops_hdr->timestamp = get_seconds();
>>> +
>>> +       if (big_oops_buf) {
>>> +               rc = zip_oops(size);
>>> +               /*
>>> +                * If compression fails copy recent log messages from
>>> +                * big_oops_buf to oops_data.
>>> +                */
>>> +               if (rc != 0) {
>>> +                       int hsize = pstore_get_header_size();
>>
>> I think I would rather see the API to pstore_write() changed to
>> include explicit details about header sizes. Mkaing hsize a global
>> seems unwise, since it's not strictly going to be a constant value. It
>> could change between calls to the writer, for example.
>
>
> Introducing headersize in pstore_write() API would need changes at
> multiple places whereits being called. The idea is to move the
> compression support to pstore infrastructure so that other platforms
> could also make use of it. Once the compression support gets in,
> header size argument in pstore_write() will have to be deprecated.
>
> Till the time compression support for pstore goes in, can't we call
> pstore_header_size before every write call to knowthe header size.

I think it would be cleaner for it to be passed as part of the
pstore_write function. It does mean touching all the other callers,
but using a global variable for this is just wrong, IMO. :)

It would be a progression, at some point in the future, the
pstore_write function could lose the header size argument when it was
the right time.

-Kees

>
>> Beyond that, this all seems sensible, though it would be kind of cool
>> to move this compression logic into the pstore core so it would get
>> used by default (or through a module parameter).
>> -Kees
>>
>>> +                       size_t diff = size - oops_data_sz + hsize;
>>> +
>>> +                       if (size > oops_data_sz) {
>>> +                               memcpy(oops_data, big_oops_buf, hsize);
>>> +                               memcpy(oops_data + hsize, big_oops_buf +
>>> diff,
>>> +                                       oops_data_sz - hsize);
>>> +
>>> +                               oops_hdr->report_length = (u16)
>>> oops_data_sz;
>>> +                       } else
>>> +                               memcpy(oops_data, big_oops_buf, size);
>>> +               } else
>>> +                       err_type = ERR_TYPE_KERNEL_PANIC_GZ;
>>> +       }
>>> +
>>>          rc = nvram_write_os_partition(&oops_log_partition, oops_buf,
>>> -               (int) (sizeof(*oops_hdr) + size), ERR_TYPE_KERNEL_PANIC,
>>> +               (int) (sizeof(*oops_hdr) + oops_hdr->report_length),
>>> err_type,
>>>                  count);
>>>
>>>          if (rc != 0)
>>> @@ -600,10 +683,11 @@ static ssize_t nvram_pstore_read(u64 *id, enum
>>> pstore_type_id *type,
>>>          struct oops_log_info *oops_hdr;
>>>          unsigned int err_type, id_no, size = 0;
>>>          struct nvram_os_partition *part = NULL;
>>> -       char *buff = NULL;
>>> -       int sig = 0;
>>> +       char *buff = NULL, *big_buff = NULL;
>>> +       int rc, sig = 0;
>>>          loff_t p;
>>>
>>> +read_partition:
>>>          read_type++;
>>>
>>>          switch (nvram_type_ids[read_type]) {
>>> @@ -666,6 +750,25 @@ static ssize_t nvram_pstore_read(u64 *id, enum
>>> pstore_type_id *type,
>>>          if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) {
>>>                  oops_hdr = (struct oops_log_info *)buff;
>>>                  *buf = buff + sizeof(*oops_hdr);
>>> +
>>> +               if (err_type == ERR_TYPE_KERNEL_PANIC_GZ) {
>>> +                       big_buff = kmalloc(big_oops_buf_sz, GFP_KERNEL);
>>> +                       if (!big_buff)
>>> +                               return -ENOMEM;
>>> +
>>> +                       rc = unzip_oops(buff, big_buff);
>>> +
>>> +                       if (rc != 0) {
>>> +                               kfree(buff);
>>> +                               kfree(big_buff);
>>> +                               goto read_partition;
>>> +                       }
>>> +
>>> +                       oops_hdr = (struct oops_log_info *)big_buff;
>>> +                       *buf = big_buff + sizeof(*oops_hdr);
>>> +                       kfree(buff);
>>> +               }
>>> +
>>>                  time->tv_sec = oops_hdr->timestamp;
>>>                  time->tv_nsec = 0;
>>>                  return oops_hdr->report_length;
>>> @@ -687,17 +790,18 @@ static int nvram_pstore_init(void)
>>>   {
>>>          int rc = 0;
>>>
>>> -       nvram_pstore_info.buf = oops_data;
>>> -       nvram_pstore_info.bufsize = oops_data_sz;
>>> +       if (big_oops_buf) {
>>> +               nvram_pstore_info.buf = big_oops_buf;
>>> +               nvram_pstore_info.bufsize = big_oops_buf_sz;
>>> +       } else {
>>> +               nvram_pstore_info.buf = oops_data;
>>> +               nvram_pstore_info.bufsize = oops_data_sz;
>>> +       }
>>>
>>>          rc = pstore_register(&nvram_pstore_info);
>>>          if (rc != 0)
>>>                  pr_err("nvram: pstore_register() failed, defaults to "
>>>                                  "kmsg_dump; returned %d\n", rc);
>>> -       else
>>> -               /*TODO: Support compression when pstore is configured */
>>> -               pr_info("nvram: Compression of oops text supported only
>>> when "
>>> -                               "pstore is not configured");
>>>
>>>          return rc;
>>>   }
>>> @@ -731,11 +835,6 @@ static void __init nvram_init_oops_partition(int
>>> rtas_partition_exists)
>>>          oops_data = oops_buf + sizeof(struct oops_log_info);
>>>          oops_data_sz = oops_log_partition.size - sizeof(struct
>>> oops_log_info);
>>>
>>> -       rc = nvram_pstore_init();
>>> -
>>> -       if (!rc)
>>> -               return;
>>> -
>>>          /*
>>>           * Figure compression (preceded by elimination of each line's
>>> <n>
>>>           * severity prefix) will reduce the oops/panic report to at most
>>> @@ -759,6 +858,11 @@ static void __init nvram_init_oops_partition(int
>>> rtas_partition_exists)
>>>                  stream.workspace = NULL;
>>>          }
>>>
>>> +       rc = nvram_pstore_init();
>>> +
>>> +       if (!rc)
>>> +               return;
>>> +
>>>          rc = kmsg_dump_register(&nvram_kmsg_dumper);
>>>          if (rc != 0) {
>>>                  pr_err("nvram: kmsg_dump_register() failed; returned
>>> %d\n", rc);
>>>
>>
>>
>> --
>> Kees Cook
>> Chrome OS Security
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>>
>



--
Kees Cook
Chrome OS Security