Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 19 Nov 2011 13:03:15 +0000 (UTC)
From:      Marius Strobl <marius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org
Subject:   svn commit: r227713 - in stable/8: share/man/man9 sys/dev/ofw sys/kern sys/powerpc/powerpc sys/sys
Message-ID:  <201111191303.pAJD3FZZ044075@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: marius
Date: Sat Nov 19 13:03:14 2011
New Revision: 227713
URL: http://svn.freebsd.org/changeset/base/227713

Log:
  MFC: r227537
  
  As it turns out, r186347 actually is insufficient to avoid the use of the
  curthread-accessing part of mtx_{,un}lock(9) when using a r210623-style
  curthread implementation on sparc64, crashing the kernel in its early
  cycles as PCPU isn't set up, yet (and can't be set up as OFW is one of the
  things we need for that, which leads to a chicken-and-egg problem). What
  happens is that due to the fact that the idea of r210623 actually is to
  allow the compiler to cache invocations of curthread, it factors out
  obtaining curthread needed for both mtx_lock(9) and mtx_unlock(9) to
  before the branch based on kobj_mutex_inited when compiling the kernel
  without the debugging options. So change kobj_class_compile_static(9)
  to just never acquire kobj_mtx, effectively restricting it to its
  documented use, and add a kobj_init_static(9) for initializing objects
  using a class compiled with the former and that also avoids using mutex(9)
  (and malloc(9)). Also assert in both of these functions that they are
  used in their intended way only.
  While at it, inline kobj_register_method() and kobj_unregister_method()
  as there wasn't much point for factoring them out in the first place
  and so that a reader of the code has to figure out the locking for
  fewer functions missing a KOBJ_ASSERT.
  Tested on powerpc{,64} by andreast.
  
  Reviewed by:	nwhitehorn (earlier version), jhb

Modified:
  stable/8/share/man/man9/Makefile
  stable/8/share/man/man9/kobj.9
  stable/8/sys/dev/ofw/openfirm.c
  stable/8/sys/kern/subr_kobj.c
  stable/8/sys/powerpc/powerpc/platform.c
  stable/8/sys/powerpc/powerpc/pmap_dispatch.c
  stable/8/sys/sys/kobj.h
Directory Properties:
  stable/8/share/man/man9/   (props changed)
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)

Modified: stable/8/share/man/man9/Makefile
==============================================================================
--- stable/8/share/man/man9/Makefile	Sat Nov 19 12:55:34 2011	(r227712)
+++ stable/8/share/man/man9/Makefile	Sat Nov 19 13:03:14 2011	(r227713)
@@ -724,7 +724,8 @@ MLINKS+=kobj.9 DEFINE_CLASS.9 \
 	kobj.9 kobj_class_free.9 \
 	kobj.9 kobj_create.9 \
 	kobj.9 kobj_delete.9 \
-	kobj.9 kobj_init.9
+	kobj.9 kobj_init.9 \
+	kobj.9 kobj_init_static.9
 MLINKS+=kproc.9 kproc_create.9 \
 	kproc.9 kthread_create.9 \
 	kproc.9 kproc_exit.9 \

Modified: stable/8/share/man/man9/kobj.9
==============================================================================
--- stable/8/share/man/man9/kobj.9	Sat Nov 19 12:55:34 2011	(r227712)
+++ stable/8/share/man/man9/kobj.9	Sat Nov 19 13:03:14 2011	(r227713)
@@ -28,7 +28,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd April 4, 2000
+.Dd November 14, 2011
 .Dt KOBJ 9
 .Os
 .Sh NAME
@@ -48,6 +48,8 @@
 .Ft void
 .Fn kobj_init "kobj_t obj" "kobj_class_t cls"
 .Ft void
+.Fn kobj_init_static "kobj_t obj" "kobj_class_t cls"
+.Ft void
 .Fn kobj_delete "kobj_t obj" "struct malloc_type *mtype"
 .Fn DEFINE_CLASS name "kobj_method_t *methods" "size_t size"
 .Sh DESCRIPTION
@@ -88,10 +90,14 @@ Objects created in this way should be fr
 Clients which would like to manage the allocation of memory
 themselves should call
 .Fn kobj_init
+or
+.Fn kobj_init_static
 with a pointer to the memory for the object and the class which
 implements it.
 It is also possible to use
 .Fn kobj_init
+and
+.Fn kobj_init_static
 to change the class for an object.
 This should be done with care as the classes must agree on the layout
 of the object.
@@ -109,13 +115,19 @@ A client should not normally need to cal
 will automatically be compiled the first time it is used.
 If a class is to be used before
 .Xr malloc 9
