[PATCH 0/4] ntsync: some small fixes for doc and selftests

[PATCH 0/4] ntsync: some small fixes for doc and selftests

[Su Hui]

There are four small fixes for ntsync test and doc. I divided these into
four different patches due to different types of errors. If one patch is
better, I can do it too.

Su Hui (4):
  selftests: ntsync: fix the wrong condition in wake_all
  selftests: ntsync: avoid possible overflow in 32-bit machine
  selftests: ntsync: update config
  docs: ntsync: update NTSYNC_IOC_*

 Documentation/userspace-api/ntsync.rst         | 18 +++++++++---------
 tools/testing/selftests/drivers/ntsync/config  |  2 +-
 .../testing/selftests/drivers/ntsync/ntsync.c  |  6 +++---
 3 files changed, 13 insertions(+), 13 deletions(-)

-- 
2.30.2

[PATCH 1/4] selftests: ntsync: fix the wrong condition in wake_all

[Su Hui]

When  'manual=false' and  'signaled=true', then expected value when using
NTSYNC_IOC_CREATE_EVENT should be greater than zero. Fix this typo error.

Signed-off-by: Su Hui <suhui@nfschina.com>
---
 tools/testing/selftests/drivers/ntsync/ntsync.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 3aad311574c4..bfb6fad653d0 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -968,7 +968,7 @@ TEST(wake_all)
 	auto_event_args.manual = false;
 	auto_event_args.signaled = true;
 	objs[3] = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &auto_event_args);
-	EXPECT_EQ(0, objs[3]);
+	EXPECT_LE(0, objs[3]);
 
 	wait_args.timeout = get_abs_timeout(1000);
 	wait_args.objs = (uintptr_t)objs;
-- 
2.30.2

[PATCH 2/4] selftests: ntsync: avoid possible overflow in 32-bit machine

[Su Hui]

When using '-m32' flag  to compile this test with gcc, there are some
errors when running test:

 ntsync.c:785:wake_any:Expected ETIMEDOUT (110) == ret (0)
 ntsync.c:823:wake_any:Expected (1) (1) == __count (0)
 ...
 FAILED: 7 / 11 tests passed.
 Totals: pass:7 fail:4 xfail:0 xpass:0 skip:0 error:0

There is an overflow about 'timeout'. 'timespec->tv_sec' is 4 bytes in
32-bit machine. And 'timeout.tv_sec * 1000000000' causing the overflow
problem, adding a cast to avoid this problem.

