On Sat, Jun 18, 2022 at 03:59:08PM +0300, Dmitry Kozlyuk wrote:
> 2022-06-14 16:47 (UTC-0700), Tyler Retzlaff:
> 
> > +int
> > +rte_thread_create(rte_thread_t *thread_id,
> > +           const rte_thread_attr_t *thread_attr,
> > +           rte_thread_func thread_func, void *args)
> > +{
> > [...]
> > +           if (thread_attr->priority ==
> > +                           RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
> > +                   ret = ENOTSUP;
> > +                   goto cleanup;
> > +           }
> > +           ret = thread_map_priority_to_os_value(thread_attr->priority,
> > +                           &param.sched_priority, &policy);
> > +           if (ret != 0)
> > +                   goto cleanup;
> 
> thread_map_priority_to_os_value() already checks for unsupported values,
> why not let it do this particular check?

okay, so i looked at this more closely.

* thread_map_priority_to_os_value() just does mapping and
  RTE_THREAD_PRIORITY_REALTIME_CRITICAL is a valid mapping so it does not
  fail. by design it does one thing and one thing only perform the value
  mapping. admittedly it does not map every valid value right now, maybe it
  should?

  for consistency in the behavior of the function i'd suggest we don't
  make it have special cases in case we later decide to expand for
  additional valid mappings.

* rte_thread_set_priority() does fail with ENOTSUP on non-windows if
  provided RTE_THREAD_PRIORITY_REALTIME_CRITICAL however it cannot be
  used at this point since the thread has not been created.

if no further follow up on this i'm going to leave it as is.

thanks

Reply via email to