Re: [U-Boot] [PATCH] fw_setenv: avoid writing environment when nothing has changed

2018-09-25 Thread Rasmus Villemoes
On 2018-09-24 09:42, Alex Kiernan wrote:
> On Wed, Sep 5, 2018 at 8:23 PM Rasmus Villemoes
>  wrote:
>>
>> In the case where one deletes an already-non-existing variable, or sets
>> a variable to the value it already has, there is no point in writing the
>> environment back, thus reducing wear on the underlying storage
>> device.
>>
>> Signed-off-by: Rasmus Villemoes 
> 
> If you were running with a redundant env, and you were trying to sync
> both copies, wouldn't you want a non-changing write to happen?

Hm, probably, yes. But I'd still like to avoid it if they are already in
sync, so perhaps I could just add

  if (memcmp(environment.data, redundant->data, ENV_SIZE))
environment.dirty = 1;

after reading in the second copy [using that environment.data at that
point is still ((struct env_image_redundant *)addr0)->data ].

Rasmus
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] fw_setenv: avoid writing environment when nothing has changed

2018-09-24 Thread Joakim Tjernlund
On Mon, 2018-09-24 at 08:42 +0100, Alex Kiernan wrote:
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open attachments unless you recognize the sender and know the 
> content is safe.
> 
> 
> On Wed, Sep 5, 2018 at 8:23 PM Rasmus Villemoes
>  wrote:
> > 
> > In the case where one deletes an already-non-existing variable, or sets
> > a variable to the value it already has, there is no point in writing the
> > environment back, thus reducing wear on the underlying storage
> > device.
> > 
> > Signed-off-by: Rasmus Villemoes 
> 
> If you were running with a redundant env, and you were trying to sync
> both copies, wouldn't you want a non-changing write to happen?

That is a valid point, I have sometimes used this property to make
sure I have a valid env. in both copies.

 Jocke
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] fw_setenv: avoid writing environment when nothing has changed

2018-09-24 Thread Alex Kiernan
On Wed, Sep 5, 2018 at 8:23 PM Rasmus Villemoes
 wrote:
>
> In the case where one deletes an already-non-existing variable, or sets
> a variable to the value it already has, there is no point in writing the
> environment back, thus reducing wear on the underlying storage
> device.
>
> Signed-off-by: Rasmus Villemoes 

If you were running with a redundant env, and you were trying to sync
both copies, wouldn't you want a non-changing write to happen?

> ---
>  tools/env/fw_env.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index a5d75958e1..87aaa15198 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -110,6 +110,7 @@ struct environment {
> unsigned char *flags;
> char *data;
> enum flag_scheme flag_scheme;
> +   int dirty;
>  };
>
>  static struct environment environment = {
> @@ -508,6 +509,9 @@ int fw_env_flush(struct env_opts *opts)
> if (!opts)
> opts = _opts;
>
> +   if (!environment.dirty)
> +   return 0;
> +
> /*
>  * Update CRC
>  */
> @@ -553,7 +557,8 @@ int fw_env_write(char *name, char *value)
>
> deleting = (oldval && !(value && strlen(value)));
> creating = (!oldval && (value && strlen(value)));
> -   overwriting = (oldval && (value && strlen(value)));
> +   overwriting = (oldval && (value && strlen(value) &&
> + strcmp(oldval, value)));
>
> /* check for permission */
> if (deleting) {
> @@ -593,6 +598,7 @@ int fw_env_write(char *name, char *value)
> /* Nothing to do */
> return 0;
>
> +   environment.dirty = 1;
> if (deleting || overwriting) {
> if (*++nxt == '\0') {
> *env = '\0';
> @@ -1441,6 +1447,7 @@ int fw_env_open(struct env_opts *opts)
> "Warning: Bad CRC, using default 
> environment\n");
> memcpy(environment.data, default_environment,
>sizeof(default_environment));
> +   environment.dirty = 1;
> }
> } else {
> flag0 = *environment.flags;
> @@ -1503,6 +1510,7 @@ int fw_env_open(struct env_opts *opts)
> "Warning: Bad CRC, using default 
> environment\n");
> memcpy(environment.data, default_environment,
>sizeof(default_environment));
> +   environment.dirty = 1;
> dev_current = 0;
> } else {
> switch (environment.flag_scheme) {
> --
> 2.16.4
>
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot



-- 
Alex Kiernan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] fw_setenv: avoid writing environment when nothing has changed

2018-09-24 Thread Michael Nazzareno Trimarchi
Hi



On Mon., 24 Sep. 2018, 8:19 am Rasmus Villemoes, 
wrote:

> On 2018-09-05 21:22, Rasmus Villemoes wrote:
> > In the case where one deletes an already-non-existing variable, or sets
> > a variable to the value it already has, there is no point in writing the
> > environment back, thus reducing wear on the underlying storage
> > device.
>
> ping
>

Sorry I will give a try and make sense in my use cases

Michael

> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] fw_setenv: avoid writing environment when nothing has changed

2018-09-24 Thread Rasmus Villemoes
On 2018-09-05 21:22, Rasmus Villemoes wrote:
> In the case where one deletes an already-non-existing variable, or sets
> a variable to the value it already has, there is no point in writing the
> environment back, thus reducing wear on the underlying storage
> device.

ping
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH] fw_setenv: avoid writing environment when nothing has changed

2018-09-05 Thread Rasmus Villemoes
In the case where one deletes an already-non-existing variable, or sets
a variable to the value it already has, there is no point in writing the
environment back, thus reducing wear on the underlying storage
device.

Signed-off-by: Rasmus Villemoes 
---
 tools/env/fw_env.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index a5d75958e1..87aaa15198 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -110,6 +110,7 @@ struct environment {
unsigned char *flags;
char *data;
enum flag_scheme flag_scheme;
+   int dirty;
 };
 
 static struct environment environment = {
@@ -508,6 +509,9 @@ int fw_env_flush(struct env_opts *opts)
if (!opts)
opts = _opts;
 
+   if (!environment.dirty)
+   return 0;
+
/*
 * Update CRC
 */
@@ -553,7 +557,8 @@ int fw_env_write(char *name, char *value)
 
deleting = (oldval && !(value && strlen(value)));
creating = (!oldval && (value && strlen(value)));
-   overwriting = (oldval && (value && strlen(value)));
+   overwriting = (oldval && (value && strlen(value) &&
+ strcmp(oldval, value)));
 
/* check for permission */
if (deleting) {
@@ -593,6 +598,7 @@ int fw_env_write(char *name, char *value)
/* Nothing to do */
return 0;
 
+   environment.dirty = 1;
if (deleting || overwriting) {
if (*++nxt == '\0') {
*env = '\0';
@@ -1441,6 +1447,7 @@ int fw_env_open(struct env_opts *opts)
"Warning: Bad CRC, using default 
environment\n");
memcpy(environment.data, default_environment,
   sizeof(default_environment));
+   environment.dirty = 1;
}
} else {
flag0 = *environment.flags;
@@ -1503,6 +1510,7 @@ int fw_env_open(struct env_opts *opts)
"Warning: Bad CRC, using default 
environment\n");
memcpy(environment.data, default_environment,
   sizeof(default_environment));
+   environment.dirty = 1;
dev_current = 0;
} else {
switch (environment.flag_scheme) {
-- 
2.16.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot