2012-05-26 13:08:27

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 0/5] Some pstore/ramoops fixes

Hi Greg,

In the light of Linus' response, and I said this to Colin already, I'll
just zap a prz at boot time for pstore/console interface, which means
that nowadays there shouldn't be any objections to this bunch of fixes.

These are valid fixes for v3.5, they restore old pstore's behavior
nuances, which I changed accidentaly.

Except for the last patch, which is just a fix I happened to make when
I got bored of the warning. :-) Not a regression fix, though.

Thanks,

---
fs/pstore/inode.c | 2 +-
fs/pstore/ram.c | 3 +++
fs/pstore/ram_core.c | 27 +++++++++++++++++----------
include/linux/pstore_ram.h | 2 ++
4 files changed, 23 insertions(+), 11 deletions(-)

--
Anton Vorontsov
Email: [email protected]


2012-05-26 13:10:20

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 1/5] pstore/ram: Should update old dmesg buffer before reading

Without the update, we'll only see the new dmesg buffer after the
reboot, but previously we could see it right away. Making an oops
visible in pstore filesystem before reboot is a somewhat dubious
feature, but removing it wasn't an intentional change, so let's
restore it.

For this we have to make persistent_ram_save_old() safe for calling
multiple times, and also extern it.

Signed-off-by: Anton Vorontsov <[email protected]>
Acked-by: Kees Cook <[email protected]>
---
fs/pstore/ram.c | 2 ++
fs/pstore/ram_core.c | 15 ++++++++-------
include/linux/pstore_ram.h | 1 +
3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 9123cce..16ff733 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -106,6 +106,8 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
time->tv_sec = 0;
time->tv_nsec = 0;

+ /* Update old/shadowed buffer. */
+ persistent_ram_save_old(prz);
size = persistent_ram_old_size(prz);
*buf = kmalloc(size, GFP_KERNEL);
if (*buf == NULL)
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 31f8d18..235513c 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -250,23 +250,24 @@ static void notrace persistent_ram_update(struct persistent_ram_zone *prz,
persistent_ram_update_ecc(prz, start, count);
}

-static void __init
-persistent_ram_save_old(struct persistent_ram_zone *prz)
+void persistent_ram_save_old(struct persistent_ram_zone *prz)
{
struct persistent_ram_buffer *buffer = prz->buffer;
size_t size = buffer_size(prz);
size_t start = buffer_start(prz);
- char *dest;

- persistent_ram_ecc_old(prz);
+ if (!size)
+ return;

- dest = kmalloc(size, GFP_KERNEL);
- if (dest == NULL) {
+ if (!prz->old_log) {
+ persistent_ram_ecc_old(prz);
+ prz->old_log = kmalloc(size, GFP_KERNEL);
+ }
+ if (!prz->old_log) {
pr_err("persistent_ram: failed to allocate buffer\n");
return;
}

- prz->old_log = dest;
prz->old_log_size = size;
memcpy(prz->old_log, &buffer->data[start], size - start);
memcpy(prz->old_log + size - start, &buffer->data[0], start);
diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
index 7ed7fd4..4491e8f 100644
--- a/include/linux/pstore_ram.h
+++ b/include/linux/pstore_ram.h
@@ -75,6 +75,7 @@ struct persistent_ram_zone *persistent_ram_init_ringbuffer(struct device *dev,
int persistent_ram_write(struct persistent_ram_zone *prz, const void *s,
unsigned int count);

+void persistent_ram_save_old(struct persistent_ram_zone *prz);
size_t persistent_ram_old_size(struct persistent_ram_zone *prz);
void *persistent_ram_old(struct persistent_ram_zone *prz);
void persistent_ram_free_old(struct persistent_ram_zone *prz);
--
1.7.9.2

2012-05-26 13:10:27

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 2/5] pstore/ram_core: Do not reset restored zone's position and size

Otherwise, the files will survive just one reboot, and on a subsequent
boot they will disappear.

Signed-off-by: Anton Vorontsov <[email protected]>
---
fs/pstore/ram_core.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 235513c..f6650d1 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -406,6 +406,7 @@ static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool
" size %zu, start %zu\n",
buffer_size(prz), buffer_start(prz));
persistent_ram_save_old(prz);
+ return 0;
}
} else {
pr_info("persistent_ram: no valid data in buffer"
--
1.7.9.2

2012-05-26 13:10:33

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 4/5] pstore/ram: Should zap persistent zone on unlink

Otherwise, unlinked file will reappear on the next boot.

Reported-by: Kees Cook <[email protected]>
Signed-off-by: Anton Vorontsov <[email protected]>
Acked-by: Kees Cook <[email protected]>
---
fs/pstore/ram.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 16ff733..453030f 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -186,6 +186,7 @@ static int ramoops_pstore_erase(enum pstore_type_id type, u64 id,
return -EINVAL;

persistent_ram_free_old(cxt->przs[id]);
+ persistent_ram_zap(cxt->przs[id]);

return 0;
}
--
1.7.9.2

2012-05-26 13:10:38

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 5/5] pstore/inode: Make pstore_fill_super() static

There's no reason to extern it. The patch fixes the annoying sparse
warning:

CHECK fs/pstore/inode.c
fs/pstore/inode.c:264:5: warning: symbol 'pstore_fill_super' was not
declared. Should it be static?

Signed-off-by: Anton Vorontsov <[email protected]>
Acked-by: Kees Cook <[email protected]>
---
fs/pstore/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 1950788..49b40ea 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -258,7 +258,7 @@ fail:
return rc;
}

