Tinkering with rebuildfstab

[Rich]

Hi curaga
New timing results:

Code: [Select]

tc@E310:~/rebuildfstab$ time ./newerfindblockdevs > /dev/null
real    0m 0.03s
user    0m 0.01s
sys     0m 0.02s
tc@E310:~/rebuildfstab$
This version also filters out my card readers empty slots (sdc - sdf):

Code: [Select]

tc@E310:~/rebuildfstab$ time ./newerfindblockdevs
DEVNAME=sda     DEVMAJOR=8
DEVNAME=sda1    DEVMAJOR=8
DEVNAME=sda2    DEVMAJOR=8
DEVNAME=sda3    DEVMAJOR=8
DEVNAME=sda4    DEVMAJOR=8
DEVNAME=sda5    DEVMAJOR=8
DEVNAME=sda6    DEVMAJOR=8
DEVNAME=sda7    DEVMAJOR=8
DEVNAME=sdb     DEVMAJOR=8
DEVNAME=sdb1    DEVMAJOR=8
DEVNAME=sr0     DEVMAJOR=11
DEVNAME=sr1     DEVMAJOR=11
DEVNAME=sdg     DEVMAJOR=8
real    0m 0.03s
user    0m 0.01s
sys     0m 0.02s
tc@E310:~/rebuildfstab$
I've attached the script that contains the modified section of rebuildfstab
I'm working on speeding up.

[CNK]

Great spotting Rich! That's a much neater source for the info, as well as a quicker one.

[Rich]

Hi CNK
Thanks, accessing information that the kernel already exposes is much faster.
I'm currently evaluating a replacement for the  /usr/sbin/fstype  script.

[Rich]

Hi curaga
I just looked at the github version and realized a lot of changes have
been made since the TC10 version I'm working with. I copied that
version and when I tried to run it on my TC10 system I got this error:

Code: [Select]

tc@E310:~/rebuildfstab$ time sudo ./rebuildfstab.git
./rebuildfstab.git: line 71: syntax error: unexpected redirection
Command exited with non-zero status 2
real    0m 0.01s
user    0m 0.00s
sys     0m 0.01s
tc@E310:~/rebuildfstab$
This is the section in question:

