svn commit: r277643 - in head/sys: arm/arm dev/mem i386/i386 mips/mips sparc64/sparc64
Konstantin Belousov
kostikbel at gmail.com
Sat Jan 24 15:51:23 UTC 2015
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
> > >
> > > 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.
Like this.
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 <sys/mutex.h>
#include <sys/proc.h>
#include <sys/signalvar.h>
+#include <sys/sx.h>
#include <sys/systm.h>
#include <sys/uio.h>
@@ -72,6 +73,9 @@ MALLOC_DEFINE(M_MEMDESC, "memdesc", "memory range descriptors");
struct mem_range_softc mem_range_softc;
+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 = (int)uio->uio_offset & PAGE_MASK;
c = (u_int)(PAGE_SIZE - ((int)iov->iov_base & PAGE_MASK));
@@ -114,6 +119,7 @@ memrw(struct cdev *dev, struct uio *uio, int flags)
c = min(c, (u_int)iov->iov_len);
error = uiomove((caddr_t)&_tmppt[o], (int)c, uio);
pmap_qremove((vm_offset_t)_tmppt, 1);
+ sx_xunlock(&tmppt_lock);
continue;
}
else if (dev2unit(dev) == CDEV_MINOR_KMEM) {
More information about the svn-src-head
mailing list