-int pstore_fill_super(struct super_block *sb, void *data, int silent)
+static int pstore_fill_super(struct super_block *sb, void *data, int silent)
{
struct inode *inode;

--
1.7.9.2

2012-05-26 13:10:59

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 3/5] pstore/ram_core: Factor persistent_ram_zap() out of post_init()

A handy function that we will use outside of ram_core soon. But
so far just factor it out and start using it in post_init().

Signed-off-by: Anton Vorontsov <[email protected]>
Acked-by: Kees Cook <[email protected]>
---
fs/pstore/ram_core.c | 11 ++++++++---
include/linux/pstore_ram.h | 1 +
2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index f6650d1..c5fbdbb 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -320,6 +320,13 @@ void persistent_ram_free_old(struct persistent_ram_zone *prz)
prz->old_log_size = 0;
}

+void persistent_ram_zap(struct persistent_ram_zone *prz)
+{
+ atomic_set(&prz->buffer->start, 0);
+ atomic_set(&prz->buffer->size, 0);
+ persistent_ram_update_header_ecc(prz);
+}
+
static void *persistent_ram_vmap(phys_addr_t start, size_t size)
{
struct page **pages;
@@ -414,8 +421,7 @@ static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool
}

prz->buffer->sig = PERSISTENT_RAM_SIG;
- atomic_set(&prz->buffer->start, 0);
- atomic_set(&prz->buffer->size, 0);
+ persistent_ram_zap(prz);

return 0;
}
@@ -450,7 +456,6 @@ struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
goto err;

persistent_ram_post_init(prz, ecc);
- persistent_ram_update_header_ecc(prz);

return prz;
err:
diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
index 4491e8f..3b823d4 100644
--- a/include/linux/pstore_ram.h
+++ b/include/linux/pstore_ram.h
@@ -69,6 +69,7 @@ struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
size_t size,
bool ecc);
void persistent_ram_free(struct persistent_ram_zone *prz);
+void persistent_ram_zap(struct persistent_ram_zone *prz);
struct persistent_ram_zone *persistent_ram_init_ringbuffer(struct device *dev,
bool ecc);

--
1.7.9.2

2012-06-12 01:02:35

by Anton Vorontsov

[permalink] [raw]
Subject: Re: [PATCH 0/5] Some pstore/ramoops fixes

On Sat, May 26, 2012 at 06:06:50AM -0700, Anton Vorontsov wrote:
> In the light of Linus' response, and I said this to Colin already, I'll
> just zap a prz at boot time for pstore/console interface, which means
> that nowadays there shouldn't be any objections to this bunch of fixes.
>
> These are valid fixes for v3.5, they restore old pstore's behavior
> nuances, which I changed accidentaly.
>
> Except for the last patch, which is just a fix I happened to make when
> I got bored of the warning. :-) Not a regression fix, though.
>
> Thanks,
>
> ---
> fs/pstore/inode.c | 2 +-
> fs/pstore/ram.c | 3 +++
> fs/pstore/ram_core.c | 27 +++++++++++++++++----------
> include/linux/pstore_ram.h | 2 ++
> 4 files changed, 23 insertions(+), 11 deletions(-)

Hi Greg,

Have you had a chance to look into these? Plus into
"[PATCH v5 0/11] Merge ram_console into pstore" series, I believe
there were no objections as well.

Thanks!

p.s. I must confess I have a huge pile of battery-related patches in
my inbox that I have to review/apply, and folks start to send similar
pings as I send to you now. :-) I should probably start working on
my backlog to improve my karma, hehe.

--
Anton Vorontsov
Email: [email protected]

2012-06-12 01:44:26

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 0/5] Some pstore/ramoops fixes

On Mon, Jun 11, 2012 at 06:00:51PM -0700, Anton Vorontsov wrote:
> On Sat, May 26, 2012 at 06:06:50AM -0700, Anton Vorontsov wrote:
> > In the light of Linus' response, and I said this to Colin already, I'll
> > just zap a prz at boot time for pstore/console interface, which means
> > that nowadays there shouldn't be any objections to this bunch of fixes.
> >
> > These are valid fixes for v3.5, they restore old pstore's behavior
> > nuances, which I changed accidentaly.
> >
> > Except for the last patch, which is just a fix I happened to make when
> > I got bored of the warning. :-) Not a regression fix, though.
> >
> > Thanks,
> >
> > ---
> > fs/pstore/inode.c | 2 +-
> > fs/pstore/ram.c | 3 +++
> > fs/pstore/ram_core.c | 27 +++++++++++++++++----------
> > include/linux/pstore_ram.h | 2 ++
> > 4 files changed, 23 insertions(+), 11 deletions(-)
>
> Hi Greg,
>
> Have you had a chance to look into these? Plus into
> "[PATCH v5 0/11] Merge ram_console into pstore" series, I believe
> there were no objections as well.

I'm getting there, give me this week to get through them all...

2012-06-13 23:53:36

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 0/5] Some pstore/ramoops fixes

On Sat, May 26, 2012 at 06:06:50AM -0700, Anton Vorontsov wrote:
> Hi Greg,
>
> In the light of Linus' response, and I said this to Colin already, I'll
> just zap a prz at boot time for pstore/console interface, which means
> that nowadays there shouldn't be any objections to this bunch of fixes.
>
> These are valid fixes for v3.5, they restore old pstore's behavior
> nuances, which I changed accidentaly.
>
> Except for the last patch, which is just a fix I happened to make when
> I got bored of the warning. :-) Not a regression fix, though.

All now applied, thanks.

greg k-h