PERFORCE change 103162 for review
John Baldwin
jhb at freebsd.org
Fri Aug 4 16:01:49 UTC 2006
On Friday 04 August 2006 08:38, Roman Divacky wrote:
> http://perforce.freebsd.org/chv.cgi?CH=103162
>
> Change 103162 by rdivacky at rdivacky_witten on 2006/08/04 12:38:26
>
> Switch to using atomic_add_int() for refcounting.
>
>
> Affected files ...
>
> .. //depot/projects/soc2006/rdivacky_linuxolator/compat/linux/linux_futex.c#13
edit
>
> Differences ...
>
>
==== //depot/projects/soc2006/rdivacky_linuxolator/compat/linux/linux_futex.c#13
(text+ko) ====
>
> @@ -274,7 +274,7 @@
> FUTEX_LOCK;
> LIST_FOREACH(f, &futex_list, f_list) {
> if (f->f_uaddr == uaddr) {
> - f->f_refcount++;
> + atomic_add_int(&f->f_refcount, 1);
> FUTEX_UNLOCK;
> return f;
> }
> @@ -284,7 +284,7 @@
> /* Not found, create it */
> f = malloc(sizeof(*f), M_LINUX, M_WAITOK);
> f->f_uaddr = uaddr;
> - f->f_refcount = 1;
> + atomic_set_int(&f->f_refcount, 1);
> TAILQ_INIT(&f->f_waiting_proc);
> FUTEX_LOCK;
> LIST_INSERT_HEAD(&futex_list, f, f_list);
> @@ -297,7 +297,7 @@
> futex_put(f)
> struct futex *f;
> {
> - f->f_refcount--;
> + atomic_add_int(&f->f_refcount, -1);
> if (f->f_refcount == 0) {
> FUTEX_LOCK;
> LIST_REMOVE(f, f_list);
This is racy. Another thread can obtain a reference after you do the 0 check
and you will free out from under it. If you wanted to use atomic ops, you
should use the refcount API in sys/refcount.h instead. However, that would
still be racy in this case because of the LIST of these structures. Instead,
you need to revert this and just hold the FUTEX_LOCK earlier in futex_put()
before you do the f->f_refcount-- so that you can remove it from the list
safely without other threads gaining a reference to it.
--
John Baldwin
More information about the p4-projects
mailing list