[arch-general] [PATCH] rc.sysinit: only call modprobe once

[Dave Reisner]

On Mon, Sep 20, 2010 at 05:39:25PM +0200, Kurt J. Bosch wrote:
> 2010-09-20 04:10, Dave Reisner:
> >On Mon, Sep 20, 2010 at 11:57:06AM +1000, Allan McRae wrote:
> >>On 20/09/10 11:54, Dave Reisner wrote:
> >>>Use modprobe -a and a bash PE to filter the MODULES array.
> >>>
> >>>Signed-off-by: Dave Reisner<d at falconindy.com>
> >>>---
> >>>  rc.sysinit |   12 ++++--------
> >>>  1 files changed, 4 insertions(+), 8 deletions(-)
> >>>
> >>>diff --git a/rc.sysinit b/rc.sysinit
> >>>index 09d5e97..4b6e1e7 100755
> >>>--- a/rc.sysinit
> >>>+++ b/rc.sysinit
> >>>@@ -92,14 +92,10 @@ if /bin/pidof -o %PPID /sbin/udevd>/dev/null; then
> >>>  fi
> >>>
> >>>  # Load modules from the MODULES array defined in rc.conf
> >>>-if [[ $load_modules != off&&   -f /proc/modules ]]; then
> >>>-    stat_busy "Loading Modules"
> >>>-    for mod in "${MODULES[@]}"; do
> >>>-	if [[ $mod = ${mod#!} ]]; then
> >>>-	    /sbin/modprobe $mod
> >>>-	fi
> >>>-    done
> >>>-    stat_done
> >>>+if [[ $load_modules != off&&   -f /proc/modules ]]&&   (( ${#MODULES[@]}>   0 )); then
> >>>+	stat_busy "Loading Modules"
> >>>+		/sbin/modprobe -a "${MODULES[@]/#\!*/}"
> >>>+	stat_done
> >>>  fi
> >>>
> >>>  # Wait for udev uevents
> >>
> >>Does this still work in the "null" case where there is only modules
> >>specified with "!" in the front?
> >>
> >>Allan
> >>
> >
> >Excellent observation -- it would not. Counting the size of the array
> >with the PE in place isn't possible (or desirable), either. Perhaps a
> >more graceful solution is to reassign the output of the PE to a new
> >array and operate based on that. Certainly still beats calling modprobe
> >for every element in the array, imo.
> >
> >d
> >
> 
> I think it could be done like so:
> 
> From 6465c90fc851b12cfce791228415ae1c2823e050 Mon Sep 17 00:00:00 2001
> From: Kurt J. Bosch <kjb-temp-2009 at alpenjodel.de>
> Date: Mon, 20 Sep 2010 17:31:54 +0200
> Subject: [PATCH] rc.sysinit: only call modprobe once (as proposed by
> Dave Reisner)
> 
> ---
>  rc.sysinit |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/rc.sysinit b/rc.sysinit
> index 09d5e97..07b3f67 100755
> --- a/rc.sysinit
> +++ b/rc.sysinit
> @@ -92,13 +92,10 @@ if /bin/pidof -o %PPID /sbin/udevd >/dev/null; then
>  fi
> 
>  # Load modules from the MODULES array defined in rc.conf
> -if [[ $load_modules != off && -f /proc/modules ]]; then
> +modules=$( echo ${MODULES[*]/\!*/} )
> +if [[ $modules && $load_modules != off && -f /proc/modules ]]; then
>      stat_busy "Loading Modules"
> -    for mod in "${MODULES[@]}"; do
> -	if [[ $mod = ${mod#!} ]]; then
> -	    /sbin/modprobe $mod
> -	fi
> -    done
> +    /sbin/modprobe -a $modules
>      stat_done
>  fi
> 
> -- 
> 1.7.0.3
> 

Your echo is redundant. Just quote the expansion and assign it.

modules="${MODULES[@]/#\!*}"

I think we're a looooong way away from the day when there's a '!' as
part of a module name, but I think it's probably best to anchor the
expression to the start of each element regardless.

d