From owner-freebsd-net@FreeBSD.ORG Thu May 23 21:05:43 2013 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 9C88576C; Thu, 23 May 2013 21:05:43 +0000 (UTC) (envelope-from guy.helmer@gmail.com) Received: from mail-ie0-x232.google.com (mail-ie0-x232.google.com [IPv6:2607:f8b0:4001:c03::232]) by mx1.freebsd.org (Postfix) with ESMTP id 687A511A; Thu, 23 May 2013 21:05:43 +0000 (UTC) Received: by mail-ie0-f178.google.com with SMTP id f4so2588920iea.23 for ; Thu, 23 May 2013 14:05:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to:x-mailer; bh=hE9PGv8YxtTJhSJ4p81wT8WNaGu7CrEIY0tLWpOPDRI=; b=Yhf95Vl7mHvvLwMbzjDSoDzMv3OJaalFJL7x8jvl34BmHog/fXD4sSE9RhOV5r6JeS GEgszuHq+WY2aGK4eFA+5DdtrvWABBqljol3PuuuTVjXoNmZVzq9KsEmqAHIH7r0rLBi 5SHvV2EEoG2L5mIqY/Q9vTh7LpZrOmzAKuHQctYmDtOKSlpXPwYcwKdp+KDLsM94os5m PpT4zsTIUO24MnoDg5eiHWXwoHuxAvfG9aOIsBOb1Aom41F5hLyeHf+BxxpeHjEq5TP8 DRCglwTYxf3oDbf7aYv+hf5QjU/FoO0VTCgRDpmnsiSWly7QuQrBSTs9+Jwb1FTVmFp1 Q6Gg== X-Received: by 10.42.27.146 with SMTP id j18mr11629524icc.54.1369343143074; Thu, 23 May 2013 14:05:43 -0700 (PDT) Received: from [192.168.1.107] (173-25-205-212.client.mchsi.com. [173.25.205.212]) by mx.google.com with ESMTPSA id j3sm13510959igv.4.2013.05.23.14.05.40 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 23 May 2013 14:05:41 -0700 (PDT) Content-Type: text/plain; charset=iso-8859-1 Mime-Version: 1.0 (Mac OS X Mail 6.3 \(1503\)) Subject: Re: bpf hold buffer in-use flag From: Guy Helmer In-Reply-To: <201301091535.04904.jhb@freebsd.org> Date: Thu, 23 May 2013 16:05:39 -0500 Content-Transfer-Encoding: quoted-printable Message-Id: <4EA47178-7CE2-40CE-A767-2689FAF7BEBD@gmail.com> References: <9C928117-2230-4F01-9B95-B6D945AF4416@gmail.com> <201301091535.04904.jhb@freebsd.org> To: John Baldwin X-Mailer: Apple Mail (2.1503) Cc: freebsd-net@freebsd.org X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.14 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, 23 May 2013 21:05:43 -0000 On Jan 9, 2013, at 2:35 PM, John Baldwin wrote: > On Tuesday, November 13, 2012 4:40:57 pm Guy Helmer wrote: >> To try to completely resolve the race in bpfread(), I have put = together=20 > these changes to add a flag to indicate when the hold buffer cannot be=20= > modified because it is in use. Since it's my first time using = mtx_sleep() and=20 > wakeup(), I wanted to run these past the list to see if I can get any = feedback=20 > on the approach. >>=20 >>=20 >> Index: bpf.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- bpf.c (revision 242997) >> +++ bpf.c (working copy) >> @@ -819,6 +819,7 @@ bpfopen(struct cdev *dev, int flags, int fmt, = stru >> * particular buffer method. >> */ >> bpf_buffer_init(d); >> + d->bd_hbuf_in_use =3D 0; >> d->bd_bufmode =3D BPF_BUFMODE_BUFFER; >> d->bd_sig =3D SIGIO; >> d->bd_direction =3D BPF_D_INOUT; >> @@ -872,6 +873,9 @@ bpfread(struct cdev *dev, struct uio *uio, int = iof >> callout_stop(&d->bd_callout); >> timed_out =3D (d->bd_state =3D=3D BPF_TIMED_OUT); >> d->bd_state =3D BPF_IDLE; >> + while (d->bd_hbuf_in_use) >> + mtx_sleep(&d->bd_hbuf_in_use, &d->bd_lock, >> + PRINET|PCATCH, "bd_hbuf", 0); >=20 > You need to check the return value here, otherwise the PCATCH is = useless (you=20 > will just go back to sleep instead of failing with an error if this is=20= > interrupted by a signal).=20 Thanks for the feedback (sorry it's taken so long to get to it). Would = this change correctly handle interruptions? Index: bpf.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- bpf.c (revision 250941) +++ bpf.c (working copy) @@ -856,9 +856,14 @@ callout_stop(&d->bd_callout); timed_out =3D (d->bd_state =3D=3D BPF_TIMED_OUT); d->bd_state =3D BPF_IDLE; - while (d->bd_hbuf_in_use) - mtx_sleep(&d->bd_hbuf_in_use, &d->bd_lock, + while (d->bd_hbuf_in_use) { + error =3D mtx_sleep(&d->bd_hbuf_in_use, &d->bd_lock, PRINET|PCATCH, "bd_hbuf", 0); + if (error =3D=3D EINTR || error =3D=3D ERESTART) { + BPFD_UNLOCK(d); + return (error); + } + } /* * If the hold buffer is empty, then do a timed sleep, which * ends when the timeout expires or when enough packets