Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals

[Philippe Gerum <rpm@xenomai.org>]

On 09/05/2018 11:44 AM, Jan Kiszka wrote:
> On 2018-09-05 11:33, Philippe Gerum wrote:
>> On 09/05/2018 10:34 AM, Jan Kiszka wrote:
>>> On 2018-09-05 10:16, Philippe Gerum wrote:
>>>> On 09/04/2018 07:48 PM, Jan Kiszka wrote:
>>>>>> diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
>>>>>> index 7773a08e5..32df635a8 100644
>>>>>> --- a/kernel/cobalt/synch.c
>>>>>> +++ b/kernel/cobalt/synch.c
>>>>>> @@ -902,7 +902,7 @@ void xnsynch_release_all_ownerships(struct
>>>>>> xnthread *thread)
>>>>>>     }
>>>>>>     EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships);
>>>>>>     -#if XENO_DEBUG(MUTEX_RELAXED)
>>>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>>>>>>       /*
>>>>>>      * Detect when a thread is about to sleep on a synchronization
>>>>>>
>>>>>
>>>>> Confused: This is not for next, is it?
>>>>>
>>>>> E.g. that hunk above does not apply there anymore, for about a year...
>>>>>
>>>>
>>>> Something looks wrong here. The three patches apply on commit
>>>> #e23d58715
>>>> [1] at [2], which is on top of stable/v3.0.x, post-3.0.7 material.
>>>> Which
>>>> tree/branch are you looking at?
>>>
>>> next - stable is yours.
>>>
>>
>> Is it? Ok, for now, until a -stable maintainer is appointed eventually.
>>
>>> If you like review of stable backports, please tag those commits as
>>> [stable] or so. And maybe also explain any deviation.
>>>
>>
>> The way -stable was used is obviously different in the new process, and
>> the patches were consistent with the previous process, so let's drop
>> those patches if you can't review them, I'll rebase over the upcoming
>> -next branch to make it easier for everyone.
>>
>>> My vision would be that only patches that are in next/master would make
>>> it to stable.
>>
>> I understand that you want to align with the basics of the mainline
>> kernel workflow, I'm ok with this. Please set (or re-purpose) the
>> (existing) git branches on gitlab the way you want to, to clarify the
>> situation.
>>
>>   Ideally, no or only minor tweaking should be needed to
>>> such commits for the backport.
>>>
>>
>> This should be ok in most cases. Unfortunately, the XENO_DEBUG() fixes
>> are going to be an exception because they have to spread all over the
>> map - unless they don't go to stable, which could be an issue since the
>> corresponding bug may produce bad kernels.
>>
> 
> It seems we addressed most of the XENO_DEBUG cases in next. Are the

Yes, there is only one missing:

kernel/cobalt/intr.c:#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)

> related commits no stable candidates because they do more? If they are,
> I would start with backporting them, and then address what remains.
> 

The related commits are in essence -stable candidates, done at a time
when -stable was actually the code base the -next branch would be
rebased on periodically (we used to inherit bug fixes in next, not the
other way around via backports).

The main issue with the conflicting patch stems from the fact that the
XENO_DEBUG() changes were originally based on the -next branch, and I
did not backport them to -stable. Now I'm kind of doing so, plugging the
last hole in the process for -stable. That should be done for -next, ...
next.

I'll submit the proper patches.

-- 
Philippe.