Re: [PATCH] revl: Add sched() to thread::Builder

[Philippe Gerum <rpm@xenomai.org>]

Jussi Viiri <ilmai@iki.fi> writes:

> When spawning threads with Builder, it wasn't possible to set the scheduling policy of the spawned thread as you don't have access to the Thread object after spawning. Added that option to Builder.
>
> Signed-off-by: Jussi Viiri <ilmai@iki.fi>
>
> ---
>
>  src/thread.rs | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/src/thread.rs b/src/thread.rs
> index d487658..b0300af 100644
> --- a/src/thread.rs
> +++ b/src/thread.rs
> @@ -11,6 +11,7 @@ use std::ptr;
>  use std::os::raw::c_int;
>  use std::io::Error;
>  use std::ffi::CString;
> +use evl_sys::SchedPolicy;
>  use evl_sys::{
>      evl_attach_thread,
>      evl_unblock_thread,
> @@ -29,6 +30,7 @@ pub struct Builder {
>      visible: bool,
>      observable: bool,
>      unicast: bool,
> +    sched: sched::SchedAttrs,

       sched: Option<SchedAttrs>,

I would rather use an option here, zero as a sched_policy currently
means SCHED_OTHER/NORMAL. Assuming zero is the 'unset' policy value
would cause this policy setting to be ignored by Thread::attach().

Granted, we are missing the definition of SchedOther as a unit type (and
its PolicyParam trait implementation) in sched.rs as well, so that we
may apply SCHED_OTHER to the caller (SCHED_OTHER is an alias to
(SCHED_WEAK,0) for the EVL core).

>  }
>    impl Builder {
> @@ -53,6 +55,7 @@ impl Builder {
>              visible: false,
>              observable: false,
>              unicast: false,
> +            sched: sched::get_zero_attrs(),

               sched: None,

>          }
>      }
>      /// Set the thread name. This name must conform to the [naming
> @@ -106,6 +109,11 @@ impl Builder {
>          self.unicast = true;
>          self
>      }
> +    // Set scheduling policy for the thread
> +    pub fn sched(mut self, policy: impl sched::PolicyParam) -> Self {
> +        self.sched = policy.to_attr();
> +        self
> +    }
>      /// Attach the calling thread to the EVL core, consuming the
>      /// builder.
>      ///
> @@ -257,11 +265,24 @@ impl Thread {
>  	        evl_attach_thread(c_flags, ptr::null())
>              }
>  	};
> +
>  	// evl_attach_thread() returns a valid file descriptor or -errno.
> -	match ret {
> -	    0.. => return Ok(Thread(ret)),
> -            _ => return Err(Error::from_raw_os_error(-ret)),
> +    let thread = Thread(ret);
> +
> +    if builder.sched.0.sched_policy as i32 != 0 {
> +        let c_attrs_ptr: *const evl_sched_attrs = &builder.sched.0;

Consequently, we could have something a bit more idiomatic instead:

       if let Some(attrs) = builder.sched {
               let c_attrs_ptr: *const evl_sched_attrs = &attrs.0;
               ...
       }

-- 
Philippe.