Re: cv_fdrestart()

[Jason Thorpe <thorpej%me.com@localhost>]

> On Oct 14, 2023, at 6:38 AM, Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
> 
>> Date: Fri, 13 Oct 2023 13:41:50 -0700
>> From: Jason Thorpe <thorpej%me.com@localhost>
>> 
>>> On Oct 13, 2023, at 11:48 AM, Andrew Doran <ad%netbsd.org@localhost> wrote:
>>> 
>>> Add cv_fdrestart() (better name suggestions welcome):
>> 
>> Oooooof.
> 
> Why do we need this at all?

Indeed.

> Condition variable semantics is standard, well-studied, and
> well-understood.  This is a foundational API that essentially every
> kernel subsystem relies integrally on, and it strikes me as a mistake
> to tie it in with file descriptors or the ERESTART mechanism.
> 
> I would ask that a controversial change like this be reverted until we
> have had substantive discussion giving a compelling reason to change
> such a foundational API to add custom, nonstandard semantics wiring it
> up to file descriptors and our ERESTART mechanism.
> 
> We can just do
> 
>        foo->flags |= FOO_RESTART;
>        cv_broadcast(cv);
> 
> and then the corresponding wait logic can do
> 
>        if (foo->flags & FOO_RESTART)
>                return ERESTART;
>        cv_wait(cv);
>        goto retry;

See also eventfd_wait().

My main objection was “putting fd-specific restart logic in condvars”.  Of course it would be better to not pollute condvars at all.

-- thorpej