lkml.org 
[lkml]   [2023]   [Aug]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v7 8/8] kbuild: modinst: do modules_install step by step
From
On 08/08/23 00:14, Masahiro Yamada wrote:
> On Mon, Aug 7, 2023 at 5:08 PM Shreenidhi Shedi <yesshedi@gmail.com> wrote:
>>
>> On 07/08/23 01:02, Masahiro Yamada wrote:
>>> On Fri, Jun 23, 2023 at 11:54 PM Shreenidhi Shedi <yesshedi@gmail.com> wrote:
>>>>
>>>> Currently Makefile.modinst does three tasks on each module built:
>>>> - Install modules
>>>> - Sign modules
>>>> - Compress modules
>>>>
>>>> All the above tasks happen from a single place.
>>>>
>>>> This patch divides this task further and uses a different makefile for
>>>> each task.
>>>> Signing module logic is completely refactored and everything happens
>>>> from a shell script now.
>>>>
>>>> Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com>
>>>
>>>
>>> This patch is bad in multiple ways.
>>>
>>> 1. Break "make modules_sign"
>>
>> Correct, somehow I missed it. I will fix it.
>> I'm using below command to test sign only option. Please let me know if
>> I should use something else.
>>
>> make modules_sign modules_sign_only=1 INSTALL_MOD_PATH=$PWD/tmp -j8
>>
>>> 2. Serialize the installation steps, that is, works less efficiently
>>
>> Even in the existing system it happens in serially.
>
> The existing code runs in parallel.
>
> 1. Copy the module "foo.ko" to the destination
> 2. Sign the module "bar.ko"
> 3. Compress the module "baz.ko"
>
> Those three have no dependency among them, so
> should be able to run in parallel.
>
> Your code serializes 1 -> 2 -> 3
>
>
>
>> And the existing
>> method takes more time than the proposed version.
>>
>> root@ph5dev:~/linux-6.3.5 # ./test.sh orig
>>
>> real 0m14.699s
>> user 0m55.519s
>> sys 0m9.036s
>>
>> root@ph5dev:~/linux-6.3.5 # ./test.sh new
>>
>> real 0m13.327s
>> user 0m46.885s
>> sys 0m6.770s
>>
>> Here is my test script.
>> ```
>> #!/bin/bash
>>
>> set -e
>>
>> if [ "$1" != "new" ] && [ "$1" != "orig" ]; then
>> echo "invalid arg, ($0 [orig|new])" >&2
>> exit 1
>> fi
>>
>> rm -rf $PWD/tmp
>>
>> s="scripts/sign-file.c"
>> m="scripts/Makefile.modinst"
>> fns=($s $m)
>>
>> for f in ${fns[@]}; do
>> cp $f.$1 $f
>> done
>>
>> cd scripts
>> gcc -o sign-file sign-file.c -lcrypto
>> cd -
>>
>> time make modules_install INSTALL_MOD_PATH=$PWD/tmp -s -j$(nproc)
>> ```
>>
>>> 3. Increase code without adding any benefits.
>>> Agree with increased code but this change is one step closer to Unix
>> philosophy, do one thing well wrt modules_install.
>
>
> I do not understand why "closer to Unix philosophy"?
>
> You are adding extra/unnecessary complexity.
>
> Currently, the parallel job is managed by Make's job server.
>
> You are introducing another way of parallel execution
> in scripts/signfile.sh
> (and you completely ignored -j <jobs> option to Make,
> and always spawned $(nproc) threads).
>
>
> Leave the parallel execution GNU Make.
> That is how Kbuild works _properly_.
>
>
>
>
>>> There is no good reason to do these chang >I hope the data I provided above to your 2nd point provides evidence
>> that this fix is improving existing system. Please take a look again.
>
>
> I saw it. I re-confirmed this is not an improvement. Thanks for the data.
>
> As I replied to the other thread, my measurement did not show an
> attractive result.
> https://lore.kernel.org/lkml/CAK7LNATNRchNoj0Y6sdb+_81xwV3kAX57-w5q2zew-q8RyzJVg@mail.gmail.com/T/#m8234fc76e631363fbe6bdfb6e45ef6727fc48e80
>
>
>>
>>> NACK.
>>
>> Hi Masahiro Yamada,
>>
>> Replies inline above.
>>
>> Please correct me if my understanding is wrong. Thanks a lot for your
>> time and patience. Have a nice time ahead.
>
>
> I must let you know you are misunderstanding
> the meaning of NACK.
>
>
> NACK means:
> "I do not like it. Please do not submit it any more".
>
>
>
> --
> Best Regards
> Masahiro Yamada

Thanks for all the advice and your time, I'm getting used to this
process. I dropped Kbuild changes from my patch series v9. PTAL.

Have a nice time ahead.

--
Shedi

\
 
 \ /
  Last update: 2023-08-09 19:30    [W:0.044 / U:0.200 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site