From owner-freebsd-current@FreeBSD.ORG Fri Oct 3 14:04:17 2014 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 03B36AB4; Fri, 3 Oct 2014 14:04:17 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id CDE5E116; Fri, 3 Oct 2014 14:04:16 +0000 (UTC) Received: from ralph.baldwin.cx (pool-173-70-85-31.nwrknj.fios.verizon.net [173.70.85.31]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 04BC4B984; Fri, 3 Oct 2014 10:04:15 -0400 (EDT) From: John Baldwin To: Gleb Smirnoff Subject: Re: [PATCH] Fix OACTIVE for an(4) Date: Fri, 03 Oct 2014 08:58:25 -0400 Message-ID: <16070615.NdPaubu6kG@ralph.baldwin.cx> User-Agent: KMail/4.12.5 (FreeBSD/10.1-BETA2; KDE/4.12.5; amd64; ; ) In-Reply-To: <20141003081328.GZ73266@FreeBSD.org> References: <2113392.UOaBFTpimf@ralph.baldwin.cx> <201410021116.27583.jhb@freebsd.org> <20141003081328.GZ73266@FreeBSD.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Fri, 03 Oct 2014 10:04:15 -0400 (EDT) Cc: Adrian Chadd , freebsd-current X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Oct 2014 14:04:17 -0000 On Friday, October 03, 2014 12:13:28 PM Gleb Smirnoff wrote: > On Thu, Oct 02, 2014 at 11:16:27AM -0400, John Baldwin wrote: > J> > I haven't looked at the rest of the driver; is everything else around > J> > OACTIVE locked correctly and consistently? > J> > J> As well as OACTIVE is for any other driver. > > Let me jump into this topic and discuss the if_drv_flags :) > > It seems to me that this in general was a wrong concept, both > IFF_DRV_OACTIVE and IFF_DRV_RUNNING. The internal state of > the driver can be known only to the driver itself and should > be stored in the softc, covered with internal lock. > > There is simply no way to racelessly tell the state from the > outside, without obtaining driver lock. > > In my ifnet plans I am considering to remove if_drv_flags. > Can anyone convince me that this is a wrong idea and they > should be kept? They used to be in if_flags. Robert moved them to if_drv_flags precisely so that drivers could control their synchronization instead of doing a crazy locking dance with the ifnet layer. They are still exported to userland as IFF_RUNNING and IFF_OACTIVE in if_flags, and they may still be somewhat useful for reporting that state to userland (and races in reading if_drv_flags for that reporting are harmless). That said, I think long term if we make the stack aware of multiple input queues we will certainly use something that does not have the same raciness as OACTIVE. Note, btw, you could fix OACTIVE in various drivers by simply grabbing the IF_LOCK on ifp->if_snd while setting or clearing OACTIVE to close the current OACTIVE race (you don't need it for all writes to if_drv_flags, you just want if_handoff() to read the current value of OACTIVE, so only locking writes to OACTIVE would be sufficient). -- John Baldwin