[PATCH v4] pstore: Avoid duplicate call of persistent_ram_zap()

[PATCH v4] pstore: Avoid duplicate call of persistent_ram_zap()

[Peng Wang]

When initialing prz with invalid data in buffer(no PERSISTENT_RAM_SIG),
function call path is like this:

ramoops_init_prz ->
|
|-> persistent_ram_new -> persistent_ram_post_init -> persistent_ram_zap
|
|-> persistent_ram_zap

As we can see, persistent_ram_zap() is called twice.
We can avoid this by adding an option to persistent_ram_new(), and
only call persistent_ram_zap() when it is needed.

Signed-off-by: Peng Wang <wangpeng15@xiaomi.com>
---
 fs/pstore/ram.c            | 4 +---
 fs/pstore/ram_core.c       | 5 +++--
 include/linux/pstore_ram.h | 1 +
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index ffcff6516e89..b51901f97dc2 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -640,7 +640,7 @@ static int ramoops_init_prz(const char *name,
 
 	label = kasprintf(GFP_KERNEL, "ramoops:%s", name);
 	*prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info,
-				  cxt->memtype, 0, label);
+				  cxt->memtype, PRZ_FLAG_ZAP_OLD, label);
 	if (IS_ERR(*prz)) {
 		int err = PTR_ERR(*prz);
 
@@ -649,8 +649,6 @@ static int ramoops_init_prz(const char *name,
 		return err;
 	}
 
-	persistent_ram_zap(*prz);
-
 	*paddr += sz;
 
 	return 0;
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 12e21f789194..2ededd1ea1c2 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -505,15 +505,16 @@ static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig,
 			pr_debug("found existing buffer, size %zu, start %zu\n",
 				 buffer_size(prz), buffer_start(prz));
 			persistent_ram_save_old(prz);
-			return 0;
+			if (!(prz->flags & PRZ_FLAG_ZAP_OLD))
+				return 0;
 		}
 	} else {
 		pr_debug("no valid data in buffer (sig = 0x%08x)\n",
 			 prz->buffer->sig);
+		prz->buffer->sig = sig;
 	}
 
 	/* Rewind missing or invalid memory area. */
-	prz->buffer->sig = sig;
 	persistent_ram_zap(prz);
 
 	return 0;
diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
index 602d64725222..6e94980357d2 100644
--- a/include/linux/pstore_ram.h
+++ b/include/linux/pstore_ram.h
@@ -30,6 +30,7 @@
  * PRZ_FLAG_NO_LOCK is used. For all other cases, locking is required.
  */
 #define PRZ_FLAG_NO_LOCK	BIT(0)
+#define PRZ_FLAG_ZAP_OLD	BIT(1)
 
 struct persistent_ram_buffer;
 struct rs_control;
-- 
2.19.1

Re: [PATCH v4] pstore: Avoid duplicate call of persistent_ram_zap()

[Kees Cook]

On Tue, Oct 30, 2018 at 12:52 AM, Peng Wang <wangpeng15@xiaomi.com> wrote:
> When initialing prz with invalid data in buffer(no PERSISTENT_RAM_SIG),
> function call path is like this:
>
> ramoops_init_prz ->
> |
> |-> persistent_ram_new -> persistent_ram_post_init -> persistent_ram_zap
> |
> |-> persistent_ram_zap
>
> As we can see, persistent_ram_zap() is called twice.
> We can avoid this by adding an option to persistent_ram_new(), and
> only call persistent_ram_zap() when it is needed.
>
> Signed-off-by: Peng Wang <wangpeng15@xiaomi.com>

Thanks! I've applied this to my devel branch. I'll have it in
linux-next once -rc2 lands.

-Kees

