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

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 rasmus.villemoes@prevas.dk --- 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 = &default_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) {

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

Hi
On Mon., 24 Sep. 2018, 8:19 am Rasmus Villemoes, rasmus.villemoes@prevas.dk 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

On Wed, Sep 5, 2018 at 8:23 PM Rasmus Villemoes rasmus.villemoes@prevas.dk 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 rasmus.villemoes@prevas.dk
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 = &default_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

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 rasmus.villemoes@prevas.dk 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 rasmus.villemoes@prevas.dk
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

On 2018-09-24 09:42, Alex Kiernan wrote:
On Wed, Sep 5, 2018 at 8:23 PM Rasmus Villemoes rasmus.villemoes@prevas.dk 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 rasmus.villemoes@prevas.dk
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
participants (4)
-
Alex Kiernan
-
Joakim Tjernlund
-
Michael Nazzareno Trimarchi
-
Rasmus Villemoes