[LITMUS^RT] proposing liblitmus API additions and changes
Glenn Elliott
gelliott at cs.unc.edu
Sat Mar 2 00:04:36 CET 2013
Thanks for the comments! Some inline below.
On Mar 1, 2013, at 5:35 PM, Björn Brandenburg <bbb at mpi-sws.org> wrote:
> Hi Glenn,
>
> thanks a lot for your patches! I'm very glad to see more "infrastructure improvements" patches that make the system easier to use or more robust. Comments inline.
<snip>
> These changes look ok to me (though I liked the be_ in be_migrate_to() to indicate that only best-effort tasks may call it). I left a few low-level comments in Github.
Why is this restriction necessary? Perhaps I was running into a bug earlier, but I ran into the situation where non-be tasks not on a CPU in the proper cluster could not transition to real-time. I had to use be_migrate_to() to make things work. I assumed that "be_" was a relic and that the proper affinity masks always had to be setup for all task categories.
>> #2: prop/pthreadification
>> This branch introduces a new pthread-styled API to setting up real-time task parameters. The sporadic_* functions and macros are taking ever more parameters. Worse, these parameters do not always apply to the active scheduler (ex. fixed priority). This API should hopefully simply things with a familiar interface and make code more expressive.
>
> Mh, this looks less straightforward to me. To be honest, it looks a bit too complicated for my taste.
>
> First, the sporadic_* macros were never meant to be the primary interface. The fill-in-a-struct method is the primary method for configuring task parameters. The macros were intended to be convenience wrappers for common cases. If they seem more trouble than benefit now, I'm happy removing them entirely.
>
> Second, what's wrong with simply populating a struct and passing it to a syscall? I don't really see what we gain from introducing a getter/setter API, especially not with verbose litmus_attr_… prefixes. Pthreads was standardized to be portable and needs all this indirection; we're not portable anyway so I'd prefer to keep it simple. That said, if there's broad consensus that this API is better I'm ok merging it. It just seems to make things more complicated to me.
You raise many fair points. My goal was to make things bullet proof. Perhaps this is just me, but I think this API would be easier for someone new to grasp quickly. "Oh, it's like pthreads. I set these flags and the task changes how it runs." But there is a lot more code. For many of the fields, getters and setters are simple wrappers that we could do away with. However, I did have to add some extra fields to litmus_attr_t to get the automatic CPU affinity stuff to work. Is that useful, or should we let the programmer set up the affinity masks themselves?
> Let's standardize on nanoseconds everywhere, with the exception of APIs that operate on doubles. Unit conversion should be limited to parameter parsing code.
How do you feel about adding "_ns" to each parameter name? I guess we could put a big comment in the litmus.h header. "ALL TIME VALUES ARE NANOSECOND." I'm okay with either.
>> (5) caller only needs to specify the parameters they care about (no need to deal with budgets or fixed priorities);
>
> The same is true with the param struct approach. Just zero the struct first and then fill in what you want.
This works as long as every default behavior we want is denoted by a 0-value. We also have to reassign values if we want to change the default behaviors. How about a rt_task or litmus_attr initialization function? That would be even easier than calling memset().
> I would suggest to merge a simplified and rebased version of the migration API changes, and to wait for consensus regarding the attr patch set.
No problem!
More information about the litmus-dev
mailing list