Date: Thu, 22 Jan 2015 11:12:10 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Warner Losh <imp@bsdimp.com> Cc: src-committers@FreeBSD.org, mips@freebsd.org, svn-src-all@FreeBSD.org, arm@freebsd.org, sparc64@freebsd.org, svn-src-head@FreeBSD.org Subject: Re: svn commit: r277421 - head/sys/powerpc/powerpc Message-ID: <20150122091210.GV42409@kib.kiev.ua> In-Reply-To: <81FEE3A9-C522-48A3-9292-893685C640AF@bsdimp.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Jan 21, 2015 at 06:04:57PM -0700, Warner Losh wrote: > > > On Jan 21, 2015, at 12:54 AM, Konstantin Belousov <kostikbel@gmail.com> 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) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150122091210.GV42409>