From owner-freebsd-mips@FreeBSD.ORG Thu Jan 22 09:12:17 2015 Return-Path: Delivered-To: mips@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 116A54D8; Thu, 22 Jan 2015 09:12:17 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 8C03E214; Thu, 22 Jan 2015 09:12:16 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id t0M9CAfv070722 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 22 Jan 2015 11:12:10 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua t0M9CAfv070722 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id t0M9CAjh070721; Thu, 22 Jan 2015 11:12:10 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 22 Jan 2015 11:12:10 +0200 From: Konstantin Belousov To: Warner Losh Subject: Re: svn commit: r277421 - head/sys/powerpc/powerpc Message-ID: <20150122091210.GV42409@kib.kiev.ua> References: <201501201621.t0KGLx26096724@svn.freebsd.org> <20150120191438.GL42409@kib.kiev.ua> <54BF240C.2090107@freebsd.org> <20150121075410.GM42409@kib.kiev.ua> <81FEE3A9-C522-48A3-9292-893685C640AF@bsdimp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <81FEE3A9-C522-48A3-9292-893685C640AF@bsdimp.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: src-committers@FreeBSD.org, mips@freebsd.org, svn-src-all@FreeBSD.org, arm@freebsd.org, Nathan Whitehorn , sparc64@freebsd.org, svn-src-head@FreeBSD.org X-BeenThere: freebsd-mips@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Porting FreeBSD to MIPS List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Jan 2015 09:12:17 -0000 On Wed, Jan 21, 2015 at 06:04:57PM -0700, Warner Losh wrote: > > > On Jan 21, 2015, at 12:54 AM, Konstantin Belousov wrote: > > > > On Tue, Jan 20, 2015 at 07:59:08PM -0800, Nathan Whitehorn wrote: > >> > >> On 01/20/15 11:14, Konstantin Belousov wrote: > >>> On Tue, Jan 20, 2015 at 04:21:59PM +0000, Nathan Whitehorn wrote: > >>>> Author: nwhitehorn > >>>> Date: Tue Jan 20 16:21:59 2015 > >>>> New Revision: 277421 > >>>> URL: https://svnweb.freebsd.org/changeset/base/277421 > >>>> > >>>> Log: > >>>> There does not seem to be any reason to acquire GIANT here. Follow amd64 > >>>> in removing it. > >>>> > >>>> MFC after: 1 month > >>>> > >>>> Modified: > >>>> head/sys/powerpc/powerpc/mem.c > >>>> > >>>> Modified: head/sys/powerpc/powerpc/mem.c > >>>> ============================================================================== > >>>> --- head/sys/powerpc/powerpc/mem.c Tue Jan 20 15:45:09 2015 (r277420) > >>>> +++ head/sys/powerpc/powerpc/mem.c Tue Jan 20 16:21:59 2015 (r277421) > >>>> @@ -100,8 +100,6 @@ memrw(struct cdev *dev, struct uio *uio, > >>>> cnt = 0; > >>>> error = 0; > >>>> > >>>> - GIANT_REQUIRED; > >>>> - > >>> This is not an acquisition, to be pedantic. Really, it is cdevsw which > >>> has D_NEEDGIANT flag which acquires Giant. After architectures get > >>> rid of GIANT_REQUIRED, flag can be removed. > >>> > >> Just so I understand, you are not objecting to this commit, right? > > Absolutely not, this is the right thing to do. > > > >> Just > >> pointing out that (a) my commit message was wrong and that (b) once all > >> architectures make this change (presumably more involved) we can get rid > >> of the D_NEEDGIANT in /sys/dev/mem/memdev.c? > > Exactly. > > There doesn???t seem to be a reason for i386 either. Was just looking > at the code today on the plane for unrelated reasons. > I agree. For i386, there is definitely no reason to require Giant. I looked over arm+mips+sparc64, and I do not see a reason. Note that I do not know/understand their pmap implementations, but I expect that Giant is not needed due to properties of the used (pmap) functions. Below is the trivial patch. Would be nice to get at least a nod from some arch maintainers. diff --git a/sys/arm/arm/mem.c b/sys/arm/arm/mem.c index 460a004..30d4b1d 100644 --- a/sys/arm/arm/mem.c +++ b/sys/arm/arm/mem.c @@ -82,8 +82,6 @@ memrw(struct cdev *dev, struct uio *uio, int flags) int error = 0; vm_offset_t addr, eaddr; - GIANT_REQUIRED; - while (uio->uio_resid > 0 && error == 0) { iov = uio->uio_iov; if (iov->iov_len == 0) { diff --git a/sys/dev/mem/memdev.c b/sys/dev/mem/memdev.c index 37bad15..c3d1af1 100644 --- a/sys/dev/mem/memdev.c +++ b/sys/dev/mem/memdev.c @@ -52,7 +52,7 @@ static struct cdev *memdev, *kmemdev; static struct cdevsw mem_cdevsw = { .d_version = D_VERSION, - .d_flags = D_MEM|D_NEEDGIANT, + .d_flags = D_MEM, .d_open = memopen, .d_read = memrw, .d_write = memrw, diff --git a/sys/i386/i386/mem.c b/sys/i386/i386/mem.c index 9c83f47..b036bd3 100644 --- a/sys/i386/i386/mem.c +++ b/sys/i386/i386/mem.c @@ -86,10 +86,6 @@ memrw(struct cdev *dev, struct uio *uio, int flags) int error = 0; vm_offset_t addr; - /* XXX UPS Why ? */ - GIANT_REQUIRED; - - if (dev2unit(dev) != CDEV_MINOR_MEM && dev2unit(dev) != CDEV_MINOR_KMEM) return EIO; diff --git a/sys/mips/mips/mem.c b/sys/mips/mips/mem.c index d40c424..08bb6b0 100644 --- a/sys/mips/mips/mem.c +++ b/sys/mips/mips/mem.c @@ -85,8 +85,6 @@ memrw(struct cdev *dev, struct uio *uio, int flags) cnt = 0; error = 0; - GIANT_REQUIRED; - pmap_page_init(&m); while (uio->uio_resid > 0 && !error) { iov = uio->uio_iov; diff --git a/sys/sparc64/sparc64/mem.c b/sys/sparc64/sparc64/mem.c index d09f6b8..6bd5225 100644 --- a/sys/sparc64/sparc64/mem.c +++ b/sys/sparc64/sparc64/mem.c @@ -99,8 +99,6 @@ memrw(struct cdev *dev, struct uio *uio, int flags) error = 0; ova = 0; - GIANT_REQUIRED; - while (uio->uio_resid > 0 && error == 0) { iov = uio->uio_iov; if (iov->iov_len == 0) {