Signed-off-by: Su Hui <suhui@nfschina.com>
---
 tools/testing/selftests/drivers/ntsync/ntsync.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index bfb6fad653d0..ded83dc58e6b 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -103,7 +103,7 @@ static int wait_objs(int fd, unsigned long request, __u32 count,
 
 	clock_gettime(CLOCK_MONOTONIC, &timeout);
 
-	args.timeout = timeout.tv_sec * 1000000000 + timeout.tv_nsec;
+	args.timeout = (__u64)timeout.tv_sec * 1000000000 + timeout.tv_nsec;
 	args.count = count;
 	args.objs = (uintptr_t)objs;
 	args.owner = owner;
@@ -729,7 +729,7 @@ static __u64 get_abs_timeout(unsigned int ms)
 {
 	struct timespec timeout;
 	clock_gettime(CLOCK_MONOTONIC, &timeout);
-	return (timeout.tv_sec * 1000000000) + timeout.tv_nsec + (ms * 1000000);
+	return ((__u64)timeout.tv_sec * 1000000000) + timeout.tv_nsec + (ms * 1000000);
 }
 
 static int wait_for_thread(pthread_t thread, unsigned int ms)
-- 
2.30.2

[PATCH 3/4] selftests: ntsync: update config

[Su Hui]

ntsync should be tested when CONFIG_NTSYNC is setting rather than
CONFIG_WINESYNC, correct this.

Signed-off-by: Su Hui <suhui@nfschina.com>
---
 tools/testing/selftests/drivers/ntsync/config | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/drivers/ntsync/config b/tools/testing/selftests/drivers/ntsync/config
index 60539c826d06..0aa68de147af 100644
--- a/tools/testing/selftests/drivers/ntsync/config
+++ b/tools/testing/selftests/drivers/ntsync/config
@@ -1 +1 @@
-CONFIG_WINESYNC=y
+CONFIG_NTSYNC=y
-- 
2.30.2

[PATCH 4/4] docs: ntsync: update NTSYNC_IOC_*

[Su Hui]

Some macros in the document do not match the code and the header files,
correct these macros according to the definitions in the code.

Signed-off-by: Su Hui <suhui@nfschina.com>
---
 Documentation/userspace-api/ntsync.rst | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/userspace-api/ntsync.rst b/Documentation/userspace-api/ntsync.rst
index 25e7c4aef968..969774bf4c60 100644
--- a/Documentation/userspace-api/ntsync.rst
+++ b/Documentation/userspace-api/ntsync.rst
@@ -152,9 +152,9 @@ The ioctls on the device file are as follows:
 
 The ioctls on the individual objects are as follows:
 
-.. c:macro:: NTSYNC_IOC_SEM_POST
+.. c:macro:: NTSYNC_IOC_SEM_RELEASE
 
-  Post to a semaphore object. Takes a pointer to a 32-bit integer,
+  Release a semaphore object. Takes a pointer to a 32-bit integer,
   which on input holds the count to be added to the semaphore, and on
   output contains its previous count.
 
@@ -186,7 +186,7 @@ The ioctls on the individual objects are as follows:
   unowned and signaled, and eligible threads waiting on it will be
   woken as appropriate.
 
-.. c:macro:: NTSYNC_IOC_SET_EVENT
+.. c:macro:: NTSYNC_IOC_EVENT_SET
 
   Signal an event object. Takes a pointer to a 32-bit integer, which on
   output contains the previous state of the event.
@@ -194,12 +194,12 @@ The ioctls on the individual objects are as follows:
   Eligible threads will be woken, and auto-reset events will be
   designaled appropriately.
 
-.. c:macro:: NTSYNC_IOC_RESET_EVENT
+.. c:macro:: NTSYNC_IOC_EVENT_RESET
 
   Designal an event object. Takes a pointer to a 32-bit integer, which
   on output contains the previous state of the event.
 
-.. c:macro:: NTSYNC_IOC_PULSE_EVENT
+.. c:macro:: NTSYNC_IOC_EVENT_PULSE
 
   Wake threads waiting on an event object while leaving it in an
   unsignaled state. Takes a pointer to a 32-bit integer, which on
@@ -213,7 +213,7 @@ The ioctls on the individual objects are as follows:
   afterwards, and a simultaneous read operation will always report the
   event as unsignaled.
 
-.. c:macro:: NTSYNC_IOC_READ_SEM
+.. c:macro:: NTSYNC_IOC_SEM_READ
 
   Read the current state of a semaphore object. Takes a pointer to
   struct :c:type:`ntsync_sem_args`, which is used as follows:
@@ -225,7 +225,7 @@ The ioctls on the individual objects are as follows:
      * - ``max``
        - On output, contains the maximum count of the semaphore.
 
-.. c:macro:: NTSYNC_IOC_READ_MUTEX
+.. c:macro:: NTSYNC_IOC_MUTEX_READ
 
   Read the current state of a mutex object. Takes a pointer to struct
   :c:type:`ntsync_mutex_args`, which is used as follows:
@@ -242,7 +242,7 @@ The ioctls on the individual objects are as follows:
   ``EOWNERDEAD``. In this case, ``count`` and ``owner`` are set to
   zero.
 
-.. c:macro:: NTSYNC_IOC_READ_EVENT
+.. c:macro:: NTSYNC_IOC_EVENT_READ
 
   Read the current state of an event object. Takes a pointer to struct
   :c:type:`ntsync_event_args`, which is used as follows:
@@ -255,7 +255,7 @@ The ioctls on the individual objects are as follows:
        - On output, contains 1 if the event is a manual-reset event,
          and 0 otherwise.
 
-.. c:macro:: NTSYNC_IOC_KILL_OWNER
+.. c:macro:: NTSYNC_IOC_MUTEX_KILL
 
   Mark a mutex as unowned and abandoned if it is owned by the given
   owner. Takes an input-only pointer to a 32-bit integer denoting the
-- 
2.30.2

Re: [PATCH 1/4] selftests: ntsync: fix the wrong condition in wake_all

[Dan Carpenter]

On Fri, Mar 14, 2025 at 03:14:51PM +0800, Su Hui wrote:
> When  'manual=false' and  'signaled=true', then expected value when using
> NTSYNC_IOC_CREATE_EVENT should be greater than zero. Fix this typo error.
> 
> Signed-off-by: Su Hui <suhui@nfschina.com>
> ---
>  tools/testing/selftests/drivers/ntsync/ntsync.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
> index 3aad311574c4..bfb6fad653d0 100644
> --- a/tools/testing/selftests/drivers/ntsync/ntsync.c
> +++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
> @@ -968,7 +968,7 @@ TEST(wake_all)
>  	auto_event_args.manual = false;
>  	auto_event_args.signaled = true;
>  	objs[3] = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &auto_event_args);
> -	EXPECT_EQ(0, objs[3]);
> +	EXPECT_LE(0, objs[3]);

