From owner-freebsd-net Mon Jun 3 16:39:40 2002 Delivered-To: freebsd-net@freebsd.org Received: from angelica.unixdaemons.com (angelica.unixdaemons.com [209.148.64.135]) by hub.freebsd.org (Postfix) with ESMTP id 71ECD37B400 for ; Mon, 3 Jun 2002 16:39:34 -0700 (PDT) Received: from angelica.unixdaemons.com (bmilekic@localhost.unixdaemons.com [127.0.0.1]) by angelica.unixdaemons.com (8.12.3/8.12.1) with ESMTP id g53NdMMR030519; Mon, 3 Jun 2002 19:39:23 -0400 (EDT) X-Authentication-Warning: angelica.unixdaemons.com: Host bmilekic@localhost.unixdaemons.com [127.0.0.1] claimed to be angelica.unixdaemons.com Received: (from bmilekic@localhost) by angelica.unixdaemons.com (8.12.3/8.12.1/Submit) id g53NdMrJ030516; Mon, 3 Jun 2002 19:39:22 -0400 (EDT) (envelope-from bmilekic) Date: Mon, 3 Jun 2002 19:39:22 -0400 From: Bosko Milekic To: Archie Cobbs Cc: Julian Elischer , Alfred Perlstein , freebsd-net@FreeBSD.ORG Subject: Re: Race condition with M_EXT ref count? Message-ID: <20020603193922.B29296@unixdaemons.com> References: <200206032254.g53Ms2O48132@arch20m.dellroad.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: <200206032254.g53Ms2O48132@arch20m.dellroad.org>; from archie@dellroad.org on Mon, Jun 03, 2002 at 03:54:02PM -0700 Sender: owner-freebsd-net@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org On Mon, Jun 03, 2002 at 03:54:02PM -0700, Archie Cobbs wrote: [...] > If you don't mind, I'll let you deal with the -current case, since > I'm not familiar enough with -current locking. Below is my proposed > patch for -stable. -current does not have this "problem." > It should be clear that in -stable at least, there can be a race > between an interrupt handler that free's a cluster mbuf and mid-level > code that is free'ing the same mbuf, for example. > > -Archie > > __________________________________________________________________________ > Archie Cobbs * Packet Design * http://www.packetdesign.com > > --- sys/kern/uipc_mbuf.c.orig Mon Jun 3 15:43:25 2002 > +++ sys/kern/uipc_mbuf.c Mon Jun 3 15:52:28 2002 > @@ -707,11 +707,16 @@ > n->m_len = min(len, m->m_len - off); > if (m->m_flags & M_EXT) { > n->m_data = m->m_data + off; > - if(!m->m_ext.ext_ref) > - mclrefcnt[mtocl(m->m_ext.ext_buf)]++; > - else > - (*(m->m_ext.ext_ref))(m->m_ext.ext_buf, > - m->m_ext.ext_size); > + if (m->m_ext.ext_ref == NULL) { > + atomic_add_char( > + &mclrefcnt[mtocl(m->m_ext.ext_buf)], 1); > + } else { > + int s = splimp(); > + > + (*m->m_ext.ext_ref)(m->m_ext.ext_buf, > + m->m_ext.ext_size); > + splx(s); > + } > n->m_ext = m->m_ext; > n->m_flags |= M_EXT; > } else > @@ -754,11 +759,15 @@ > n->m_len = m->m_len; > if (m->m_flags & M_EXT) { > n->m_data = m->m_data; > - if(!m->m_ext.ext_ref) > - mclrefcnt[mtocl(m->m_ext.ext_buf)]++; > - else > - (*(m->m_ext.ext_ref))(m->m_ext.ext_buf, > - m->m_ext.ext_size); > + if (m->m_ext.ext_ref == NULL) > + atomic_add_char(&mclrefcnt[mtocl(m->m_ext.ext_buf)], 1); > + else { > + int s = splimp(); > + > + (*m->m_ext.ext_ref)(m->m_ext.ext_buf, > + m->m_ext.ext_size); > + splx(s); > + } > n->m_ext = m->m_ext; > n->m_flags |= M_EXT; > } else { > @@ -777,11 +786,16 @@ > n->m_len = m->m_len; > if (m->m_flags & M_EXT) { > n->m_data = m->m_data; > - if(!m->m_ext.ext_ref) > - mclrefcnt[mtocl(m->m_ext.ext_buf)]++; > - else > - (*(m->m_ext.ext_ref))(m->m_ext.ext_buf, > - m->m_ext.ext_size); > + if (m->m_ext.ext_ref == NULL) { > + atomic_add_char( > + &mclrefcnt[mtocl(m->m_ext.ext_buf)], 1); > + } else { > + int s = splimp(); > + > + (*m->m_ext.ext_ref)(m->m_ext.ext_buf, > + m->m_ext.ext_size); > + splx(s); > + } > n->m_ext = m->m_ext; > n->m_flags |= M_EXT; > } else { > @@ -1125,11 +1139,15 @@ > if (m->m_flags & M_EXT) { > n->m_flags |= M_EXT; > n->m_ext = m->m_ext; > - if(!m->m_ext.ext_ref) > - mclrefcnt[mtocl(m->m_ext.ext_buf)]++; > - else > - (*(m->m_ext.ext_ref))(m->m_ext.ext_buf, > - m->m_ext.ext_size); > + if (m->m_ext.ext_ref == NULL) > + atomic_add_char(&mclrefcnt[mtocl(m->m_ext.ext_buf)], 1); > + else { > + int s = splimp(); > + > + (*m->m_ext.ext_ref)(m->m_ext.ext_buf, > + m->m_ext.ext_size); > + splx(s); > + } > m->m_ext.ext_size = 0; /* For Accounting XXXXXX danger */ > n->m_data = m->m_data + len; > } else { It would be better if the thing was in a macro to allow for consistent ref. count manipulation, like in -CURRENT. But whatever, I don't really care. -- Bosko Milekic bmilekic@unixdaemons.com bmilekic@FreeBSD.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message