-is initialised,
+and
+.Xr mutex 9
+are initialised,
 then
 .Fn kobj_class_compile_static
 should be called with the class and a pointer to a statically
 allocated
 .Vt kobj_ops
 structure before the class is used to initialise any objects.
+In that case, also
+.Fn kobj_init_static
+should be used instead of
+.Fn kobj_init .
 .Pp
 To define a class, first define a simple array of
 .Vt kobj_method_t .

Modified: stable/8/sys/dev/ofw/openfirm.c
==============================================================================
--- stable/8/sys/dev/ofw/openfirm.c	Sat Nov 19 12:55:34 2011	(r227712)
+++ stable/8/sys/dev/ofw/openfirm.c	Sat Nov 19 13:03:14 2011	(r227713)
@@ -121,7 +121,7 @@ OF_init(void *cookie)
 	 * then statically initialize the OFW object.
 	 */
 	kobj_class_compile_static(ofw_def_impl, &ofw_kernel_kops);
-	kobj_init((kobj_t)ofw_obj, ofw_def_impl);
+	kobj_init_static((kobj_t)ofw_obj, ofw_def_impl);
 
 	OFW_INIT(ofw_obj, cookie);
 

Modified: stable/8/sys/kern/subr_kobj.c
==============================================================================
--- stable/8/sys/kern/subr_kobj.c	Sat Nov 19 12:55:34 2011	(r227712)
+++ stable/8/sys/kern/subr_kobj.c	Sat Nov 19 13:03:14 2011	(r227713)
@@ -60,18 +60,9 @@ static struct mtx kobj_mtx;
 static int kobj_mutex_inited;
 static int kobj_next_id = 1;
 
-/*
- * In the event that kobj_mtx has not been initialized yet,
- * we will ignore it, and run without locks in order to support
- * use of KOBJ before mutexes are available. This early in the boot
- * process, everything is single threaded and so races should not
- * happen. This is used to provide the PMAP layer on PowerPC, as well
- * as board support.
- */
-
-#define KOBJ_LOCK()	if (kobj_mutex_inited) mtx_lock(&kobj_mtx);
-#define KOBJ_UNLOCK()	if (kobj_mutex_inited) mtx_unlock(&kobj_mtx);
-#define KOBJ_ASSERT(what) if (kobj_mutex_inited) mtx_assert(&kobj_mtx,what);
+#define	KOBJ_LOCK()		mtx_lock(&kobj_mtx)
+#define	KOBJ_UNLOCK()		mtx_unlock(&kobj_mtx)
+#define	KOBJ_ASSERT(what)	mtx_assert(&kobj_mtx, what);
 
 SYSCTL_UINT(_kern, OID_AUTO, kobj_methodcount, CTLFLAG_RD,
 	   &kobj_next_id, 0, "");
@@ -104,28 +95,11 @@ kobj_error_method(void)
 }
 
 static void
-kobj_register_method(struct kobjop_desc *desc)
-{
-	KOBJ_ASSERT(MA_OWNED);
-
-	if (desc->id == 0) {
-		desc->id = kobj_next_id++;
-	}
-}
-
-static void
-kobj_unregister_method(struct kobjop_desc *desc)
-{
-}
-
-static void
 kobj_class_compile_common(kobj_class_t cls, kobj_ops_t ops)
 {
 	kobj_method_t *m;
 	int i;
 
-	KOBJ_ASSERT(MA_OWNED);
-
 	/*
 	 * Don't do anything if we are already compiled.
 	 */
@@ -135,8 +109,10 @@ kobj_class_compile_common(kobj_class_t c
 	/*
 	 * First register any methods which need it.
 	 */
-	for (i = 0, m = cls->methods; m->desc; i++, m++)
-		kobj_register_method(m->desc);
+	for (i = 0, m = cls->methods; m->desc; i++, m++) {
+		if (m->desc->id == 0)
+			m->desc->id = kobj_next_id++;
+	}
 
 	/*
 	 * Then initialise the ops table.
@@ -159,7 +135,7 @@ kobj_class_compile(kobj_class_t cls)
 	 */
 	ops = malloc(sizeof(struct kobj_ops), M_KOBJ, M_NOWAIT);
 	if (!ops)
-		panic("kobj_compile_methods: out of memory");
+		panic("%s: out of memory", __func__);
 
 	KOBJ_LOCK();
 	
@@ -182,17 +158,14 @@ void
 kobj_class_compile_static(kobj_class_t cls, kobj_ops_t ops)
 {
 
-	KOBJ_ASSERT(MA_NOTOWNED);
+	KASSERT(kobj_mutex_inited == 0,
+	    ("%s: only supported during early cycles", __func__));
 
 	/*
 	 * Increment refs to make sure that the ops table is not freed.
 	 */
-	KOBJ_LOCK();
-
 	cls->refs++;
 	kobj_class_compile_common(cls, ops);
-
-	KOBJ_UNLOCK();
 }
 
 static kobj_method_t*
@@ -259,8 +232,6 @@ kobj_lookup_method(kobj_class_t cls,
 void
 kobj_class_free(kobj_class_t cls)
 {
-	int i;
-	kobj_method_t *m;
 	void* ops = NULL;
 
 	KOBJ_ASSERT(MA_NOTOWNED);
@@ -272,10 +243,9 @@ kobj_class_free(kobj_class_t cls)
 	 */
 	if (cls->refs == 0) {
 		/*
-		 * Unregister any methods which are no longer used.
+		 * For now we don't do anything to unregister any methods
+		 * which are no longer used.
 		 */
-		for (i = 0, m = cls->methods; m->desc; i++, m++)
-			kobj_unregister_method(m->desc);
 
 		/*
 		 * Free memory and clean up.
@@ -308,6 +278,14 @@ kobj_create(kobj_class_t cls,
 	return obj;
 }
 
+static void
+kobj_init_common(kobj_t obj, kobj_class_t cls)
+{
+
+	obj->ops = cls->ops;
+	cls->refs++;
+}
+
 void
 kobj_init(kobj_t obj, kobj_class_t cls)
 {
@@ -329,13 +307,22 @@ kobj_init(kobj_t obj, kobj_class_t cls)
 		goto retry;
 	}
 
-	obj->ops = cls->ops;
-	cls->refs++;
+	kobj_init_common(obj, cls);
 
 	KOBJ_UNLOCK();
 }
 
 void
+kobj_init_static(kobj_t obj, kobj_class_t cls)
+{
+
+	KASSERT(kobj_mutex_inited == 0,
+	    ("%s: only supported during early cycles", __func__));
+
+	kobj_init_common(obj, cls);
+}
+
+void
 kobj_delete(kobj_t obj, struct malloc_type *mtype)
 {
 	kobj_class_t cls = obj->ops->cls;

Modified: stable/8/sys/powerpc/powerpc/platform.c
==============================================================================
--- stable/8/sys/powerpc/powerpc/platform.c	Sat Nov 19 12:55:34 2011	(r227712)
+++ stable/8/sys/powerpc/powerpc/platform.c	Sat Nov 19 13:03:14 2011	(r227713)
@@ -140,7 +140,7 @@ platform_probe_and_attach()
 		 * then statically initialise the MMU object
 		 */
 		kobj_class_compile_static(platp, &plat_kernel_kops);
-		kobj_init((kobj_t)plat_obj, platp);
+		kobj_init_static((kobj_t)plat_obj, platp);
 
 		prio = PLATFORM_PROBE(plat_obj);
 
@@ -178,7 +178,7 @@ platform_probe_and_attach()
 	 */
 
 	kobj_class_compile_static(plat_def_impl, &plat_kernel_kops);
-	kobj_init((kobj_t)plat_obj, plat_def_impl);
+	kobj_init_static((kobj_t)plat_obj, plat_def_impl);
 
 	strlcpy(plat_name,plat_def_impl->name,sizeof(plat_name));
 

Modified: stable/8/sys/powerpc/powerpc/pmap_dispatch.c
==============================================================================
--- stable/8/sys/powerpc/powerpc/pmap_dispatch.c	Sat Nov 19 12:55:34 2011	(r227712)
+++ stable/8/sys/powerpc/powerpc/pmap_dispatch.c	Sat Nov 19 13:03:14 2011	(r227713)
@@ -402,7 +402,7 @@ pmap_bootstrap(vm_offset_t start, vm_off
 	 * then statically initialise the MMU object
 	 */
 	kobj_class_compile_static(mmu_def_impl, &mmu_kernel_kops);
-	kobj_init((kobj_t)mmu_obj, mmu_def_impl);
+	kobj_init_static((kobj_t)mmu_obj, mmu_def_impl);
 
 	MMU_BOOTSTRAP(mmu_obj, start, end);
 }

Modified: stable/8/sys/sys/kobj.h
==============================================================================
--- stable/8/sys/sys/kobj.h	Sat Nov 19 12:55:34 2011	(r227712)
+++ stable/8/sys/sys/kobj.h	Sat Nov 19 13:03:14 2011	(r227713)
@@ -201,6 +201,7 @@ kobj_t		kobj_create(kobj_class_t cls,
  * Initialise a pre-allocated object.
  */
 void		kobj_init(kobj_t obj, kobj_class_t cls);
+void		kobj_init_static(kobj_t obj, kobj_class_t cls);
 
 /*
  * Delete an object. If mtype is non-zero, free the memory.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201111191303.pAJD3FZZ044075>