It's kind of weird how these macros put the constant on the left.
It returns an "fd" on success.  So this look reasonable.  It probably
won't return the zero fd so we could probably check EXPECT_LT()?

regards,
dan carpenter

Re: [PATCH 1/4] selftests: ntsync: fix the wrong condition in wake_all

[Elizabeth Figura]

On Friday, 14 March 2025 02:14:51 CDT Su Hui wrote:
> When  'manual=false' and  'signaled=true', then expected value when using
> NTSYNC_IOC_CREATE_EVENT should be greater than zero. Fix this typo error.
> 
> Signed-off-by: Su Hui <suhui@nfschina.com>
> ---
>  tools/testing/selftests/drivers/ntsync/ntsync.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
> index 3aad311574c4..bfb6fad653d0 100644
> --- a/tools/testing/selftests/drivers/ntsync/ntsync.c
> +++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
> @@ -968,7 +968,7 @@ TEST(wake_all)
>  	auto_event_args.manual = false;
>  	auto_event_args.signaled = true;
>  	objs[3] = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &auto_event_args);
> -	EXPECT_EQ(0, objs[3]);
> +	EXPECT_LE(0, objs[3]);
>  
>  	wait_args.timeout = get_abs_timeout(1000);
>  	wait_args.objs = (uintptr_t)objs;
> 

Reviewed-by: Elizabeth Figura <zfigura@codeweavers.com>

Re: [PATCH 2/4] selftests: ntsync: avoid possible overflow in 32-bit machine

[Elizabeth Figura]

