From owner-freebsd-net@FreeBSD.ORG Thu Jul 8 12:03:26 2010 Return-Path: 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 A079F1065675; Thu, 8 Jul 2010 12:03:26 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 168118FC18; Thu, 8 Jul 2010 12:03:25 +0000 (UTC) Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id o68C3HQE033705 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 8 Jul 2010 15:03:17 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4) with ESMTP id o68C3Hns028080; Thu, 8 Jul 2010 15:03:17 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4/Submit) id o68C3HJt028079; Thu, 8 Jul 2010 15:03:17 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 8 Jul 2010 15:03:17 +0300 From: Kostik Belousov To: Andre Oppermann Message-ID: <20100708120317.GD2408@deviant.kiev.zoral.com.ua> References: <7C3D15DD6E8F464998CA1470D8A322F302BB9F72@ES02CO.wgti.net> <20100707205041.GO13238@deviant.kiev.zoral.com.ua> <4C358843.5000001@freebsd.org> <4C358A01.8080206@icyb.net.ua> <20100708082943.GB2439@deviant.kiev.zoral.com.ua> <4C358EE5.8070003@icyb.net.ua> <20100708094255.GA2408@deviant.kiev.zoral.com.ua> <4C35B7C4.40507@freebsd.org> <20100708114705.GC2408@deviant.kiev.zoral.com.ua> <4C35BD7A.8020408@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="BRE3mIcgqKzpedwo" Content-Disposition: inline In-Reply-To: <4C35BD7A.8020408@freebsd.org> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-2.3 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_50, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: freebsd-net@freebsd.org, Ming Fu , Andriy Gapon , bz@freebsd.org, rwatson@freebsd.org, lstewart@freebsd.org Subject: Re: kern/123095 kern/131602 sendfile X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Jul 2010 12:03:26 -0000 --BRE3mIcgqKzpedwo Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 08, 2010 at 01:58:50PM +0200, Andre Oppermann wrote: > On 08.07.2010 13:47, Kostik Belousov wrote: > >On Thu, Jul 08, 2010 at 01:34:28PM +0200, Andre Oppermann wrote: > >>On 08.07.2010 11:42, Kostik Belousov wrote: > >>>On Thu, Jul 08, 2010 at 11:40:05AM +0300, Andriy Gapon wrote: > >>>>on 08/07/2010 11:29 Kostik Belousov said the following: > >>>>>Right, the patch maps the page in sf buffer read-only (on i386 only). > >>>>>But note the parallel posting with m_cat() change. It is still not > >>>>>enough, > >>>>>and I am not set up for the real network testing ATM. > >>>> > >>>>Could you also try to experiment with mb_dupcl? > >>>>Namely transfer M_RDONLY from source mbuf. > >>> > >>>Right, it is it. > >>> > >>>Below is my current patch including debugging facilities that seems to > >>>work. > >>>Real changes that needed are in m_cat and mb_dupcl. > >>> > >>... > >>>diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c > >>>index f41eb03..1701ef2 100644 > >>>--- a/sys/kern/uipc_mbuf.c > >>>+++ b/sys/kern/uipc_mbuf.c > >>>@@ -301,7 +301,7 @@ mb_dupcl(struct mbuf *n, struct mbuf *m) > >>> n->m_ext.ext_size =3D m->m_ext.ext_size; > >>> n->m_ext.ref_cnt =3D m->m_ext.ref_cnt; > >>> n->m_ext.ext_type =3D m->m_ext.ext_type; > >>>- n->m_flags |=3D M_EXT; > >>>+ n->m_flags |=3D M_EXT | (M_RDONLY& m->m_flags); > >>> } > >> > >>Having the M_EXT flag always implies readonly and M_WRITABLE gets this > >>right. Not inheriting all the flags from the source seems questionable. > >>So IMHO this should be done here: > >> > >> n->m_flags |=3D (M_EXT | m->m_flags) > >> > >>> /* > >>>@@ -911,7 +911,8 @@ m_cat(struct mbuf *m, struct mbuf *n) > >>> m =3D m->m_next; > >>> while (n) { > >>> if (m->m_flags& M_EXT || > >>>- m->m_data + m->m_len + n->m_len>=3D&m->m_dat[MLEN]) { > >>>+ m->m_data + m->m_len + n->m_len>=3D&m->m_dat[MLEN] || > >>>+ !M_WRITABLE(m)) { > >> > >>Here you can fully replace the (m->m_flags& M_EXT) test with > >>M_WRITABLE(). The M_EXT test is included in it. > >> > >>> /* just join the two chains */ > >>> m->m_next =3D n; > >>> return; > > > >The patch is below. Works as well for me, thank you for the feedback. >=20 > You may want to run the change to m_dupcl() by rwatson just to be sure. > It should be the right thing to do but better have another mbuf expert > look at it. Added Robert to Cc: list, let's see. >=20 > Does this fix the sendfile corruption problem for all cases or just the > panic for the loopback case? I do not know. It fixes both panics with debugging patches applied and indeed prevents corruption, for my setup on loopback. I have to defer to external testing for !lo case. >=20 > >diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c > >index f41eb03..58567a4 100644 > >--- a/sys/kern/uipc_mbuf.c > >+++ b/sys/kern/uipc_mbuf.c > >@@ -301,7 +301,7 @@ mb_dupcl(struct mbuf *n, struct mbuf *m) > > n->m_ext.ext_size =3D m->m_ext.ext_size; > > n->m_ext.ref_cnt =3D m->m_ext.ref_cnt; > > n->m_ext.ext_type =3D m->m_ext.ext_type; > >- n->m_flags |=3D M_EXT; > >+ n->m_flags |=3D M_EXT | m->m_flags; > > } > > > > /* > >@@ -910,7 +910,7 @@ m_cat(struct mbuf *m, struct mbuf *n) > > while (m->m_next) > > m =3D m->m_next; > > while (n) { > >- if (m->m_flags& M_EXT || > >+ if (!M_WRITABLE(m) || > > m->m_data + m->m_len + n->m_len>=3D&m->m_dat[MLEN]) { > > /* just join the two chains */ > > m->m_next =3D n; >=20 > --=20 > Andre --BRE3mIcgqKzpedwo Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (FreeBSD) iEYEARECAAYFAkw1voUACgkQC3+MBN1Mb4jo7QCg2zdDNnOQdHGNfeV9gJfOKiGY r3MAnjJhTjbbHzuLHBvC7+k7cENXoCuI =rL2f -----END PGP SIGNATURE----- --BRE3mIcgqKzpedwo--