> ---
>  fs/pstore/ram.c            | 4 +---
>  fs/pstore/ram_core.c       | 5 +++--
>  include/linux/pstore_ram.h | 1 +
>  3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index ffcff6516e89..b51901f97dc2 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -640,7 +640,7 @@ static int ramoops_init_prz(const char *name,
>
>         label = kasprintf(GFP_KERNEL, "ramoops:%s", name);
>         *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info,
> -                                 cxt->memtype, 0, label);
> +                                 cxt->memtype, PRZ_FLAG_ZAP_OLD, label);
>         if (IS_ERR(*prz)) {
>                 int err = PTR_ERR(*prz);
>
> @@ -649,8 +649,6 @@ static int ramoops_init_prz(const char *name,
>                 return err;
>         }
>
> -       persistent_ram_zap(*prz);
> -
>         *paddr += sz;
>
>         return 0;
> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
> index 12e21f789194..2ededd1ea1c2 100644
> --- a/fs/pstore/ram_core.c
> +++ b/fs/pstore/ram_core.c
> @@ -505,15 +505,16 @@ static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig,
>                         pr_debug("found existing buffer, size %zu, start %zu\n",
>                                  buffer_size(prz), buffer_start(prz));
>                         persistent_ram_save_old(prz);
> -                       return 0;
> +                       if (!(prz->flags & PRZ_FLAG_ZAP_OLD))
> +                               return 0;
>                 }
>         } else {
>                 pr_debug("no valid data in buffer (sig = 0x%08x)\n",
>                          prz->buffer->sig);
> +               prz->buffer->sig = sig;
>         }
>
>         /* Rewind missing or invalid memory area. */
> -       prz->buffer->sig = sig;
>         persistent_ram_zap(prz);
>
>         return 0;
> diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
> index 602d64725222..6e94980357d2 100644
> --- a/include/linux/pstore_ram.h
> +++ b/include/linux/pstore_ram.h
> @@ -30,6 +30,7 @@
>   * PRZ_FLAG_NO_LOCK is used. For all other cases, locking is required.
>   */
>  #define PRZ_FLAG_NO_LOCK       BIT(0)
> +#define PRZ_FLAG_ZAP_OLD       BIT(1)
>
>  struct persistent_ram_buffer;
>  struct rs_control;
> --
> 2.19.1
>



-- 
Kees Cook

Re: [PATCH v4] pstore: Avoid duplicate call of persistent_ram_zap()

[Joel Fernandes]

On Tue, Oct 30, 2018 at 03:52:34PM +0800, Peng Wang wrote:
> When initialing prz with invalid data in buffer(no PERSISTENT_RAM_SIG),
> function call path is like this:
> 
> ramoops_init_prz ->
> |
> |-> persistent_ram_new -> persistent_ram_post_init -> persistent_ram_zap
> |
> |-> persistent_ram_zap
> 
> As we can see, persistent_ram_zap() is called twice.
> We can avoid this by adding an option to persistent_ram_new(), and
> only call persistent_ram_zap() when it is needed.
> 
> Signed-off-by: Peng Wang <wangpeng15@xiaomi.com>
> ---
>  fs/pstore/ram.c            | 4 +---
>  fs/pstore/ram_core.c       | 5 +++--
>  include/linux/pstore_ram.h | 1 +
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index ffcff6516e89..b51901f97dc2 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -640,7 +640,7 @@ static int ramoops_init_prz(const char *name,
>  
>  	label = kasprintf(GFP_KERNEL, "ramoops:%s", name);
>  	*prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info,
> -				  cxt->memtype, 0, label);
> +				  cxt->memtype, PRZ_FLAG_ZAP_OLD, label);
>  	if (IS_ERR(*prz)) {
>  		int err = PTR_ERR(*prz);

Looks good to me except the minor comment below:

>  
> @@ -649,8 +649,6 @@ static int ramoops_init_prz(const char *name,
>  		return err;
>  	}
>  
> -	persistent_ram_zap(*prz);
> -
>  	*paddr += sz;
>  
>  	return 0;
> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
> index 12e21f789194..2ededd1ea1c2 100644
> --- a/fs/pstore/ram_core.c
> +++ b/fs/pstore/ram_core.c
> @@ -505,15 +505,16 @@ static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig,
>  			pr_debug("found existing buffer, size %zu, start %zu\n",
>  				 buffer_size(prz), buffer_start(prz));
>  			persistent_ram_save_old(prz);
> -			return 0;
> +			if (!(prz->flags & PRZ_FLAG_ZAP_OLD))
> +				return 0;

This could be written differently.

We could just do:

if (prz->flags & PRZ_FLAG_ZAP_OLD)
	persistent_ram_zap(prz);

And remove the zap from below below.

Since Kees already took this patch, I can just patch this in my series if
Kees and you are Ok with this suggestion.

Sorry for the delay in my RFC series, I just got back from paternity leave
and I'm catching up with email.

thanks,

- Joel

>  		}
>  	} else {
>  		pr_debug("no valid data in buffer (sig = 0x%08x)\n",
>  			 prz->buffer->sig);
> +		prz->buffer->sig = sig;
>  	}
>  
>  	/* Rewind missing or invalid memory area. */
> -	prz->buffer->sig = sig;
>  	persistent_ram_zap(prz);
>  
>  	return 0;

