> Am 11.11.2015 um 13:49 schrieb Bruce Ashfield <bruce.ashfi...@gmail.com>:
>
> On Wed, Nov 11, 2015 at 4:00 AM, Jens Rehsack <rehs...@gmail.com> wrote:
>>
>>> Am 11.11.2015 um 03:01 schrieb Bruce Ashfield <bruce.ashfi...@gmail.com>:
>>>
>>> On Tue, Nov 10, 2015 at 4:33 AM, Jens Rehsack <rehs...@gmail.com> wrote:
>>>>
>>>>> Am 14.10.2015 um 21:48 schrieb Bruce Ashfield <bruce.ashfi...@gmail.com>:
>>>>>
>>>>> On Wed, Oct 14, 2015 at 3:30 PM, S. Lockwood-Childs <s...@vctlabs.com>
>>>>> wrote:
>>>>>> http://patchwork.openembedded.org/patch/99875/
>>>>>>
>>>>>> Apparently this patch is still not in master, and I just ran across the
>>>>>> problem with an externally built module (omaplfb from omap3-sgx-modules)
>>>>>> in meta-ti layer.
>>>>>>
>>>>>> What's the plan for getting a correct Module.symver into shared_workdir
>>>>>> for external modules to build against? Above patch, or does someone have
>>>>>> an even better idea?
>>>>>
>>>>> Richard and I sync'd on this while in Dublin @ ELCe, and the changes
>>>>> aren't missing
>>>>> from master by mistake .. but more because we are still working to come
>>>>> up with
>>>>> a comprehensive solution (tracked in bugzilla).
>>>>>
>>>>> The solution is pretty much what I described before, we are balancing
>>>>> applications
>>>>> and tasks that do not need kernel modules to be built, versus external
>>>>> modules
>>>>> that depend on symbols from other modules. The devil is in the
>>>>> details, and getting
>>>>> a non-racy, task locked solution that allows the recipe writer to
>>>>> explicitly decide
>>>>> whether they need modules built or not .. attempts at detecting the
>>>>> need, or forcing
>>>>> a one size fits all solution have all lead to dead ends.
>>>>>
>>>>> Since we are close to a release point, I'm still working on this out
>>>>> of the tree, and
>>>>> will propose some changes when the tree looks stable.
>>>>>
>>>>> For now, you can carry the patch locally, or you can append to the kernel
>>>>> module
>>>>> compilation task and do a second copy of the symvers file to the share
>>>>> directory.
>>>>>
>>>>> i.e. a variant of this:
>>>>> http://patchwork.openembedded.org/patch/94891/, done in a
>>>>> bbappend versus the class.
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Bruce
>>>>
>>>> This is kind of insane to try a fix duplicating a job in a probably wrong
>>>> way
>>>> (even a tiny) because of performance issue.
>>>
>>> I have a fix already queued for this,
>>
>> I've seen the fix you referred at
>> http://patchwork.openembedded.org/patch/94891/,
>> this is broken.
>
> No that isn't it. But also, no, that isn't broken. It works, but there is a
> potential for a race, which is what we've been fixing.
>
> Can you elaborate on why you'd declare that broken ?
Because you fix one race condition by introducing another one. If there is
another word than broken for it, I'm happy to learn that,
>>
>>> but I'm currently out of the office ..
>>> I swear, every time I go on vacation, someone brings this up.
>>>
>>> I've been waiting for the smoke to clear on the 2.0 release before
>>> posting it .. so please, just a bit more patience and I'll send out
>>> that series.
>>
>> Maybe giving us an impression whether it's correct or just work for you(tm)
>> :P
>
> For everyone. Richard wouldn't exactly take my series and kernel
> updates if they were only just for me. :)
I didn't talk about your work for the kernel, I'm just talking about your
patch to fix initially mentioned race condition.
>>>> Any 3rd party kernel module depends on do_shared_workdir - so
>>>> do_shared_workdir
>>>> must wait for do_compile_kernelmodules - that's it by design (build is done
>>>> relying on dependencies ordered in a directed, noncyclical graph.
>>>>
>>>> Since compile_kernelmodules is between compile and strip, I vote for
>>>>
>>>> $ git diff
>>>> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
>>>> index 5e8b6cf..49d7561 100644
>>>> --- a/meta/classes/kernel.bbclass
>>>> +++ b/meta/classes/kernel.bbclass
>>>> @@ -253,7 +253,7 @@ kernel_do_install() {
>>>> }
>>>> do_install[prefuncs] += "package_get_auto_pr"
>>>>
>>>> -addtask shared_workdir after do_compile before do_compile_kernelmodules
>>>> +addtask shared_workdir after do_compile_kernelmodules before do_strip
>>>> addtask shared_workdir_setscene
>>>>
>>>> do_shared_workdir_setscene () {
>>>>
>>>> But that's surely kind of smell, whether before do_strip and do_install is
>>>> preferred. Mandatory is, that do_shared_workdir must not be processed
>>>> before
>>>> do_compile_kernelmodules finishes.
>>>>
>>>> There is no way to avoid it, and if those 5 seconds slow down your build,
>>>> there is probably another thing which should be fixed.
>>>
>>> It's not 5 seconds. It is much more on some machines and configurations.
>>
>> Depends on your build machine and much more, your sstate cache,
>> changes to kernel and why you do a full build after each kernel
>> change instead of just deploying the kernel.
>
> Sure. I've been at this for years now .. and I've seen and used lots
> of configs. We are trying to give a choice on what level of building
> triggers.
>
>>
>>> The point is that not everyone who is building modules depends on
>>> other modules and not everyone that is building against the kernel
>>> may even want modules built.
>>
>> That's only build time. How often do you recompile your kernel that
>> this dependency really matters?
>
> I rebuild the kernel a lot .. I'm doing kernel and kernel tools maintenance
> after all :)
Understood, so your primary tooling becomes slower.
>>> The fix is to just re-copy the symbols after kernel modules are
>>> built, and put the onus on the recipe writer to depend on the
>>> right task/variant. By default, do_shared_workdir won't have that
>>> dependency, but anyone with a recipe that does depend on other
>>> module symbols will get that extra copy and dependency created.
>>
>> This is simply wrong, period. Because (I already explained that)
>> module-base.bbclass adds a configure-stage dependency to do_shared_workdir.
>>
>
> And I explained that I didn't need that explanation.
I'm sorry - but I didn't got that. I still don't get that you
don't need that explanation. Even if you understand more and deeper
what's going on when building kernel and tools and 3rd party
modules around that - several people running into that race condition
and you disagree that moving the race keeps the thing broken.
> There's really no need to escalate your language. It isn't needed.
>
>> You can avoid the mistake by adding a sane stage for redo_shared_workdir
>> after do_compile_kernelmodules before do_strip and reassign the 3rd
>> party module bbclass to depend on redo_shared_workdir.
>>
>> OTOH I think, build performance isn't worth a however sophisticated
>> fragile patch. Let's apply Stefan's patch and be my guest for sending
>> an performance optimized one when you figured it out.
>>
>> @Richard - isn't it worth use the correct, even if slow, patch and
>> let Bruce anytime in not to distant future provide an optimized one
>> which can be settled and backported for 2.0.1?
>>
>> Isn't release time soon enough?
>>
>> @Stefan - would you agree to "addtask shared_workdir after
>> do_compile_kernelmodules before do_strip"
>> instead of "addtask shared_workdir after do_compile_kernelmodules before
>> do_install"?
>>
>
> I'm checking out of the conversation, since I'm on vacation and shouldn't
> be near a computer.
>
> I indicated months ago that RP has the final say, and he can merge any
> variants of the patches he wants, since they don't break anything and
> fix the problem.
>
> RP: I'll ack either patch that has been posted. I'll will revisit all of this
> when I'm working on my 2.1 kernel packaging changes anyway, so
> there's no need to wait.
Great! Thanks, Bruce.
Cheers
--
Jens Rehsack - rehs...@gmail.com
--
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
