From owner-svn-src-all@FreeBSD.ORG Sat Jan 24 18:53:52 2015 Return-Path: Delivered-To: svn-src-all@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 8993514; Sat, 24 Jan 2015 18:53:52 +0000 (UTC) Received: from smtp6.ore.mailhop.org (smtp6.ore.mailhop.org [54.149.35.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 669891B3; Sat, 24 Jan 2015 18:53:52 +0000 (UTC) Received: from [73.34.117.227] (helo=ilsoft.org) by smtp6.ore.mailhop.org with esmtpsa (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) (Exim 4.82) (envelope-from ) id 1YF5q5-0001Ea-Vp; Sat, 24 Jan 2015 18:53:46 +0000 Received: from revolution.hippie.lan (revolution.hippie.lan [172.22.42.240]) by ilsoft.org (8.14.9/8.14.9) with ESMTP id t0OIrhSx028822; Sat, 24 Jan 2015 11:53:43 -0700 (MST) (envelope-from ian@freebsd.org) X-Mail-Handler: DuoCircle Outbound SMTP X-Originating-IP: 73.34.117.227 X-Report-Abuse-To: abuse@duocircle.com (see https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information for abuse reporting information) X-MHO-User: U2FsdGVkX1/8g6knC+8vHA0Y0eqVwyNl Message-ID: <1422125623.1038.68.camel@freebsd.org> Subject: Re: svn commit: r277643 - in head/sys: arm/arm dev/mem i386/i386 mips/mips sparc64/sparc64 From: Ian Lepore To: Alan Cox Date: Sat, 24 Jan 2015 11:53:43 -0700 In-Reply-To: <54C3E563.4070903@rice.edu> References: <201501241251.t0OCpGa8053192@svn.freebsd.org> <1422111397.1038.53.camel@freebsd.org> <20150124154240.GV42409@kib.kiev.ua> <54C3E563.4070903@rice.edu> Content-Type: text/plain; charset="us-ascii" X-Mailer: Evolution 3.12.8 FreeBSD GNOME Team Port Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Cc: Konstantin Belousov , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 24 Jan 2015 18:53:52 -0000 On Sat, 2015-01-24 at 12:33 -0600, Alan Cox wrote: > On 01/24/2015 09:42, Konstantin Belousov wrote: > > On Sat, Jan 24, 2015 at 07:56:37AM -0700, Ian Lepore wrote: > >> On Sat, 2015-01-24 at 12:51 +0000, Konstantin Belousov wrote: > >>> Author: kib > >>> Date: Sat Jan 24 12:51:15 2015 > >>> New Revision: 277643 > >>> URL: https://svnweb.freebsd.org/changeset/base/277643 > >>> > >>> Log: > >>> Remove Giant from /dev/mem and /dev/kmem. It is definitely not needed > >>> for i386, and from the code inspection, nothing in the > >>> arm/mips/sparc64 implementations depends on it. > >>> > >> I'm not sure I agree with that. On arm the memrw() implementation uses > >> a single statically-allocated page of kva space into which it maps each > >> physical page in turn in the main loop. What prevents preemption or > >> multicore access to /dev/mem from trying to use that single page for > >> multiple operations at once? > > I see, thank you for noting this. > > > > But, I do not think that Giant is a solution for the problem. uiomove() > > call accesses userspace, which may fault and cause sleep. If the > > thread sleeps, the Giant is automatically dropped, so there is no real > > protection. > > > > I think dump exclusive sx around whole memrw() should be enough. > > > > I can revert the commit for now, or I can leave it as is while > > writing the patch with sx and waiting for somebody review. What > > would you prefer ? > > > > P.S. mips uses uiomove_fromphys(), avoiding transient mapping, > > and sparc allocates KVA when needed. > > > > > > While we're here, it's worth noting that the arm version of /dev/mem is > not functionally equivalent to that of amd64 or i386. Arm disallows > access to non-DRAM addresses through /dev/mem. That's true for the read/write interface, but not for mmap(). In fact, we have users insisting that mmap() on /dev/mem should provide userland access to memory-mapped devices, and we have ARM architecture rules that say you can't map the same physical address multiple times with different attributes, such as being Device memory in the kernel and Strongly Ordered when mapped into userland. But if the memory isn't mapped S-O for userland, they have no hope of usefully accessing the devices (because they don't have access to cache and buffer maintenance). Even "normal" memory has a variety of attributes that make the temporary mappings done in memrw() a bit iffy, although I'm not sure we're doing anything right now that could lead to trouble. Trouble lurks though if we ever start using some of the more subtle features of the arm memory architecture, such as turning off the sharable attribute on pages that are inherently per-cpu to avoid the overhead of hardware cache coherence when we know only one core can access the pages. -- Ian