Re: [PATCH v4] pstore: Avoid duplicate call of persistent_ram_zap()

[Kees Cook]

On Tue, Oct 30, 2018 at 2:38 PM, Joel Fernandes <joel@joelfernandes.org> wrote:
> On Tue, Oct 30, 2018 at 03:52:34PM +0800, Peng Wang wrote:
>> When initialing prz with invalid data in buffer(no PERSISTENT_RAM_SIG),
>> function call path is like this:
>>
>> ramoops_init_prz ->
>> |
>> |-> persistent_ram_new -> persistent_ram_post_init -> persistent_ram_zap
>> |
>> |-> persistent_ram_zap
>>
>> As we can see, persistent_ram_zap() is called twice.
>> We can avoid this by adding an option to persistent_ram_new(), and
>> only call persistent_ram_zap() when it is needed.
>>
>> Signed-off-by: Peng Wang <wangpeng15@xiaomi.com>
>> ---
>>  fs/pstore/ram.c            | 4 +---
>>  fs/pstore/ram_core.c       | 5 +++--
>>  include/linux/pstore_ram.h | 1 +
>>  3 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
>> index ffcff6516e89..b51901f97dc2 100644
>> --- a/fs/pstore/ram.c
>> +++ b/fs/pstore/ram.c
>> @@ -640,7 +640,7 @@ static int ramoops_init_prz(const char *name,
>>
>>       label = kasprintf(GFP_KERNEL, "ramoops:%s", name);
>>       *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info,
>> -                               cxt->memtype, 0, label);
>> +                               cxt->memtype, PRZ_FLAG_ZAP_OLD, label);
>>       if (IS_ERR(*prz)) {
>>               int err = PTR_ERR(*prz);
>
> Looks good to me except the minor comment below:
>
>>
>> @@ -649,8 +649,6 @@ static int ramoops_init_prz(const char *name,
>>               return err;
>>       }
>>
>> -     persistent_ram_zap(*prz);
>> -
>>       *paddr += sz;
>>
>>       return 0;
>> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
>> index 12e21f789194..2ededd1ea1c2 100644
>> --- a/fs/pstore/ram_core.c
>> +++ b/fs/pstore/ram_core.c
>> @@ -505,15 +505,16 @@ static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig,
>>                       pr_debug("found existing buffer, size %zu, start %zu\n",
>>                                buffer_size(prz), buffer_start(prz));
>>                       persistent_ram_save_old(prz);
>> -                     return 0;
>> +                     if (!(prz->flags & PRZ_FLAG_ZAP_OLD))
>> +                             return 0;
>
> This could be written differently.
>
> We could just do:
>
> if (prz->flags & PRZ_FLAG_ZAP_OLD)
>         persistent_ram_zap(prz);
>
> And remove the zap from below below.

I actually rearranged things a little to avoid additional round-trips
on the mailing list. :)

> Since Kees already took this patch, I can just patch this in my series if
> Kees and you are Ok with this suggestion.

I've put it up here:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=pstore/devel&id=ac564e023248e3f4d87917b91d12376ddfca5000

> Sorry for the delay in my RFC series, I just got back from paternity leave
> and I'm catching up with email.

No worries! It's many weeks until the next merge window. :)

