Re: [PATCH] cobalt/posix/process: pipeline: abstract kernel event handlers

[Philippe Gerum <rpm@xenomai.org>]

Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 08.01.21 11:22, Philippe Gerum wrote:
>> 
>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>> 
>>> On 08.01.21 10:49, Philippe Gerum wrote:
>>>>
>>>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>>>>
>>>>> On 08.01.21 08:08, Jan Kiszka via Xenomai wrote:
>>>>>> On 07.01.21 19:27, Jan Kiszka via Xenomai wrote:
>>>>>>> On 07.01.21 18:56, Jan Kiszka via Xenomai wrote:
>>>>>>>> On 17.12.20 19:02, Philippe Gerum wrote:
>>>>>>>>> From: Philippe Gerum <rpm@xenomai.org>
>>>>>>>>>
>>>>>>>>> Although there are significant commonalities between the I-pipe and
>>>>>>>>> Dovetail when it comes to dealing with synchronous kernel events,
>>>>>>>>> there is no strict 1:1 mapping between the two kernel interfaces.
>>>>>>>>>
>>>>>>>>> As an initial step, move all the code handling the kernel events to
>>>>>>>>> the I-pipe section. We may exploit commonalities between the I-pipe
>>>>>>>>> and Dovetail in this area as we gradually merge support for the
>>>>>>>>> latter.
>>>>>>>>>
>>>>>>>>> No functional change is introduced.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Philippe Gerum <rpm@xenomai.org>
>>>>>>>>> ---
>>>>>>>>>  .../cobalt/kernel/ipipe/pipeline/kevents.h    |  31 +
>>>>>>>>>  kernel/cobalt/ipipe/Makefile                  |   4 +-
>>>>>>>>>  kernel/cobalt/ipipe/kevents.c                 | 860 ++++++++++++++++++
>>>>>>>>>  kernel/cobalt/posix/process.c                 | 846 +----------------
>>>>>>>>>  kernel/cobalt/posix/process.h                 |   4 +
>>>>>>>>>  kernel/cobalt/posix/signal.c                  |   5 +
>>>>>>>>>  kernel/cobalt/posix/signal.h                  |   2 +
>>>>>>>>>  kernel/cobalt/thread.c                        |   4 +-
>>>>>>>>>  kernel/cobalt/trace/cobalt-core.h             |  12 +-
>>>>>>>>>  9 files changed, 930 insertions(+), 838 deletions(-)
>>>>>>>>>  create mode 100644 include/cobalt/kernel/ipipe/pipeline/kevents.h
>>>>>>>>>  create mode 100644 kernel/cobalt/ipipe/kevents.c
>>>>>>>>>
>>>>>>>>> diff --git a/include/cobalt/kernel/ipipe/pipeline/kevents.h b/include/cobalt/kernel/ipipe/pipeline/kevents.h
>>>>>>>>> new file mode 100644
>>>>>>>>> index 000000000..30425a96b
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/include/cobalt/kernel/ipipe/pipeline/kevents.h
>>>>>>>>> @@ -0,0 +1,31 @@
>>>>>>>>> +/*
>>>>>>>>> + * SPDX-License-Identifier: GPL-2.0
>>>>>>>>> + *
>>>>>>>>> + * Copyright (C) 2019 Philippe Gerum  <rpm@xenomai.org>
>>>>>>>>> + */
>>>>>>>>> +
>>>>>>>>> +#ifndef _COBALT_KERNEL_IPIPE_KEVENTS_H
>>>>>>>>> +#define _COBALT_KERNEL_IPIPE_KEVENTS_H
>>>>>>>>> +
>>>>>>>>> +struct cobalt_process;
>>>>>>>>> +struct cobalt_thread;
>>>>>>>>> +
>>>>>>>>> +static inline
>>>>>>>>> +int pipeline_attach_process(struct cobalt_process *process)
>>>>>>>>> +{
>>>>>>>>> +	return 0;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static inline
>>>>>>>>> +void pipeline_detach_process(struct cobalt_process *process)
>>>>>>>>> +{ }
>>>>>>>>> +
>>>>>>>>> +int pipeline_prepare_current(void);
>>>>>>>>> +
>>>>>>>>> +void pipeline_attach_current(struct xnthread *thread);
>>>>>>>>> +
>>>>>>>>> +int pipeline_trap_kevents(void);
>>>>>>>>> +
>>>>>>>>> +void pipeline_enable_kevents(void);
>>>>>>>>> +
>>>>>>>>> +#endif /* !_COBALT_KERNEL_IPIPE_KEVENTS_H */
>>>>>>>>> diff --git a/kernel/cobalt/ipipe/Makefile b/kernel/cobalt/ipipe/Makefile
>>>>>>>>> index 6021008fb..5170bb32b 100644
>>>>>>>>> --- a/kernel/cobalt/ipipe/Makefile
>>>>>>>>> +++ b/kernel/cobalt/ipipe/Makefile
>>>>>>>>> @@ -1,3 +1,5 @@
>>>>>>>>> +ccflags-y += -Ikernel
>>>>>
>>>>> "... -I$(srctree)/kernel" - or 5.4 and newer does not build. Also fixed
>>>>> up now.
>>>>>
>>>>
>>>> Please wait. You see way too many issues on merging this patch. Although
>>>> I'm not building on 4.14 (only covering the two latest LTS, i.e. 5.10
>>>> and 4.19), 4.19 does build here. So this may be an issue with options,
>>>> and there might be several other annoying bugs ahead. It would be better
>>>> for me to include 4.14 in my test list, using your .config(s), and
>>>> re-submit from this patch and on.
>>>
>>> Just have a look at our CI/CT. That last issue here was just not caught
>>> by the build test of the Xenomai repo, only by xenomai-image, because
>>> the former does in-tree builds.
>>>
>> 
>> Ok, so shall we reset to the situation prior to merging #7faeaecb8?
>> 
>
> I would continue with testing 12fd31469682 for now, as a next step via
> xenomai-images on various targets (I'm currently just waiting for
> xenomai-images patch by Quirin). So far it looks we have things under
> control now. Or do you have further concerns?

No known issues so far, the idea of resetting was more about sparing you
the need for fixing more annoying glitches with 4.14 I did not even
compile-tested. If you feel confident about the current state, I'm fine
with re-merging your changes on top of my tree and resume from there.

>
> next is not master. If we see further issues, I will simply reset it
> again. I will definitely not merge next into master very soon.
>

For sure, this is definitely bleeding edge stuff at this point.

-- 
Philippe.