Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 17 Apr 2013 12:09:40 GMT
From:      ruffianeo <ruffianeo@googlemail.com>
To:        freebsd-gnats-submit@FreeBSD.org
Subject:   misc/177902: atomic.h - vital functions missing.
Message-ID:  <201304171209.r3HC9es9032621@red.freebsd.org>
Resent-Message-ID: <201304171210.r3HCA0rk032734@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help

>Number:         177902
>Category:       misc
>Synopsis:       atomic.h - vital functions missing.
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          change-request
>Submitter-Id:   current-users
>Arrival-Date:   Wed Apr 17 12:10:00 UTC 2013
>Closed-Date:
>Last-Modified:
>Originator:     ruffianeo
>Release:        FreeBSD 9.1
>Organization:
private
>Environment:
FreeBSD bsd001 9.1-RELEASE FreeBSD 9.1-RELEASE #0 r243826: Tue Dec  4 06:55:39 UTC 2012     root@obrian.cse.buffalo.edu:/usr/obj/usr/src/sys/GENERIC  i386
>Description:
Use case: Fast Reference counting with atomics.
On windows: InterlockedDecrement().
On other unix: atomic_sub_and_test().

With the current set of atomic functions in atomic.h for FreeBSD, reference counting cannot be realized.

As you can see for example in the NDIS port, this leads to subtle bugs (race conditions) while people try to realize this use case with the existing set of atomic functions:

http://fxr.watson.org/fxr/source/compat/ndis/subr_ntoskrnl.c?v=FREEBSD91#L202,2373

The example "workaround" bears a race condition. The return (*addend) races with other threads modifying the variable with other atomic functions. The return value is not always the result of the atomic operation in the line above.

Hence, I suggest to review atomic.h and to add missing functionality, such that it is fit for the use case "Reference counting".

One way could be to add the atomic_sub_and_test() but in my opinion, it is possible to find a better solution following the semantics of InterlockedDecrement(), which allows for a few more use cases than the atomic_sub_and_test() method.

>How-To-Repeat:
Inspect the flawed workaround in NDIS port (2 liner), notice the race condition.
Try to find a fix for this function with current set of atomic functions. In my opinion, there is no fix possible right now.

>Fix:
Various solutions possible:
1. currently atomic_subtract() returns void. If it returned the result of the operation, reference counting could be realized.
2. add atomic_sub_and_test() to the set of atomic functions in atomic.h
3. add a pair of functions with increment/decrement semantics following Interlocked API semantics of windows. 

>Release-Note:
>Audit-Trail:
>Unformatted:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201304171209.r3HC9es9032621>