Patchwork [v2,09/12] udev-cache: get system config immediately before cache

login
register
mail settings
Submitter Richard Tollerton
Date Aug. 22, 2014, 9:30 p.m.
Message ID <7db18f933460f111085749ee49a70796eab4068a.1408742574.git.rich.tollerton@ni.com>
Download mbox | patch
Permalink /patch/78855/
State New
Headers show

Comments

Richard Tollerton - Aug. 22, 2014, 9:30 p.m.
The system device configuration -- /proc/devices, /proc/cmdline, etc. --
and the contents of /dev must be stored as simultaneously as possible.
Otherwise, hotplug events could be processed between the time we get the
system config and the time we archive /dev, and the udev cache may be
incorrect.

To solve this, update the cached system configuration inside the
udev-cache initscript, not the udev initscript. Also, run it before
archiving /dev, because it should execute nearly instantaneously.

We still need to compute the system configuration inside the udev
initscript, to detect if it changed, so refactor it into a new function
sysconf_cmd(). This also allows administrators to modify it for
machine-specific requirements.

Evaluating the system configuration requires a shell function
readfiles() defined in the udev initscript. The only salient differences
between readfiles() and just using `cat` directly are a) readfiles()
skips files without error if they don't exist, which is necessary
because /proc/atags is in CMP_FILE_LIST but does not exist on all
targets; and b) input lines are concatenated into an output shell
variable with newlines stripped. (a) can be eliminated by not including
/proc/atags in CMP_FILE_LIST if it doesn't exist. (b) is O(n^2) anyway,
and makes $NEWDATA, $OLDDATA etc. very hard to read, and we don't need
to strip newlines anyway. So make sysconf_cmd() use `cat` and remove
readfiles().

Instead of storing system configuration in shell variables ($NEWDATA,
$OLDDATA), store them directly in files: redirect to $SYSCONF_TMP
instead of storing in $NEWDATA; read from $SYSCONF_CACHED directly
instead of reading it into $OLDDATA. This turns comparing $NEWDATA vs.
$OLDDATA into a call to `cmp`. However, busybox lacks `cmp -q`, so
instead, redirect cmp's output to /dev/null.

$OLDDATA and $NEWDATA go away. This obviates the most of error message
printed when the udev cache can't be used, but it probably wasn't of
much help, because $OLDDATA and $NEWDATA are so large that they would
have scrolled off the screen anyway. Beyond that, this error is so
important that it should probably be printed regardless of the setting
of VERBOSE. So leave the "udev: cache not used" log message and remove
everything else.

Because the udev-cache initscript evaluates the system configuration
itself, it makes no sense to signal it to run by storing the system
configuration in a file. Instead, signal udev-cache execution by
touching a file, $DEVCACHE_REGEN.

Signed-off-by: Richard Tollerton <rich.tollerton@ni.com>
---
 meta/recipes-core/udev/udev/init       | 44 +++++++++++++---------------------
 meta/recipes-core/udev/udev/udev-cache | 17 +++++++++++--
 2 files changed, 31 insertions(+), 30 deletions(-)
Richard Purdie - Aug. 23, 2014, 11:11 a.m.
On Fri, 2014-08-22 at 16:30 -0500, Richard Tollerton wrote:
> The system device configuration -- /proc/devices, /proc/cmdline, etc. --
> and the contents of /dev must be stored as simultaneously as possible.
> Otherwise, hotplug events could be processed between the time we get the
> system config and the time we archive /dev, and the udev cache may be
> incorrect.
> 
> To solve this, update the cached system configuration inside the
> udev-cache initscript, not the udev initscript. Also, run it before
> archiving /dev, because it should execute nearly instantaneously.
> 
> We still need to compute the system configuration inside the udev
> initscript, to detect if it changed, so refactor it into a new function
> sysconf_cmd(). This also allows administrators to modify it for
> machine-specific requirements.
> 
> Evaluating the system configuration requires a shell function
> readfiles() defined in the udev initscript. The only salient differences
> between readfiles() and just using `cat` directly are a) readfiles()
> skips files without error if they don't exist, which is necessary
> because /proc/atags is in CMP_FILE_LIST but does not exist on all
> targets; and b) input lines are concatenated into an output shell
> variable with newlines stripped. (a) can be eliminated by not including
> /proc/atags in CMP_FILE_LIST if it doesn't exist. (b) is O(n^2) anyway,
> and makes $NEWDATA, $OLDDATA etc. very hard to read, and we don't need
> to strip newlines anyway. So make sysconf_cmd() use `cat` and remove
> readfiles().

The reason this was done was actually c) avoid having to execute cat as
a new process.

> Instead of storing system configuration in shell variables ($NEWDATA,
> $OLDDATA), store them directly in files: redirect to $SYSCONF_TMP
> instead of storing in $NEWDATA; read from $SYSCONF_CACHED directly
> instead of reading it into $OLDDATA. This turns comparing $NEWDATA vs.
> $OLDDATA into a call to `cmp`. However, busybox lacks `cmp -q`, so
> instead, redirect cmp's output to /dev/null.

The above reason c) is also why we don't use cmp. It turned out to be
faster to read into a variable than fork/exec cmp.

Cheers,

Richard
Richard Tollerton - Aug. 25, 2014, 6:20 p.m.
Richard Purdie <richard.purdie@linuxfoundation.org> writes:

> The above reason c) is also why we don't use cmp. It turned out to be
> faster to read into a variable than fork/exec cmp.

You appear to be wrong: cat+cmp is 27-41% faster than readfiles. In
fact, testing cat alone (without cmp) shows that `cat` to tmpfs file is
2x-3x as fast as readfiles.

These figures reproduce on both a Xilinx Zynq Cortex-A9 and my Xeon dev
machine; the former running busybox sh, the latter running bash.

See below for test script.

---

#!/bin/busybox sh

files="/proc/version /proc/cmdline /proc/devices"
runs=100

readfiles () {
   READDATA=""
   for filename in $@; do
	   if [ -r $filename ]; then
		   while read line; do
			   READDATA="$READDATA$line"
		   done < $filename
	   fi
   done
}

dotest_cat_inner () {
	cat $@
}

dotest_cat () {
	cat $@ > /tmp/a.txt
	cat $@ > /tmp/b.txt
	cmp /tmp/a.txt /tmp/b.txt
}

dotest_readfiles_inner () {
	readfiles $@
	echo "$READDATA"
}

dotest_readfiles () {
	readfiles $@
	NEWDATA="$READDATA"
	readfiles $@
	OLDDATA="$OLDDATA"
	[ "$OLDDATA" = "$NEWDATA" ]
}

timetest () {
	cmd=$1
	i=$2
	shift 2
	while [ "$i" -gt 0 ]; do
		$cmd $@ >/dev/null
		i=$(( i-1 ))
	done
}

case $1 in
cat) timetest dotest_cat $runs $files ;;
readfiles) timetest dotest_readfiles $runs $files ;;
inner_cat) timetest dotest_cat_inner $runs $files ;;
inner_readfiles) timetest dotest_readfiles_inner $runs $files ;;
*) 
	printf 'cat:\n'
	time $0 cat

	printf '\nreadfiles:\n'
	time $0 readfiles

	printf '\ncat_inner:\n'
	time $0 inner_cat

	printf '\nreadfiles_inner:\n'
	time $0 inner_readfiles
	;;
esac


> Cheers,
>
> Richard
>

Patch

diff --git a/meta/recipes-core/udev/udev/init b/meta/recipes-core/udev/udev/init
index 7ffa4a6..10cbf56 100644
--- a/meta/recipes-core/udev/udev/init
+++ b/meta/recipes-core/udev/udev/init
@@ -16,21 +16,20 @@  export TZ=/etc/localtime
 [ -x @UDEVD@ ] || exit 1
 SYSCONF_CACHED="/etc/udev/cache.data"
 SYSCONF_TMP="/dev/shm/udev.cache"
+DEVCACHE_REGEN="/dev/shm/udev-regen" # create to request cache regen
+
+# List of files whose contents will be included in cached system state.
+CMP_FILE_LIST="/proc/version /proc/cmdline /proc/devices"
+[ -f /proc/atags ] && CMP_FILE_LIST="$CMP_FILE_LIST /proc/atags"
+
+# Command to compute system configuration.
+sysconf_cmd () {
+	cat -- $CMP_FILE_LIST
+}
 [ -f /etc/default/udev-cache ] && . /etc/default/udev-cache
 [ -f /etc/udev/udev.conf ] && . /etc/udev/udev.conf
 [ -f /etc/default/rcS ] && . /etc/default/rcS
 
-readfiles () {
-   READDATA=""
-   for filename in $@; do
-	   if [ -r $filename ]; then
-		   while read line; do
-			   READDATA="$READDATA$line"
-		   done < $filename
-	   fi
-   done
-}
-
 kill_udevd () {
     pid=`pidof -x udevd`
     [ -n "$pid" ] && kill $pid
@@ -62,35 +61,24 @@  case "$1" in
     mkdir -p /var/volatile/tmp
 
     # Cache handling.
-    # A list of files which are used as a criteria to judge whether the udev cache could be reused.
-    CMP_FILE_LIST="/proc/version /proc/cmdline /proc/devices /proc/atags"
     if [ "$DEVCACHE" != "" ]; then
             if [ -e $DEVCACHE ]; then
-		    readfiles $CMP_FILE_LIST
-		    NEWDATA="$READDATA"
-		    readfiles "$SYSCONF_CACHED"
-		    OLDDATA="$READDATA"
-		    if [ "$OLDDATA" = "$NEWDATA" ]; then
+		    sysconf_cmd > $SYSCONF_TMP
+		    if cmp $SYSCONF_CACHED $SYSCONF_TMP >/dev/null; then
                             tar xmf $DEVCACHE -C / -m
                             not_first_boot=1
                             [ "$VERBOSE" != "no" ] && echo "udev: using cache file $DEVCACHE"
-                            [ -e $SYSCONF_TMP ] && rm -f "$SYSCONF_TMP"
+                            [ -e "$DEVCACHE_REGEN" ] && rm -f "$DEVCACHE_REGEN"
                     else
 			    # Output detailed reason why the cached /dev is not used
-			    if [ "$VERBOSE" != "no" ]; then
-				    echo "udev: udev cache not used"
-				    echo "udev: we use $CMP_FILE_LIST as criteria to judge whether the cache /dev could be resued"
-				    echo "udev: olddata: $OLDDATA"
-				    echo "udev: newdata: $NEWDATA"
-			    fi
-			    echo "$NEWDATA" > "$SYSCONF_TMP"
+			    echo "udev: cache not used"
+			    touch "$DEVCACHE_REGEN"
                     fi
 	    else
 		    if [ "$ROOTFS_READ_ONLY" != "yes" ]; then
 			    # If rootfs is not read-only, it's possible that a new udev cache would be generated;
 			    # otherwise, we do not bother to read files.
-			    readfiles $CMP_FILE_LIST
-			    echo "$READDATA" > "$SYSCONF_TMP"
+			    touch "$DEVCACHE_REGEN"
 		    fi
             fi
     fi
diff --git a/meta/recipes-core/udev/udev/udev-cache b/meta/recipes-core/udev/udev/udev-cache
index 497d257..5a1bc78 100644
--- a/meta/recipes-core/udev/udev/udev-cache
+++ b/meta/recipes-core/udev/udev/udev-cache
@@ -19,6 +19,16 @@  export TZ=/etc/localtime
 DEVCACHE_TMP="/dev/shm/udev-cache-tmp.tar"
 SYSCONF_CACHED="/etc/udev/cache.data"
 SYSCONF_TMP="/dev/shm/udev.cache"
+DEVCACHE_REGEN="/dev/shm/udev-regen" # create to request cache regen
+
+# List of files whose contents will be included in cached system state.
+CMP_FILE_LIST="/proc/version /proc/cmdline /proc/devices"
+[ -f /proc/atags ] && CMP_FILE_LIST="$CMP_FILE_LIST /proc/atags"
+
+# Command to compute system configuration.
+sysconf_cmd () {
+	cat -- $CMP_FILE_LIST
+}
 [ -f /etc/default/udev-cache ] && . /etc/default/udev-cache
 
 if [ "$ROOTFS_READ_ONLY" = "yes" ]; then
@@ -26,13 +36,16 @@  if [ "$ROOTFS_READ_ONLY" = "yes" ]; then
     exit 0
 fi
 
-if [ "$DEVCACHE" != "" -a -e "$SYSCONF_TMP" ]; then
+if [ "$DEVCACHE" != "" -a -e "$DEVCACHE_REGEN" ]; then
 	echo "Populating dev cache"
+	# Run sysconf_cmd before `tar`, not after, so that as little time as
+	# possible elapses between creating $SYSCONF_CACHED and $DEVCACHE.
+	sysconf_cmd > "$SYSCONF_CACHED"
 	find /dev -xdev \( -type b -o -type c -o -type l \) | cut -c 2- \
 		| xargs tar cf "${DEVCACHE_TMP}" -T-
 	gzip < "${DEVCACHE_TMP}" > "$DEVCACHE"
 	rm -f "${DEVCACHE_TMP}"
-	mv "$SYSCONF_TMP" "$SYSCONF_CACHED"
+	rm -f "$DEVCACHE_REGEN"
 fi
 
 exit 0