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

[Glenn Elliott]

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).


>> +	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.


>> -	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.


> 
>> +	BUG_ON(!t);
>> +
>> +	/* prepare next release */
>> +	t->rt_param.job_params.release = start;
>> +	t->rt_param.job_params.deadline  = start + get_rt_rdeadline(t);
>> +	t->rt_param.job_params.exec_time = 0;
>> +
>> +	/* update job sequence number */
>> +	t->rt_param.job_params.job_no++;
>> +
>> +	/* don't confuse Linux */
>> +	t->rt.time_slice = 1;
>> +
> 
> The following might be a lesser evil. Duplicated logic is always scary.
> 
> inline void do_release(struct task_struct *t, lt_t time)
> {
> 	BUG_ON(!t);
> 
> 	/* prepare next release */
> 	t->rt_param.job_params.release = time;
> 	t->rt_param.job_params.deadline  = time+ get_rt_rdeadline(t);
> 	t->rt_param.job_params.exec_time = 0;
> 
> 	/* update job sequence number */
> 	t->rt_param.job_params.job_no++;
> 
> 	/* don't confuse Linux */
> 	t->rt.time_slice = 1;
> }
> 
> void release(struct task_struct *t)
> {
>        do_release(get_release(t) + get_rt_deadline(t));
> }
> 
> void release_at(struct task_struct *t, lt_t time)
> {
>        do_release(t, time);
>        set_rt_flags(t, RT_F_RUNNING);
> }

That is reasonable.  Is "t->rt. time_slize = 1" safe to call on both release_at() and prepare_for_next_period() code paths?

-Glenn