Re: [U-Boot] [PATCH] tools/fw_env: use fsync to ensure that data is physically stored

[Vincent Prince]

For completness, here is the original post url :

https://patchwork.ozlabs.org/patch/245097/
https://lists.denx.de/pipermail/u-boot/2013-May/154752.html



2017-07-13 15:11 GMT+02:00 Vincent Prince :

> Hi all,
>
> We had the same issue on our custom board, and this patch fixed it.
>
> We use fw_setenv for updating our BSP, and before the patch, 100 boards
> over 170 didn't work,
> and after, 170/170 did get the update.
>
> This patches worked well for us,
> Thanks Michael
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] tools/fw_env: use fsync to ensure that data is physically stored

[Vincent Prince]

Hi all,

We had the same issue on our custom board, and this patch fixed it.

We use fw_setenv for updating our BSP, and before the patch, 100 boards
over 170 didn't work,
and after, 170/170 did get the update.

This patches worked well for us,
Thanks Michael
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] tools/fw_env: use fsync to ensure that data is physically stored

[Mats Kärrman]

Hi Michael,

Michael Heimpold [m...@heimpold.de] wrote:
 fw_setenv state=2
 dd if=... of=/dev/mmcblk0...
 fw_setenv state=1

How about:

fw_setenv state 1  sync

BR // Mats
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] tools/fw_env: use fsync to ensure that data is physically stored

[Mark Jackson]

On 21/05/13 18:34, Michael Heimpold wrote:
 Hi Wolfgang Denx,
 
 Closing a file descriptor does not guarantee that the data has been
 successfully saved to disk, as the kernel might defer the write.

 What is the exact problem you are trying to fix?

 I mean, when exactly does adding the sync play a role?
 
 I'm using fw_setenv during system update process. The sequence
 of such a shell script is something like (much simplified):
 
 ...
 fw_setenv state=2
 dd if=... of=/dev/mmcblk0...
 fw_setenv state=1
 ...
 reboot

Not sure what final OS environment you're running, but I would think
that reboot would sync for you ?

For instance, under BusyBox, we have:-

# reboot --help
BusyBox v1.14.0 (2012-02-15 10:28:26 GMT) multi-call binary

Usage: reboot [-d delay] [-n] [-f]

Reboot the system

Options:
-d  Delay interval for rebooting
-n  No call to sync()
-f  Force reboot (don't go through init)

... and under Ubuntu, we have ...

$ reboot --help
Usage: reboot [OPTION]...
Reboot the system.

Options:
  -n, --no-sync   don't sync before reboot or halt
...

So by default, reboot would (should ?) call sync automatically.

This might point to some other issue ?

Mark J.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] tools/fw_env: use fsync to ensure that data is physically stored

[Michael Heimpold]

Hi,

  ...
  fw_setenv state=2
  dd if=... of=/dev/mmcblk0...
  fw_setenv state=1
  ...
  reboot
 
 Not sure what final OS environment you're running, but I would think
 that reboot would sync for you ?

I'm using OpenWRT and reboot links to the busybox implementation.
This implemenetation calls sync when I traced it correctly.

According to man 2 sync:

quote
DESCRIPTION
   sync() causes all buffered modifications to file metadata and data to be 
written to the underlying file systems.
/quote

When I use fw_setenv with /dev/mmcblk0, that means with a block device directly,
then I have a problem matching the filesystem layer of the description above 
with
the block layer which I am using.

Futhermore another quote from the very same man page:
quote
BUGS
   ...sync() schedules the writes, but may return before the actual writing 
is done.  However, since version  1.3.20  Linux
   does actually wait.  (This still does not guarantee data integrity: 
modern disks have large caches.)
/quote

So it seems to me, that calling sync doesn't do the job.

When looking at man 2 fsync I read 
quote
... This includes writing through or flushing a disk  cache  if
present.  The call blocks until the device reports that the transfer has 
completed
/quote

This looks much better.

However, I did not trace the call chain in linux kernel down to the block layer 
yet.
Maybe I should.

BR, Michael

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

Re: [U-Boot] [PATCH] tools/fw_env: use fsync to ensure that data is physically stored

[Wolfgang Denk]

Dear Michael Heimpold,

In message 1369078482-5863-1-git-send-email-m...@heimpold.de you wrote:
 Closing a file descriptor does not guarantee that the data has been
 successfully saved to disk, as the kernel might defer the write.

What is the exact problem you are trying to fix?

I mean, when exactly does adding the sync play a role?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Old programmers never die, they just branch to a new address.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] tools/fw_env: use fsync to ensure that data is physically stored

[Michael Heimpold]

Hi Wolfgang Denx,

  Closing a file descriptor does not guarantee that the data has been
  successfully saved to disk, as the kernel might defer the write.
 
 What is the exact problem you are trying to fix?
 
 I mean, when exactly does adding the sync play a role?

I'm using fw_setenv during system update process. The sequence
of such a shell script is something like (much simplified):

...
fw_setenv state=2
dd if=... of=/dev/mmcblk0...
fw_setenv state=1
...
reboot

The (redundant) environment is stored in a eMMC flash.
The env var 'state' gives a hint to U-Boot whether/where the process
was interrupted.

So my intension is to be absolutely sure, that when fw_setenv returns,
the environment is written out to disk.

Best regards,
Michael

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

[U-Boot] [PATCH] tools/fw_env: use fsync to ensure that data is physically stored

[Michael Heimpold]

Closing a file descriptor does not guarantee that the data has been
successfully saved to disk, as the kernel might defer the write.

Signed-off-by: Michael Heimpold m...@heimpold.de
---
 tools/env/fw_env.c |   12 
 1 file changed, 12 insertions(+)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 428e374..0420495 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1033,7 +1033,19 @@ static int flash_io (int mode)
 
rc = flash_write (fd_current, fd_target, dev_target);
 
+   if (fsync (fd_current)) {
+   fprintf (stderr,
+fsync failed on %s: %s\n,
+DEVNAME (dev_current), strerror (errno));
+   }
+
if (HaveRedundEnv) {
+   if (fsync (fd_target)) {
+   fprintf (stderr,
+fsync failed on %s: %s\n,
+DEVNAME (dev_current), strerror 
(errno));
+   }
+
if (close (fd_target)) {
fprintf (stderr,
I/O error on %s: %s\n,
-- 
1.7.10.4

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