[LITMUS^RT] request for review: patches for arbitrary deadlines

Christopher Kenna cjk at cs.unc.edu
Tue Jul 17 08:20:01 CEST 2012


On Mon, Jul 16, 2012 at 5:52 PM, Glenn Elliott <gelliott at cs.unc.edu> wrote:
> Hello All,
>
> Please find two patched attached for review that add support for
> arbitrary deadlines in Litmus.  One patch is for litmus and the other
> is for liblitmus.  Please read the patch commit messages for more
> information.
>
> I'm not exactly happy with the API changes for liblitmus because they
> are disruptive with legacy code (a relative deadline has been added).
> However, this seemed to be the correct change with respect to the API
> though.
>
> Patches are also available for view at rtsrv.unc.edu/cgit for UNC folks.
>
> Thanks,
> Glenn

In jobs.c:
-       t->rt_param.job_params.release   = t->rt_param.job_params.deadline;
-       t->rt_param.job_params.deadline += get_rt_period(t);
+       t->rt_param.job_params.release  += get_rt_period(t);
+       t->rt_param.job_params.deadline  = t->rt_param.job_params.release
+               + get_rt_rdeadline(t);

Why don't we just make a get_release(t) macro? We have one for almost
all the other parameters.

Also related to these macros, would it be possible to make them more
consistent? We have a mix of get_X(t) and get_rt_X(t) macros.
Initially I thought that get_rt were for task-related properties, but
the get_exec_cost(t) is does not use _rt. Maybe I'm missing something.

There's an extra newline at jobs.c:9 :-D

I agree with most other comments, so I won't repeat them. Especially
using deadline instead of rdeadline if possible.

I would also be for passing in a struct. We are accumulating so many
parameters that function signatures are growing quite large and
matching which lt_t corresponds to what is getting unwieldy. The
perf_event subsystem has users pass in a struct for configuration, and
I did not find it to be that annoying (barring the fact that the
documentation is outdated). See, for example, sys_perf_event_open:
https://github.com/torvalds/linux/blob/master/kernel/events/core.c#L6178




More information about the litmus-dev mailing list