-- 
Kees Cook

Re: [PATCH v4] pstore: Avoid duplicate call of persistent_ram_zap()

[Joel Fernandes]

On Tue, Oct 30, 2018 at 02:52:43PM -0700, Kees Cook wrote:
> On Tue, Oct 30, 2018 at 2:38 PM, Joel Fernandes <joel@joelfernandes.org> wrote:
> > On Tue, Oct 30, 2018 at 03:52:34PM +0800, Peng Wang wrote:
> >> When initialing prz with invalid data in buffer(no PERSISTENT_RAM_SIG),
> >> function call path is like this:
> >>
> >> ramoops_init_prz ->
> >> |
> >> |-> persistent_ram_new -> persistent_ram_post_init -> persistent_ram_zap
> >> |
> >> |-> persistent_ram_zap
> >>
> >> As we can see, persistent_ram_zap() is called twice.
> >> We can avoid this by adding an option to persistent_ram_new(), and
> >> only call persistent_ram_zap() when it is needed.
> >>
> >> Signed-off-by: Peng Wang <wangpeng15@xiaomi.com>
> >> ---
> >>  fs/pstore/ram.c            | 4 +---
> >>  fs/pstore/ram_core.c       | 5 +++--
> >>  include/linux/pstore_ram.h | 1 +
> >>  3 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> >> index ffcff6516e89..b51901f97dc2 100644
> >> --- a/fs/pstore/ram.c
> >> +++ b/fs/pstore/ram.c
> >> @@ -640,7 +640,7 @@ static int ramoops_init_prz(const char *name,
> >>
> >>       label = kasprintf(GFP_KERNEL, "ramoops:%s", name);
> >>       *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info,
> >> -                               cxt->memtype, 0, label);
> >> +                               cxt->memtype, PRZ_FLAG_ZAP_OLD, label);
> >>       if (IS_ERR(*prz)) {
> >>               int err = PTR_ERR(*prz);
> >
> > Looks good to me except the minor comment below:
> >
> >>
> >> @@ -649,8 +649,6 @@ static int ramoops_init_prz(const char *name,
> >>               return err;
> >>       }
> >>
> >> -     persistent_ram_zap(*prz);
> >> -
> >>       *paddr += sz;
> >>
> >>       return 0;
> >> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
> >> index 12e21f789194..2ededd1ea1c2 100644
> >> --- a/fs/pstore/ram_core.c
> >> +++ b/fs/pstore/ram_core.c
> >> @@ -505,15 +505,16 @@ static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig,
> >>                       pr_debug("found existing buffer, size %zu, start %zu\n",
> >>                                buffer_size(prz), buffer_start(prz));
> >>                       persistent_ram_save_old(prz);
> >> -                     return 0;
> >> +                     if (!(prz->flags & PRZ_FLAG_ZAP_OLD))
> >> +                             return 0;
> >
> > This could be written differently.
> >
> > We could just do:
> >
> > if (prz->flags & PRZ_FLAG_ZAP_OLD)
> >         persistent_ram_zap(prz);
> >
> > And remove the zap from below below.
> 
> I actually rearranged things a little to avoid additional round-trips
> on the mailing list. :)
> 
> > Since Kees already took this patch, I can just patch this in my series if
> > Kees and you are Ok with this suggestion.
> 
> I've put it up here:
> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=pstore/devel&id=ac564e023248e3f4d87917b91d12376ddfca5000

Cool, it LGTM :)

- Joel

Re: [PATCH v4] pstore: Avoid duplicate call of persistent_ram_zap()

[Peng15 Wang 王鹏]