On Friday, 14 March 2025 02:14:52 CDT Su Hui wrote:
> When using '-m32' flag  to compile this test with gcc, there are some
> errors when running test:
> 
>  ntsync.c:785:wake_any:Expected ETIMEDOUT (110) == ret (0)
>  ntsync.c:823:wake_any:Expected (1) (1) == __count (0)
>  ...
>  FAILED: 7 / 11 tests passed.
>  Totals: pass:7 fail:4 xfail:0 xpass:0 skip:0 error:0
> 
> There is an overflow about 'timeout'. 'timespec->tv_sec' is 4 bytes in
> 32-bit machine. And 'timeout.tv_sec * 1000000000' causing the overflow
> problem, adding a cast to avoid this problem.
> 
> Signed-off-by: Su Hui <suhui@nfschina.com>
> ---
>  tools/testing/selftests/drivers/ntsync/ntsync.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
> index bfb6fad653d0..ded83dc58e6b 100644
> --- a/tools/testing/selftests/drivers/ntsync/ntsync.c
> +++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
> @@ -103,7 +103,7 @@ static int wait_objs(int fd, unsigned long request, __u32 count,
>  
>  	clock_gettime(CLOCK_MONOTONIC, &timeout);
>  
> -	args.timeout = timeout.tv_sec * 1000000000 + timeout.tv_nsec;
> +	args.timeout = (__u64)timeout.tv_sec * 1000000000 + timeout.tv_nsec;
>  	args.count = count;
>  	args.objs = (uintptr_t)objs;
>  	args.owner = owner;
> @@ -729,7 +729,7 @@ static __u64 get_abs_timeout(unsigned int ms)
>  {
>  	struct timespec timeout;
>  	clock_gettime(CLOCK_MONOTONIC, &timeout);
> -	return (timeout.tv_sec * 1000000000) + timeout.tv_nsec + (ms * 1000000);
> +	return ((__u64)timeout.tv_sec * 1000000000) + timeout.tv_nsec + (ms * 1000000);
>  }
>  
>  static int wait_for_thread(pthread_t thread, unsigned int ms)
> 

Reviewed-by: Elizabeth Figura <zfigura@codeweavers.com>

Re: [PATCH 3/4] selftests: ntsync: update config

[Elizabeth Figura]

On Friday, 14 March 2025 02:14:53 CDT Su Hui wrote:
> ntsync should be tested when CONFIG_NTSYNC is setting rather than
> CONFIG_WINESYNC, correct this.
> 
> Signed-off-by: Su Hui <suhui@nfschina.com>
> ---
>  tools/testing/selftests/drivers/ntsync/config | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/drivers/ntsync/config b/tools/testing/selftests/drivers/ntsync/config
> index 60539c826d06..0aa68de147af 100644
> --- a/tools/testing/selftests/drivers/ntsync/config
> +++ b/tools/testing/selftests/drivers/ntsync/config
> @@ -1 +1 @@
> -CONFIG_WINESYNC=y
> +CONFIG_NTSYNC=y
> 

Reviewed-by: Elizabeth Figura <zfigura@codeweavers.com>

Re: [PATCH 4/4] docs: ntsync: update NTSYNC_IOC_*

[Elizabeth Figura]

On Friday, 14 March 2025 02:14:54 CDT Su Hui wrote:
> Some macros in the document do not match the code and the header files,
> correct these macros according to the definitions in the code.
> 
> Signed-off-by: Su Hui <suhui@nfschina.com>
> ---
>  Documentation/userspace-api/ntsync.rst | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/userspace-api/ntsync.rst b/Documentation/userspace-api/ntsync.rst
> index 25e7c4aef968..969774bf4c60 100644
> --- a/Documentation/userspace-api/ntsync.rst
> +++ b/Documentation/userspace-api/ntsync.rst
> @@ -152,9 +152,9 @@ The ioctls on the device file are as follows:
>  
>  The ioctls on the individual objects are as follows:
>  
> -.. c:macro:: NTSYNC_IOC_SEM_POST
> +.. c:macro:: NTSYNC_IOC_SEM_RELEASE
>  
> -  Post to a semaphore object. Takes a pointer to a 32-bit integer,
> +  Release a semaphore object. Takes a pointer to a 32-bit integer,
>    which on input holds the count to be added to the semaphore, and on
>    output contains its previous count.
>  
> @@ -186,7 +186,7 @@ The ioctls on the individual objects are as follows:
>    unowned and signaled, and eligible threads waiting on it will be
>    woken as appropriate.
>  
> -.. c:macro:: NTSYNC_IOC_SET_EVENT
> +.. c:macro:: NTSYNC_IOC_EVENT_SET
>  
>    Signal an event object. Takes a pointer to a 32-bit integer, which on
>    output contains the previous state of the event.
> @@ -194,12 +194,12 @@ The ioctls on the individual objects are as follows:
>    Eligible threads will be woken, and auto-reset events will be
>    designaled appropriately.
>  
> -.. c:macro:: NTSYNC_IOC_RESET_EVENT
> +.. c:macro:: NTSYNC_IOC_EVENT_RESET
>  
>    Designal an event object. Takes a pointer to a 32-bit integer, which
>    on output contains the previous state of the event.
>  
> -.. c:macro:: NTSYNC_IOC_PULSE_EVENT
> +.. c:macro:: NTSYNC_IOC_EVENT_PULSE
>  
>    Wake threads waiting on an event object while leaving it in an
>    unsignaled state. Takes a pointer to a 32-bit integer, which on
> @@ -213,7 +213,7 @@ The ioctls on the individual objects are as follows:
>    afterwards, and a simultaneous read operation will always report the
>    event as unsignaled.
>  
> -.. c:macro:: NTSYNC_IOC_READ_SEM
> +.. c:macro:: NTSYNC_IOC_SEM_READ
>  
>    Read the current state of a semaphore object. Takes a pointer to
>    struct :c:type:`ntsync_sem_args`, which is used as follows:
> @@ -225,7 +225,7 @@ The ioctls on the individual objects are as follows:
>       * - ``max``
>         - On output, contains the maximum count of the semaphore.
>  
> -.. c:macro:: NTSYNC_IOC_READ_MUTEX
> +.. c:macro:: NTSYNC_IOC_MUTEX_READ
>  
>    Read the current state of a mutex object. Takes a pointer to struct
>    :c:type:`ntsync_mutex_args`, which is used as follows:
> @@ -242,7 +242,7 @@ The ioctls on the individual objects are as follows:
>    ``EOWNERDEAD``. In this case, ``count`` and ``owner`` are set to
>    zero.
>  
> -.. c:macro:: NTSYNC_IOC_READ_EVENT
> +.. c:macro:: NTSYNC_IOC_EVENT_READ
>  
>    Read the current state of an event object. Takes a pointer to struct
>    :c:type:`ntsync_event_args`, which is used as follows:
> @@ -255,7 +255,7 @@ The ioctls on the individual objects are as follows:
>         - On output, contains 1 if the event is a manual-reset event,
>           and 0 otherwise.
>  
> -.. c:macro:: NTSYNC_IOC_KILL_OWNER
> +.. c:macro:: NTSYNC_IOC_MUTEX_KILL
>  
>    Mark a mutex as unowned and abandoned if it is owned by the given
>    owner. Takes an input-only pointer to a 32-bit integer denoting the
> 

Reviewed-by: Elizabeth Figura <zfigura@codeweavers.com>