Code: [Select]

     67   if [ "$MOUNTPOINT" != "none" ]; then
     68     mkdir -p "/mnt/$DEVNAME" 2>/dev/null >/dev/null
     69   fi
     70   printf "%-15s %-15s %-8s %-20s %-s\n" "$DEVROOT/$DEVNAME" "$MOUNTPOINT" "$FSTYPE" "$OPTIONS" "0 0 $ADDEDBY" >>"$TMP"
     71 done < <(find /sys/block/*/ -maxdepth 2 -name dev ! -regex '.*\(/loop\|/ram\|/zram\|\s\).*' | awk -v devroot="$DEVROOT" -v tmptab="$TMP" '
     72     BEGIN {
     73         while (getline < tmptab > 0) remain[$1]=1
     74         FS="/|!"
     75     }
Any idea why it's not backward compatible?

[curaga]

I think that's just missing functionality in the older ash.

[Rich]

Hi curaga
Thanks, I think you're right. I changed the shebang to  bash  and it runs:

Code: [Select]

tc@E310:~/rebuildfstab$ time sudo ./rebuildfstab.git
real    0m 0.92s
user    0m 0.14s
sys     0m 0.32s
tc@E310:~/rebuildfstab$ time sudo ./rebuildfstab.git
real    0m 0.69s
user    0m 0.11s
sys     0m 0.36s
tc@E310:~/rebuildfstab$

[Rich]

Hi CNK
I changed how information is gathered but kept what is done with
it basically the same.

For each test, I cleared the cache, then ran the script 3 times in succession.

Timing on Dell E310 desktop TC10 x86
Original rebuildfstab from TC10:

Code: [Select]

tc@E310:~/rebuildfstab$ time sudo rebuildfstab
real    0m 1.98s
user    0m 0.37s
sys     0m 0.83s
tc@E310:~/rebuildfstab$ time sudo rebuildfstab
real    0m 1.42s
user    0m 0.36s
sys     0m 0.83s
tc@E310:~/rebuildfstab$ time sudo rebuildfstab
real    0m 1.41s
user    0m 0.40s
sys     0m 0.76s
rebuildfstab from git, shebang changed to bash so it would run under TC10:

Code: [Select]

tc@E310:~/rebuildfstab$ time sudo ./rebuildfstab.git
real    0m 1.05s
user    0m 0.17s
sys     0m 0.54s
tc@E310:~/rebuildfstab$ time sudo ./rebuildfstab.git
real    0m 0.78s
user    0m 0.15s
sys     0m 0.39s
tc@E310:~/rebuildfstab$ time sudo ./rebuildfstab.git
real    0m 0.71s
user    0m 0.12s
sys     0m 0.39s
My version of rebuildfstab:

Code: [Select]

tc@E310:~/rebuildfstab$ time sudo ./rebuildfstab.rewrite
real    0m 0.72s
user    0m 0.06s
sys     0m 0.12s
tc@E310:~/rebuildfstab$ time sudo ./rebuildfstab.rewrite
real    0m 0.16s
user    0m 0.09s
sys     0m 0.06s
tc@E310:~/rebuildfstab$ time sudo ./rebuildfstab.rewrite
real    0m 0.23s
user    0m 0.07s
sys     0m 0.11s
I've done testing on 2 desktops (TC9 x86 and TC10 x86) and 1 laptop (TC14.0alpha1 x86_64)
and can no longer find any issues.

Timing on the laptop
rebuildfstab from TC14.0alpha1:

Code: [Select]

tc@box:~$ time sudo ./rebuildfstab
real    0m 0.36s
user    0m 0.06s
sys     0m 0.14s
tc@box:~$ time sudo ./rebuildfstab
real    0m 0.27s
user    0m 0.05s
sys     0m 0.11s
tc@box:~$ time sudo ./rebuildfstab
real    0m 0.31s
user    0m 0.05s
sys     0m 0.12s
My version of rebuildfstab:

Code: [Select]

tc@box:~$ time sudo ./rebuildfstab.rewrite
real    0m 0.14s
user    0m 0.04s
sys     0m 0.06s
tc@box:~$ time sudo ./rebuildfstab.rewrite
real    0m 0.13s
user    0m 0.04s
sys     0m 0.05s
tc@box:~$ time sudo ./rebuildfstab.rewrite
real    0m 0.09s
user    0m 0.04s
sys     0m 0.04s
I've attached a copy for you and any other interested parties to
test, examine the code, and provide feedback.

For those that feel code is self commenting, you can remove most of them like this:

Code: [Select]

tc@E310:~/rebuildfstab$ grep -v "### " rebuildfstab.rewrite > rebuildfstab.stripped
tc@E310:~/rebuildfstab$ ls -l rebuildfstab.git rebuildfstab.rewrite rebuildfstab.stripped
-rwxr-xr-x 1 tc staff 2595 Feb 27 08:56 rebuildfstab.git
-rwxr-xr-x 1 tc staff 4145 Mar  5 10:13 rebuildfstab.rewrite
-rw-r--r-- 1 tc staff 2735 Mar  5 12:33 rebuildfstab.stripped
tc@E310:~/rebuildfstab$

[CNK]

Nice work, it's the fastest yet on the first PC I've tested it on (TC 13, x86_64). It does query blkid multiple times, which as Curaga pointed out earlier is avoided in the current Git-repo code, although the "blkid -s TYPE" command that the current script uses actually takes longer to complete on its own than your whole script does in my tests. Maybe that's because that PC only has six hardware block devices present though.

Also I notice that you didn't add ext4 to the part where "relatime" gets added to the mount options for ext file systems (line 135). I see that the script in the Git repo doesn't do that at all though, and I see that the mount man page says "relatime" is now the default, so that line can probably be removed from your script too.

I'll test it out in TC 13 on another x86_64 PC later. Maybe sometime I'll try it on my 100MHz i486 laptop too and see how many hours it takes.

[curaga]

This version also misses custom mounts, and I feel the queue logic is worse than the current checks. If two more scripts happen to be spawned while one is running, the old version would only rerun once, this would rerun them all.

Regarding comments, some additions are definitely welcome, like the 98 block major etc. Though some new ones are overboard IMHO.

I'd prefer a minimal patch just replacing the find line and editing the awk script correspondingly (and a few comments). Otherwise adding custom mounts and running blkid just once to your version may need multiple grep calls, etc.

[Rich]

Hi curaga

This version also misses custom mounts, ...

Could you provide a detailed example of what gets missed please.

... and I feel the queue logic is worse than the current checks. If two more scripts happen to be spawned while one is running, the old version would only rerun once, this would rerun them all. ...

Exactly. A program calling rebuildfstab expects to find an updated fstab.
Consider 2 programs calling at the same time:

  | ProgramA | rebuildfstab scans devices, updates fstab, launches rebuildfstab again | ProgramA reads fstab |
  | ProgramB | rebuildfstab touches rescan file and exits | ProgramB reads fstab |

ProgramA is blocked until rebuildfstab updates fstab.
ProgramB is allowed to continue, unaware that rebuildfstab did not run for it.

While I don't expect this type of situation to happen often I felt it was a
better way of handling it.

... Otherwise adding custom mounts and running blkid just once ...

I'll take a closer look at that.

[curaga]

Custom mounts: say you have a line mounting /dev/sda1 to /files.

The primary caller of rebuildfstab is udev. The other known callers are at boot time and mnttool, and mnttool does not expect that behavior. So I don't think such a guarantee is needed.

[Rich]

Hi curaga

Custom mounts: say you have a line mounting /dev/sda1 to /files. ...

Do you mean something like this might already be in  /etc/fstab:

Code: [Select]

/dev/sda1       /files          ext4     noauto,users,exec    0 0and in this example I should skip adding:

Code: [Select]

/dev/sda1       /mnt/sda1       ext4     noauto,users,exec    0 0 # Added by TC

[CNK]

The old (TC13) script checked the "$TMP" list of manually-generated fstab entries for any entry already matching the device that it was about to add. It called grep once per round to do this.

One alternative approach besides the Awk solution in the new script (which I don't understand Awk well enough to touch), would be to use "grep -v" to exclude from the lines fed to the "while read" loop all those matching the first field in existing fstab entries stored in $TMP.

For example if you have "grep $FILTERS -v /proc/partitions | while read -r DEVMAJOR DEVMINOR BLOCKS DEVNAME". Start with FILTERS="-e 'loop[0-9]*$' -e 'ram[0-9]*$' -e '^$' -e ' name$'", then write a loop that adds an "-e" onto $FILTERS for each first field of "$TMP". EDIT: "grep $FILTERS -v /proc/partitions" doesn't work, I haven't got time to figure out exactly how wrong I am here...

I didn't really have time to write this post clearly, sorry if it's just confusing.

[CNK]

For example if you have "grep $FILTERS -v /proc/partitions | while read -r DEVMAJOR DEVMINOR BLOCKS DEVNAME". Start with FILTERS="-e 'loop[0-9]*$' -e 'ram[0-9]*$' -e '^$' -e ' name$'", then write a loop that adds an "-e" onto $FILTERS for each first field of "$TMP". EDIT: "grep $FILTERS -v /proc/partitions" doesn't work, I haven't got time to figure out exactly how wrong I am here...

Use backslashes (\) to escape the spaces between arguments, instead of double quotes ("), when setting the FILTERS variable and it works.

[curaga]

Rich, precisely.