From owner-svn-src-all@FreeBSD.ORG Sun Jan 25 17:07:09 2015 Return-Path: Delivered-To: svn-src-all@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 BD3D855F for ; Sun, 25 Jan 2015 17:07:09 +0000 (UTC) Received: from mail-pd0-f174.google.com (mail-pd0-f174.google.com [209.85.192.174]) (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 8D1E0B68 for ; Sun, 25 Jan 2015 17:07:09 +0000 (UTC) Received: by mail-pd0-f174.google.com with SMTP id ft15so7567176pdb.5 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=eCxO9mrj0M0QSVVCMY85PYd9bw0BqWBM3lSQP7IK81zFa8nQxf0tfxBOqYeFjsb6SF L6QHZdNm+7IzWRtmEIHqtPOU09L38ThKVdtxD8xSdBg/MKW9OkbjXceuDrPY2g9If02/ Oe2D7TRQjW44kYAAHf6m5T/2a/Ai5A2cB1TA3uXEhk28T2VUtJ8ZzOuL5MK3vnWEXfM+ 9baGP50K1WEzPl496wQ4xrk5s2eSo/o+mC76FWdrncHr3rqONs0VXhOKPfPeHYeqSUl9 mGzmQFm2mWKXwTL0FRD14NXJoEZWNdv9mgOn07DOHMHeQQv3bts1TR1jx0ozaQby0kQO Il3A== X-Gm-Message-State: ALoCoQkhuQBgvUYaqMO09Vcd8d8n2Y5T0+8UmA06m4VLDeCPG6g+npvozedAkWikFn4TxnGQXuiV 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-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: 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