Re: [BUG] sched_setattr() SCHED_DEADLINE hangs system

[Juri Lelli]

On Tue, 13 May 2014 12:43:07 +0200
Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:

> On Tue, May 13, 2014 at 11:57:49AM +0200, Juri Lelli wrote:
> >  static bool
> >  __checkparam_dl(const struct sched_attr *attr)
> >  {
> >  	return attr && attr->sched_deadline != 0 &&
> >  		(attr->sched_period == 0 ||
> > -		(s64)(attr->sched_period   - attr->sched_deadline) >= 0) &&
> > -		(s64)(attr->sched_deadline - attr->sched_runtime ) >= 0  &&
> > -		attr->sched_runtime >= (2 << (DL_SCALE - 1));
> > +		(attr->sched_period >= attr->sched_deadline)) &&
> > +		(attr->sched_deadline >= attr->sched_runtime) &&
> > +		attr->sched_runtime >= (1ULL << DL_SCALE) &&
> > +		(attr->sched_deadline < (1ULL << 63) &&
> > +		attr->sched_period < (1ULL << 63));
> >  }
> 
> Could we maybe rewrite that function to look less like a ioccc.org
> submission?
> 

Right.

> static bool
> __checkparam_dl(const struct sched_attr *attr)
> {
> 	if (!attr) /* possible at all? */
> 		return false;
> 

I'd say no, removed.

> 	/* runtime <= deadline <= period */
> 	if (attr->sched_period   < attr->sched_deadline ||
> 	    attr->sched_deadline < attr->sched_runtime)
> 		return false;
> 
> 	/*
> 	 * Since we truncate DL_SCALE bits make sure we're at least that big,
> 	 * if runtime > (1 << DL_SCALE), so must the others be per the above
> 	 */
> 	if (attr->sched_runtime <= (1ULL << DL_SCALE))
> 		return false;
> 
> 	/*
> 	 * Since we use the MSB for wrap-around and sign issues, make
> 	 * sure its not set, if period < 2^63, so must the others be.
> 	 */
> 	if (attr->sched_period & (1ULL << 63))
> 		return false;
> 
> 	return true;
> }
> 
> Did I miss anything?

period can be 0, so we have to check also sched_deadline for MSB set.

Thanks,

- Juri

---