From: Richard Weinberger <richard@nod.at>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: xenomai <xenomai@lists.linux.dev>
Subject: Re: [PATCH] [RFC] xnthread_set_periodic: Improve period adjusting support
Date: Tue, 2 Jan 2024 15:07:51 +0100 (CET) [thread overview]
Message-ID: <806798081.189858.1704204471017.JavaMail.zimbra@nod.at> (raw)
In-Reply-To: <983ceb09-c9f3-4b15-9aa1-78fb3d371df4@siemens.com>
----- Ursprüngliche Mail -----
> Von: "Jan Kiszka" <jan.kiszka@siemens.com>
> If I scan our code base correctly, this only affects RTDM tasks, at
> least in-tree. Is that your use case as well?
Exactly. I didn't implement for userspace tasks so far because my use case is
purely RTDM.
>> Alternatively, callers of xnthread_set_periodic() could access
>> task->ptimer directly and pass an adjusted start time to
>> xnthread_set_periodic().
>>
>> Signed-off-by: Richard Weinberger <richard@nod.at>
>> ---
>> kernel/cobalt/thread.c | 15 ++++++++++++---
>> 1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/cobalt/thread.c b/kernel/cobalt/thread.c
>> index ff12f288a..a5304cc8a 100644
>> --- a/kernel/cobalt/thread.c
>> +++ b/kernel/cobalt/thread.c
>> @@ -1335,9 +1335,18 @@ int xnthread_set_periodic(struct xnthread *thread,
>> xnticks_t idate,
>>
>> xntimer_set_affinity(&thread->ptimer, thread->sched);
>>
>> - if (idate == XN_INFINITE)
>> - xntimer_start(&thread->ptimer, period, period, XN_RELATIVE);
>> - else {
>> + if (idate == XN_INFINITE) {
>> + xnticks_t start = period;
>> +
>> + if (xntimer_running_p(&thread->ptimer)) {
>> + xnticks_t elapsed = xntimer_interval(&thread->ptimer) -
>> + __xntimer_get_timeout(&thread->ptimer);
>> + if (elapsed < period)
>> + start = period - elapsed;
>> + }
>> +
>> + xntimer_start(&thread->ptimer, start, period, XN_RELATIVE);
>> + } else {
>> if (timeout_mode == XN_REALTIME)
>> idate -= xnclock_get_offset(xntimer_clock(&thread->ptimer));
>> else if (timeout_mode != XN_ABSOLUTE) {
>
> If we changed the behavior of xnthread_set_period, we would also need to
> update its documentation - which so far states that "now" is the start
> time when calling this service with idate == XN_INFINITE.
Sure. This patch is just an RFC.
> But why not using idate for this use case instead? Is the pattern too
> common to push it to the user?
While this is doable, I found it a bit hacky to access task->ptimer directly.
IMHO ->ptimer is private and should not directly accessed.
At least my other prototype uses xntimer_interval() and __xntimer_get_timeout()
to get all information need from the periodic timer to change the period
while it runs.
> And if changing the current logic, I would wonder why not saying that
> idate == XN_INFINITE means that the change will only take affect for the
> next period, not the current one. Would avoid losing one shot like in
> your proposal above.
I need to think about this but as far I understand your idea this requires
an extra variable in the timer or task struct.
Thanks,
//richard
prev parent reply other threads:[~2024-01-02 14:07 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-20 19:48 [PATCH] [RFC] xnthread_set_periodic: Improve period adjusting support Richard Weinberger
2024-01-02 13:32 ` Jan Kiszka
2024-01-02 14:07 ` Richard Weinberger [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=806798081.189858.1704204471017.JavaMail.zimbra@nod.at \
--to=richard@nod.at \
--cc=jan.kiszka@siemens.com \
--cc=xenomai@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).