Skip to content

Conversation

@bigsmile74
Copy link
Contributor

What's the reason to use sync? Most of it runs in-memory or on a tmpfs. If you let Linux handle it will save about ~5 seconds in a restart.

I only see it used in the following init files with a good reason:

  • boot: to cement initial config generation.
  • done: after removal of sysupgrade (substantial file size)
  • uhttpd to cement the certificate
  • umount to cement pending writes

I experience no issues by removing them.

before:

# time /etc/init.d/pbr restart
real	0m 7.62s
user	0m 1.31s
sys	0m 0.37s

after:

# time ./pbr restart
real	0m 2.37s
user	0m 1.33s
sys	0m 0.40s

What's the reason to use sync? Most of it runs in-memory or on a tmpfs. If you let Linux handle it will save about ~5 seconds in a restart.

I only see it used in the following init files with a good reason:
* boot: to cement initial config generation.
* done: after removal of sysupgrade (substantial file size)
* uhttpd to cement the certificate
* umount to cement pending writes

I experience no issues by removing them.

before:
```
# time /etc/init.d/pbr restart
real	0m 7.62s
user	0m 1.31s
sys	0m 0.37s
```
after:
```
# time ./pbr restart
real	0m 2.37s
user	0m 1.33s
sys	0m 0.40s
```

Signed-off-by: bigsmile74 <118111679+bigsmile74@users.noreply.github.com>
@bigsmile74
Copy link
Contributor Author

bigsmile74 commented Nov 8, 2024

Also and I did not mention this, there is the penalty of wear-leveling if users run from NAND/NOR flash. So by letting those writes accumulate in cache it would only write the end result. If you still want a sync, I would do it before the script exits and in the background eg: sync & then exit.

@stangri
Copy link
Owner

stangri commented Nov 8, 2024

I believe when I first implemented the rt_tables updates there were issues with people ending up with corrupted tables, that's why the sync was implemented. The issue is if I accept the PR and start releasing new binaries, it may be a while until we get a corruption report. Again, let me sleep on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants