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

[Kurt J. Bosch]

2010-09-20 17:56, 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.
>
NAK. Try this:
MODULES=( '!foo' '!bar' )
modules="${MODULES[@]/#\!*}"
[[ $modules ]] && echo "'$modules' is not a null string"
' ' is not a null string

> 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.
>
OK, but the quotes aren't needed because word splitting is not performed 
here. So we finally get:

modules=$( echo ${MODULES[@]/#\!*} )

--
Kurt