Received-SPF: pass (google.com: domain of linux-kernel+bounces-72446-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-153-g7e3bb84806-fm-20240215.007-g7e3bb848
Precedence: bulk
MIME-Version: 1.0
Message-Id: <830b0788-35e6-4cbd-b195-254d434ba0cd@app.fastmail.com>
In-Reply-To: <20240219223833.95710-18-zfigura@codeweavers.com>
References: <20240219223833.95710-1-zfigura@codeweavers.com>
 <20240219223833.95710-18-zfigura@codeweavers.com>
Date: Tue, 20 Feb 2024 08:01:59 +0100
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Elizabeth Figura" <zfigura@codeweavers.com>,
 "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
 "Jonathan Corbet" <corbet@lwn.net>, shuah <shuah@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
 wine-devel@winehq.org,
 =?UTF-8?Q?Andr=C3=A9_Almeida?= <andrealmeid@igalia.com>,
 "Wolfram Sang" <wsa@kernel.org>, "Arkadiusz Hiler" <ahiler@codeweavers.com>,
 "Peter Zijlstra" <peterz@infradead.org>, "Andy Lutomirski" <luto@kernel.org>,
 linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org,
 "Randy Dunlap" <rdunlap@infradead.org>
Subject: Re: [PATCH v2 17/31] ntsync: Allow waits to use the REALTIME clock.
Content-Type: text/plain

On Mon, Feb 19, 2024, at 23:38, Elizabeth Figura wrote:
> NtWaitForMultipleObjects() can receive a timeout in two forms, relative or
> absolute. Relative timeouts are unaffected by changes to the system time and do
> not count down while the system suspends; for absolute timeouts the opposite is
> true.
>
> In order to make the interface and implementation simpler, the ntsync driver
> only deals in absolute timeouts. However, we need to be able to emulate both
> behaviours apropos suspension and time adjustment, which is achieved by allowing
> either the MONOTONIC or REALTIME clock to be used.
>
> Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>

I understand that there is no practical problem in building
up the API one patch at a time in the initial merge, but
it still feels wrong to have an incompatible ABI change in
the middle of the series:

> @@ -35,6 +37,8 @@ struct ntsync_wait_args {
>  	__u32 owner;
>  	__u32 index;
>  	__u32 alert;
> +	__u32 flags;
> +	__u32 pad;
>  };

If this was patch to get merged at any later point, you'd have
to support both the shorter and the longer structure layout
with their distinct ioctl command codes.

If you do a v3 series, maybe just merge this patch into the
one that introduces the struct ntsync_wait_args. Overall,
you could probably have fewer but larger patches anyway
without harming the review process, but other than this
one that is not a problem.

       Arnd