From owner-freebsd-net@FreeBSD.ORG  Mon Sep 21 14:05:23 2009
Return-Path: <owner-freebsd-net@FreeBSD.ORG>
Delivered-To: freebsd-net@FreeBSD.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id CCD651065672
	for <freebsd-net@FreeBSD.org>; Mon, 21 Sep 2009 14:05:23 +0000 (UTC)
	(envelope-from brde@optusnet.com.au)
Received: from mail02.syd.optusnet.com.au (mail02.syd.optusnet.com.au
	[211.29.132.183])
	by mx1.freebsd.org (Postfix) with ESMTP id 53B4F8FC17
	for <freebsd-net@FreeBSD.org>; Mon, 21 Sep 2009 14:05:23 +0000 (UTC)
Received: from c122-107-125-150.carlnfd1.nsw.optusnet.com.au
	(c122-107-125-150.carlnfd1.nsw.optusnet.com.au [122.107.125.150])
	by mail02.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id
	n8LE5Ev0013758
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO);
	Tue, 22 Sep 2009 00:05:15 +1000
Date: Tue, 22 Sep 2009 00:05:14 +1000 (EST)
From: Bruce Evans <brde@optusnet.com.au>
X-X-Sender: bde@delplex.bde.org
To: Andrew Brampton <brampton+freebsd-net@gmail.com>
In-Reply-To: <d41814900909210543p46894d83u6d814353ea1ee130@mail.gmail.com>
Message-ID: <20090921235604.U12163@delplex.bde.org>
References: <d41814900909210543p46894d83u6d814353ea1ee130@mail.gmail.com>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
Cc: freebsd-net@FreeBSD.org
Subject: Re: Is this a race in mbuf's refcounting?
X-BeenThere: freebsd-net@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Networking and TCP/IP with FreeBSD <freebsd-net.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-net>,
	<mailto:freebsd-net-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-net>
List-Post: <mailto:freebsd-net@freebsd.org>
List-Help: <mailto:freebsd-net-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-net>,
	<mailto:freebsd-net-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Mon, 21 Sep 2009 14:05:23 -0000

On Mon, 21 Sep 2009, Andrew Brampton wrote:

> I've been reading the FreeBSD source code to understand how mbufs are
> reference counted. However, there are a few bits of code that I'm
> wondering if they would fail under the exactly right timing. Take for
> example in uipc_mbuf.c:
>
> 286 static void
> 287 mb_dupcl(struct mbuf *n, struct mbuf *m)
> 288 {
> ...
> 293        if (*(m->m_ext.ref_cnt) == 1)
> 294                *(m->m_ext.ref_cnt) += 1;
> 295        else
> 296                atomic_add_int(m->m_ext.ref_cnt, 1);
> ...
> 305 }
>
> Now, the way I understand this code is, if ref_cnt is 1, then it is
> not shared. In that case non-atomically increment ref_cnt. However, if
> ref_cnt was something else, then it is shared so update the value in
> an atomic way. This seems valid, however what happens if two threads
> call mb_dupcl at the same time with a non-shared m. Could they both
> evaluate the if on line 293 at the same time, and then both
> non-atomically increment ref_cnt?
>
> If this could happen then we have a lost update and our reference
> counting is broken. I've also noticed that in other places similar
> optimisations are made to avoid the atomic operation.
>
> So is this a problem?

I don't see how it can work.

Also, if the count was 1, then it should become 2, but there is nothing to
flush the store to memory.  This seems to mainly enlarge the race window
for the previous problem.

Bruce