>From: Joel Fernandes <joel@joelfernandes.org>
>Sent: Wednesday, October 31, 2018 6:16
>To: Kees Cook
>Cc: Peng15 Wang 王鹏; Anton Vorontsov; Colin Cross; Tony Luck; LKML; vipwangerxiao@gmail.com
>Subject: Re: [PATCH v4] pstore: Avoid duplicate call of persistent_ram_zap()
>
>On Tue, Oct 30, 2018 at 02:52:43PM -0700, Kees Cook wrote:
>> On Tue, Oct 30, 2018 at 2:38 PM, Joel Fernandes <joel@joelfernandes.org> wrote:
>> > On Tue, Oct 30, 2018 at 03:52:34PM +0800, Peng Wang wrote:
>> >> When initialing prz with invalid data in buffer(no PERSISTENT_RAM_SIG),
>> >> function call path is like this:
>> >>
>> >> ramoops_init_prz ->
>> >> |
>> >> |-> persistent_ram_new -> persistent_ram_post_init -> persistent_ram_zap
>> >> |
>> >> |-> persistent_ram_zap
>> >>
>> >> As we can see, persistent_ram_zap() is called twice.
>> >> We can avoid this by adding an option to persistent_ram_new(), and
>> >> only call persistent_ram_zap() when it is needed.
>> >>
>> >> Signed-off-by: Peng Wang <wangpeng15@xiaomi.com>
>> >> ---
>> >>  fs/pstore/ram.c            | 4 +---
>> >>  fs/pstore/ram_core.c       | 5 +++--
>> >>  include/linux/pstore_ram.h | 1 +
>> >>  3 files changed, 5 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
>> >> index ffcff6516e89..b51901f97dc2 100644
>> >> --- a/fs/pstore/ram.c
>> >> +++ b/fs/pstore/ram.c
>> >> @@ -640,7 +640,7 @@ static int ramoops_init_prz(const char *name,
>> >>
>> >>       label = kasprintf(GFP_KERNEL, "ramoops:%s", name);
>> >>       *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info,
>> >> -                               cxt->memtype, 0, label);
>> >> +                               cxt->memtype, PRZ_FLAG_ZAP_OLD, label);
>> >>       if (IS_ERR(*prz)) {
>> >>               int err = PTR_ERR(*prz);
>> >
>> > Looks good to me except the minor comment below:
>> >
>> >>
>> >> @@ -649,8 +649,6 @@ static int ramoops_init_prz(const char *name,
>> >>               return err;
>> >>       }
>> >>
>> >> -     persistent_ram_zap(*prz);
>> >> -
>> >>       *paddr += sz;
>> >>
>> >>       return 0;
>> >> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
>> >> index 12e21f789194..2ededd1ea1c2 100644
>> >> --- a/fs/pstore/ram_core.c
>> >> +++ b/fs/pstore/ram_core.c
>> >> @@ -505,15 +505,16 @@ static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig,
>> >>                       pr_debug("found existing buffer, size %zu, start %zu\n",
>> >>                                buffer_size(prz), buffer_start(prz));
>> >>                       persistent_ram_save_old(prz);
>> >> -                     return 0;
>> >> +                     if (!(prz->flags & PRZ_FLAG_ZAP_OLD))
>> >> +                             return 0;
>> >
>> > This could be written differently.
>> >
>> > We could just do:
>> >
>> > if (prz->flags & PRZ_FLAG_ZAP_OLD)
>> >         persistent_ram_zap(prz);
>> >
>> > And remove the zap from below below.
>>
>> I actually rearranged things a little to avoid additional round-trips
>> on the mailing list. :)
>>
>> > Since Kees already took this patch, I can just patch this in my series if
>> > Kees and you are Ok with this suggestion.
>>
>> I've put it up here:
>> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=pstore/devel&id=ac564e023248e3f4d87917b91d12376ddfca5000
>
>Cool, it LGTM :)
>
>- Joel
>

Thank you all for these warm help.

This is my first time to submit a patch to community. Feel great!

Re: [PATCH v4] pstore: Avoid duplicate call of persistent_ram_zap()

[Joel Fernandes]

On Tue, Oct 30, 2018 at 8:57 PM, Peng15 Wang 王鹏 <wangpeng15@xiaomi.com> wrote:
>
>>From: Joel Fernandes <joel@joelfernandes.org>
>>Sent: Wednesday, October 31, 2018 6:16
>>To: Kees Cook
>>Cc: Peng15 Wang 王鹏; Anton Vorontsov; Colin Cross; Tony Luck; LKML; vipwangerxiao@gmail.com
>>Subject: Re: [PATCH v4] pstore: Avoid duplicate call of persistent_ram_zap()
>>
>>On Tue, Oct 30, 2018 at 02:52:43PM -0700, Kees Cook wrote:
>>> On Tue, Oct 30, 2018 at 2:38 PM, Joel Fernandes <joel@joelfernandes.org> wrote:
>>> > On Tue, Oct 30, 2018 at 03:52:34PM +0800, Peng Wang wrote:
>>> >> When initialing prz with invalid data in buffer(no PERSISTENT_RAM_SIG),
>>> >> function call path is like this:
>>> >>
>>> >> ramoops_init_prz ->
>>> >> |
>>> >> |-> persistent_ram_new -> persistent_ram_post_init -> persistent_ram_zap
>>> >> |
>>> >> |-> persistent_ram_zap
>>> >>
>>> >> As we can see, persistent_ram_zap() is called twice.
>>> >> We can avoid this by adding an option to persistent_ram_new(), and
>>> >> only call persistent_ram_zap() when it is needed.
>>> >>
>>> >> Signed-off-by: Peng Wang <wangpeng15@xiaomi.com>
>>> >> ---
>>> >>  fs/pstore/ram.c            | 4 +---
>>> >>  fs/pstore/ram_core.c       | 5 +++--
>>> >>  include/linux/pstore_ram.h | 1 +
>>> >>  3 files changed, 5 insertions(+), 5 deletions(-)
>>> >>
>>> >> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
>>> >> index ffcff6516e89..b51901f97dc2 100644
>>> >> --- a/fs/pstore/ram.c
>>> >> +++ b/fs/pstore/ram.c
>>> >> @@ -640,7 +640,7 @@ static int ramoops_init_prz(const char *name,
>>> >>
>>> >>       label = kasprintf(GFP_KERNEL, "ramoops:%s", name);
>>> >>       *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info,
>>> >> -                               cxt->memtype, 0, label);
>>> >> +                               cxt->memtype, PRZ_FLAG_ZAP_OLD, label);
>>> >>       if (IS_ERR(*prz)) {
>>> >>               int err = PTR_ERR(*prz);
>>> >
>>> > Looks good to me except the minor comment below:
>>> >
>>> >>
>>> >> @@ -649,8 +649,6 @@ static int ramoops_init_prz(const char *name,
>>> >>               return err;
>>> >>       }
>>> >>
>>> >> -     persistent_ram_zap(*prz);
>>> >> -
>>> >>       *paddr += sz;
>>> >>
>>> >>       return 0;
>>> >> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
>>> >> index 12e21f789194..2ededd1ea1c2 100644
>>> >> --- a/fs/pstore/ram_core.c
>>> >> +++ b/fs/pstore/ram_core.c
>>> >> @@ -505,15 +505,16 @@ static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig,
>>> >>                       pr_debug("found existing buffer, size %zu, start %zu\n",
>>> >>                                buffer_size(prz), buffer_start(prz));
>>> >>                       persistent_ram_save_old(prz);
>>> >> -                     return 0;
>>> >> +                     if (!(prz->flags & PRZ_FLAG_ZAP_OLD))
>>> >> +                             return 0;
>>> >
>>> > This could be written differently.
>>> >
>>> > We could just do:
>>> >
>>> > if (prz->flags & PRZ_FLAG_ZAP_OLD)
>>> >         persistent_ram_zap(prz);
>>> >
>>> > And remove the zap from below below.
>>>
>>> I actually rearranged things a little to avoid additional round-trips
>>> on the mailing list. :)
>>>
>>> > Since Kees already took this patch, I can just patch this in my series if
>>> > Kees and you are Ok with this suggestion.
>>>
>>> I've put it up here:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=pstore/devel&id=ac564e023248e3f4d87917b91d12376ddfca5000
>>
>>Cool, it LGTM :)
>>
>>- Joel
>>
>
> Thank you all for these warm help.
>
> This is my first time to submit a patch to community. Feel great!

Congrats and welcome to the mother ship ;-)

 - Joel