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

[Björn Brandenburg]

On Jul 17, 2012, at 5:45 AM, Glenn Elliott wrote:

> Thanks for the feedback!  Comments inline.
> 
>> We could avoid future API pain by passing in initialized rt_task
>> structs into sporadic_task_ns. This would also make it easier to add
>> in extra per-task fields for per-paper branches, which has been
>> handled awkwardly in the past. Besides, I've heard that 7 was a good
>> hard limit for number of parameters to a function. Now
>> sporadic_task_ns has 8.
> 
> Well, I believe those functions were originally meant to simplify interfacing with the system call.  Passing the struct is reasonable, but I think it does hide something from the user-- it's not as clear what parameters need to be filled out.  We also have macros to make things easier.  (That's not to say that we couldn't have macros and the simplified struct API).

Indeed. sporadic_task_ns() is supposed to be a convenience wrapper. It's growing decidedly inconvenient. If you want full control, set_rt_task_param() takes an initialized  struct rt_param.

I think sporadic_task_ns() should simply default to implicit deadlines. In a future patch, we should  probably also remove some of the other less-commonly used parameters (phase, task class, set_cpu_set could be rolled into partition).

Or we could get rid of it entirely and force people to simply use set_rt_task_param().

> 
>>> +	lt_t		rdeadline;
>> 
>> Why not just deadline? I doubt there will be confusion, and it looks a
>> tad cleaner.
> 
> Yes, I thought of that, but I wanted to see what others thought.  There is already a 'deadline' field in the job params struct.

Either deadline or relative_deadline, IMHO.

> 
> 
>>> -	if (tp.period < tp.exec_cost)
>>> +	if (tp.rdeadline < tp.exec_cost)
>>> 	{
>>> 		printk(KERN_INFO "litmus: real-time task %d rejected "
>>> -		       "because wcet > period\n", pid);
>>> +		       "because wcet greater than relative deadline\n", pid);
>>> 		goto out_unlock;
>>> 	}
>> 
>> As far as I can tell, this means there is no longer a check that
>> tp.period >= tp.exec_cost. Am I wrong?
> 
> Yes.  This is a density check.  If no deadline was specified by the user, then rdeadline is equal to period, in which case the conditional is as before.

Density is wcet / min(period, deadline), not wcet / deadline. The check should be MIN(tp.deadline, tp.period) < tp.exec_cost.

Thanks for the patches! I'm looking forward to merging a cleaned-up version.

Thanks,
Björn