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

[Tavian Barnes]

On 20 September 2010 16:22, Kurt J. Bosch <kjb-temp-2009 at alpenjodel.de> wrote:
> 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
>
>
>

Actually, removing the quotes is good enough:

$ MODULES=( '!foo' '!bar' )
$ modules=${MODULES[@]/#\!*}
$ [[ $modules ]] && echo "'$modules' is not a null string"
$

-- 
Tavian Barnes