[Bug 100077] libdrm atomic_add_unless() may reverse return value meaning

[Bug 100077] libdrm atomic_add_unless() may reverse return value meaning

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 1560 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=100077

            Bug ID: 100077
           Summary: libdrm atomic_add_unless() may reverse return value
                    meaning
           Product: DRI
           Version: unspecified
          Hardware: x86-64 (AMD64)
                OS: BSD (Others)
            Status: NEW
          Severity: normal
          Priority: medium
         Component: libdrm
          Assignee: dri-devel@lists.freedesktop.org
          Reporter: davshao@gmail.com

atomic_add_unless() in libdrm xf86atomic.h may reverse the meaning
of its return value.

Linux kernel documentation seems to indicate something like:
"Returns non-zero if @v was not @u, and zero otherwise."

A simple inverting the meaning of libdrm's return value
allowed glxgears to properly function for a hacked version
of pkgsrc on DragonFly 4.7-DEVELOPMENT on an Intel IvyBridge
integrated graphics machine.  glxgears was already properly
functioning on the same machine for NetBSD current, NetBSD
using its own atomic operations declared in its sys/atomic.h
header.

A one line (character) fix is of the form:

--- xf86atomic.h.orig   2015-09-22 04:34:51.000000000 +0000
+++ xf86atomic.h
@@ -111,7 +111,7 @@ static inline int atomic_add_unless(atom
        c = atomic_read(v);
        while (c != unless && (old = atomic_cmpxchg(v, c, c + add)) != c)
                c = old;
-       return c == unless;
+       return c != unless;
 }

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 2888 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH libdrm] atomic: fix atomic_add_unless() fallback's return value

[Eric Engestrom]

According to the kernel documentation:
  Returns non-zero if @v was not @u, and zero otherwise.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100077
Fixes: 63fc571863aa64683400 ("atomic: add atomic_add_unless()")
Signed-off-by: David Shao <davshao@gmail.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
Signed-off-by: Eric Engestrom <eric@engestrom.ch>
---
 xf86atomic.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xf86atomic.h b/xf86atomic.h
index 922b37da..ecb1ba86 100644
--- a/xf86atomic.h
+++ b/xf86atomic.h
@@ -111,7 +111,7 @@ static inline int atomic_add_unless(atomic_t *v, int add, int unless)
 	c = atomic_read(v);
 	while (c != unless && (old = atomic_cmpxchg(v, c, c + add)) != c)
 		c = old;
-	return c == unless;
+	return c != unless;
 }
 
 #endif
-- 
Cheers,
  Eric

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm] atomic: fix atomic_add_unless() fallback's return value

[Eric Engestrom]

On Thursday, 2017-03-16 00:08:59 +0000, Eric Engestrom wrote:
> According to the kernel documentation:
>   Returns non-zero if @v was not @u, and zero otherwise.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100077
> Fixes: 63fc571863aa64683400 ("atomic: add atomic_add_unless()")
> Signed-off-by: David Shao <davshao@gmail.com>
> Reviewed-by: Eric Engestrom <eric@engestrom.ch>
> Signed-off-by: Eric Engestrom <eric@engestrom.ch>

That last s-o-b was added automatically, by mistake. I'll remove it when
pushing this.

This patch seems trivial to verify, can someone confirm this is right?

/me is about ~94% sure, but doesn't know much about atomic ops and their
return value convention.

I checked the kernel and it does what libdrm does after this patch, so
either one should be fixed; I'm assuming kernel is right and libdrm
needs fixing, hence this patch.

Cheers,
  Eric

> ---
>  xf86atomic.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xf86atomic.h b/xf86atomic.h
> index 922b37da..ecb1ba86 100644
> --- a/xf86atomic.h
> +++ b/xf86atomic.h
> @@ -111,7 +111,7 @@ static inline int atomic_add_unless(atomic_t *v, int add, int unless)
>  	c = atomic_read(v);
>  	while (c != unless && (old = atomic_cmpxchg(v, c, c + add)) != c)
>  		c = old;
> -	return c == unless;
> +	return c != unless;
>  }
>  
>  #endif
> -- 
> Cheers,
>   Eric
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm] atomic: fix atomic_add_unless() fallback's return value

[Emil Velikov]

[+ Lionel]

On 16 March 2017 at 16:46, Eric Engestrom <eric.engestrom@imgtec.com> wrote:
> On Thursday, 2017-03-16 00:08:59 +0000, Eric Engestrom wrote:
>> According to the kernel documentation:
>>   Returns non-zero if @v was not @u, and zero otherwise.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100077
>> Fixes: 63fc571863aa64683400 ("atomic: add atomic_add_unless()")
>> Signed-off-by: David Shao <davshao@gmail.com>
>> Reviewed-by: Eric Engestrom <eric@engestrom.ch>
>> Signed-off-by: Eric Engestrom <eric@engestrom.ch>
>
> That last s-o-b was added automatically, by mistake. I'll remove it when
> pushing this.
>
> This patch seems trivial to verify, can someone confirm this is right?
>
> /me is about ~94% sure, but doesn't know much about atomic ops and their
> return value convention.
>
> I checked the kernel and it does what libdrm does after this patch, so
> either one should be fixed; I'm assuming kernel is right and libdrm
> needs fixing, hence this patch.
>
Changing the function matches the documentation, yet it doesn't [seem
to] align with how it's used.
I'm fairly sure that this will break libdrm_intel.

Lionel, can you take a look at this patch please ?
https://patchwork.freedesktop.org/patch/144252/

Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 100077] libdrm atomic_add_unless() may reverse return value meaning

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 839 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=100077

GitLab Migration User <gitlab-migration@fdo.invalid> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |MOVED

--- Comment #1 from GitLab Migration User <gitlab-migration@fdo.invalid> ---
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been
closed from further activity.

You can subscribe and participate further through the new bug through this link
to our GitLab instance: https://gitlab.freedesktop.org/mesa/drm/issues/17.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 2469 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel