libthr: thr_attr.c: EINVAL, not ENOTSUP, on invalid arguments

[olce]

Summary

On first read, POSIX may seem ambiguous about the return code for some
scheduling-related pthread functions on invalid arguments. But a more
thorough reading and a bit of standards archeology strongly suggests
that this case should be handled by EINVAL and that ENOTSUP is reserved
for implementations providing only part of the functionality required by
the POSIX option POSIX_PRIORITY_SCHEDULING (e.g., if an implementation
doesn't support SCHED_FIFO, it should return ENOTSUP on a call to, e.g.,
sched_setscheduler() with 'policy' SCHED_FIFO).

This reading is supported by the second sentence of the very definition
of ENOTSUP, as worded in CAE/XSI Issue 5 and POSIX Issue 6: "The
implementation does not support this feature of the Realtime Feature
Group.", and the fact that an additional ENOTSUP case was added to
pthread_setschedparam() in Issue 6, which introduces SCHED_SPORADIC,
saying that pthread_setschedparam() may return it when attempting to
dynamically switch to SCHED_SPORADIC on systems that doesn't support
that.

glibc, illumos and NetBSD also support that reading by always returning
EINVAL, and OpenBSD as well, since it always returns EINVAL but the
corresponding code has a comment suggesting returning ENOTSUP for
SCHED_FIFO and SCHED_RR, which it effectively doesn't support.

Additionally, always returning EINVAL fixes inconsistencies where EINVAL
would be returned on some out-of-range values and ENOTSUP on others.

[markj]

I think I agree with the reasoning behind the change. Was this motivated by some existing software which checks the error number?

[kib]

Idea with ENOTSUP is extensibility. If tomorrow we add SCHED_FREAK scheduling class, existing library would still return ENOTSUP as it should be.

[olce]

In D43006#981667, @kib wrote:

Idea with ENOTSUP is extensibility. If tomorrow we add SCHED_FREAK scheduling class, existing library would still return ENOTSUP as it should be.

I don't think it should be so in the current framework (but this is going to change, see next paragraph). Either kernel and libc/libthr are updated in sync, in which case libthr's code must have been updated, or they are not, and then I don't really see why SCHED_FREAK implemented in the new kernel would be reported by the non-updated libthr as "I know this policy, but I don't implement it" (ENOTSUP) instead of "This policy is non-existent." (EINVAL). The converse situation is not really a problem since the non-updated kernel will simply reject a non-implemented policy at time of use with EINVAL (the time-of-use check is less desirable but still standard-compliant).

By current framework, I mean the current state of affairs where libthr implements itself a small part of the functionality, i.e., validation of inputs. In my cooking priority changes, this is not the case anymore, all scheduling functionality is implemented in the kernel, with just a policy-agnostic (well, for the currently defined ones) cache in libthr. So, if I correctly understood what you meant by extensibility, this discussion will soon become obsolete: libthr will report exactly what the current kernel it is running on reports. This may be convenient, e.g., in jails scenarios with older and newer userlands.

Additionally, what the standard means AFAIU is that ENOTSUP was reserved for values it published (that should exist on all compliant systems with option "Process Scheduling") that were not yet implemented in a particular implementation (well, there's a "new" use case for ENOTSUP concerning SCHED_SPORADIC and pthread_setschedparam(), but it's not relevant in this discussion). In other words, the implementation would not be completely compliant to the "Process Scheduling" option, but at least made the effort of declaring the scheduling classes. I think this is bogus, at least in theory: The presence of symbolic constants in an implementation should simply indicate support (and conversely, the absence of such absence of support). But, in practice, I'd say that most of the time application programmers use what is in the standard or in actual implementations such as Linux without paying attention to options. That's the probable reason for OpenBSD to define SCHED_FIFO and SCHED_RR, although it doesn't support them: Ensuring source compatibility. Which then means programs can get errors at runtime. In theory, they would get ENOTSUP in this case, but in practice they get EINVAL, since that's what OpenBSD actually returns in this case (but, as I mentioned in the herald description, it also has a comment suggesting to return ENOTSUP instead, which it should standard-wise). But that doesn't change the fact that running a new program, compiled with knowledge of SCHED_FREAK, on OS versions developed against old versions of the standard will rightfully get EINVAL.

So, to wrap up, I think the distinction between ENOTSUP and EINVAL is inoperant in practice and should not even have been standardized, because userland applications cannot infer anything useful from receiving one error instead of the other.

In D43006#981662, @markj wrote:

Was this motivated by some existing software which checks the error number?

No, simply because I noticed inconsistencies on checks that return EINVAL and ENOTSUP, so I digged the subject a bit, and found that:

1. Returning ENOTSUP nowadays doesn't seem to make sense at all.
2. No other implementation does that.
3. I've yet to find an application that distinguishes between both.

So let's just remove that too subtle distinction, which in the current state is anyway inconsistently implemented in libthr.

[markj]

this discussion will soon become obsolete: libthr will report exactly what the current kernel it is running on reports

This means that exactly these checks are going away soon anyway?

[olce]

In D43006#986890, @markj wrote:

this discussion will soon become obsolete: libthr will report exactly what the current kernel it is running on reports

This means that exactly these checks are going away soon anyway?

Mmm, not exactly. The "obsolete" is in the context of what I think kib@ was referring to by "extensibility" based on his example, i.e., the fact that libthr may have a different idea of what is supported than what the kernel can actually do (if they are out of sync, e.g., in jail scenarios). What is going to change is that the currently hardcoded checks (in libthr) for policy and sched_priority will become dynamic ones: libhtr will ask the kernel instead (and cache the results), so kib's example will no more be a case where ENOTSUP can be returned. So it's not the discussion that is rendered "obsolete" but rather the presented example once the choice is made (for the other reasons I presented), sorry that it wasn't clear.