From owner-svn-src-head@FreeBSD.ORG Sun Jan 25 17:07:09 2015 Return-Path: Delivered-To: svn-src-head@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 BDFD7560 for ; Sun, 25 Jan 2015 17:07:09 +0000 (UTC) Received: from mail-pd0-f173.google.com (mail-pd0-f173.google.com [209.85.192.173]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 8D13CB66 for ; Sun, 25 Jan 2015 17:07:09 +0000 (UTC) Received: by mail-pd0-f173.google.com with SMTP id fp1so7572666pdb.4 for ; Sun, 25 Jan 2015 09:07:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:content-type:mime-version:subject:from :in-reply-to:date:cc:content-transfer-encoding:message-id:references :to; bh=IepLzZydiR0lY4SEuSkbriQzNLohb7vHeB1PYRWs/Ng=; b=QBNcyi2sxnBflQUar6qKqPiUaYbJl5NG9z+edJ5oIdbD/voIy+gRvT8DB6mnBEnwtx B3QPVHZk6qE8jy7wQdMox5z9UR5yDMG3OwPf/Jvg84q+FeMbETo5TWrPNYynIXxFy8O3 3Z/SpdVR00IE3F2MFsnDPGMegGohiT++jpI52iGZY5SI9dnqRBeSb4IickUkXVi8V9Bx nedCXYJnGBlW5sxPOrNf1xZjNGKmakUIdvucbvh4iW0zXC+82JXV0gsgRQypqhHvSuSH aqpvCg2ATZwdy9YLBszykufoE7WsbUeNUq8Vd/9HryRH/mdSWTTEAv5C/7wQIDtmuvUs s0Hg== X-Gm-Message-State: ALoCoQn0ASDPYBpjvoSN8a/YvJNEIjc3+6wjtnSpHrI8vvP8QVPJqXymz0i2z7v/dzNeFLk6fnok X-Received: by 10.68.226.69 with SMTP id rq5mr28369299pbc.116.1422205623212; Sun, 25 Jan 2015 09:07:03 -0800 (PST) Received: from netflix-mac.bsdimp.com ([50.253.99.174]) by mx.google.com with ESMTPSA id u14sm7616416pbs.85.2015.01.25.09.07.02 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sun, 25 Jan 2015 09:07:02 -0800 (PST) Sender: Warner Losh Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 8.1 \(1993\)) Subject: Re: svn commit: r277643 - in head/sys: arm/arm dev/mem i386/i386 mips/mips sparc64/sparc64 From: Warner Losh In-Reply-To: <20150124155117.GW42409@kib.kiev.ua> Date: Sun, 25 Jan 2015 10:07:00 -0700 Content-Transfer-Encoding: quoted-printable Message-Id: References: <201501241251.t0OCpGa8053192@svn.freebsd.org> <1422111397.1038.53.camel@freebsd.org> <20150124154240.GV42409@kib.kiev.ua> <20150124155117.GW42409@kib.kiev.ua> To: Konstantin Belousov X-Mailer: Apple Mail (2.1993) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers , Ian Lepore X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 25 Jan 2015 17:07:09 -0000 > On Jan 24, 2015, at 8:51 AM, Konstantin Belousov = wrote: >=20 > On Sat, Jan 24, 2015 at 05:42:40PM +0200, 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 >>>>=20 >>>> 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. >>>>=20 >>>=20 >>> 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? >>=20 >> I see, thank you for noting this. >>=20 >> 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. >>=20 >> I think dump exclusive sx around whole memrw() should be enough. >>=20 >> 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 ? >>=20 >> P.S. mips uses uiomove_fromphys(), avoiding transient mapping, >> and sparc allocates KVA when needed. >=20 > Like this. So why a sx lock and not a mutex? Warner > diff --git a/sys/arm/arm/mem.c b/sys/arm/arm/mem.c > index 30d4b1d..58b0d25 100644 > --- a/sys/arm/arm/mem.c > +++ b/sys/arm/arm/mem.c > @@ -55,6 +55,7 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > +#include > #include > #include >=20 > @@ -72,6 +73,9 @@ MALLOC_DEFINE(M_MEMDESC, "memdesc", "memory range = descriptors"); >=20 > struct mem_range_softc mem_range_softc; >=20 > +static struct sx tmppt_lock; > +SX_SYSINIT(tmppt, &tmppt_lock, "mem4map"); > + > /* ARGSUSED */ > int > memrw(struct cdev *dev, struct uio *uio, int flags) > @@ -107,6 +111,7 @@ memrw(struct cdev *dev, struct uio *uio, int = flags) > } > if (!address_valid) > return (EINVAL); > + sx_xlock(&tmppt_lock); > pmap_kenter((vm_offset_t)_tmppt, v); > o =3D (int)uio->uio_offset & PAGE_MASK; > c =3D (u_int)(PAGE_SIZE - ((int)iov->iov_base & = PAGE_MASK)); > @@ -114,6 +119,7 @@ memrw(struct cdev *dev, struct uio *uio, int = flags) > c =3D min(c, (u_int)iov->iov_len); > error =3D uiomove((caddr_t)&_tmppt[o], (int)c, = uio); > pmap_qremove((vm_offset_t)_tmppt, 1); > + sx_xunlock(&tmppt_lock); > continue; > } > else if (dev2unit(dev) =3D=3D CDEV_MINOR_KMEM) { >=20