Date: Mon, 3 Jun 2002 15:54:02 -0700 (PDT) From: Archie Cobbs <archie@dellroad.org> To: Julian Elischer <julian@elischer.org> Cc: Archie Cobbs <archie@dellroad.org>, Alfred Perlstein <bright@mu.org>, freebsd-net@freebsd.org Subject: Re: Race condition with M_EXT ref count? Message-ID: <200206032254.g53Ms2O48132@arch20m.dellroad.org> In-Reply-To: <Pine.BSF.4.21.0206031500400.43857-100000@InterJet.elischer.org> "from Julian Elischer at Jun 3, 2002 03:08:01 pm"
next in thread | previous in thread | raw e-mail | index | archive | help
Julian Elischer writes: > well, it's a valid problem form the point of view of some interrupt code > changing the ext reference at teh same time that some other code is > planning on incrementing it, but while Giant is in place it's not > likely to affect anything.. > > I presume you are talking about 4.x however right? Yes (see original email). > we can presume that each manipulator has a reference so it's not going to > GA AWAY due to a zero reference being reached, so the link can be followed > safely, which just elaves the atomicity of the addition operation. > > who are the contenders? > 1/ network mid-level code? (protected by: splnet and the BGL. > Only one cpu at a time.. > > 2/ Interrupt code running at splimp > probably freeing stuff after transmit. (receivers should have > pre-allocated buffers) > > it IS possible that there could need to be an splimp() added but I am > not clear on what part the 4.4 BGL (Big Giant Lock) plays in this.. > it may make it safe... 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. 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 { To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200206032254.g53Ms2O48132>