Re: VFS: C99 sparse format for struct vfsops

On Wednesday 04 June 2003 1:26 pm, Paul Richards wrote:
> On Wed, Jun 04, 2003 at 12:09:00PM +0100, Doug Rabson wrote:
> > On Mon, 2003-06-02 at 21:04, Paul Richards wrote:
> > > On Tue, 2003-06-03 at 18:19, M. Warner Losh wrote:
> > > > Notice how thread 1's _m gets set based on the results of the
> > > > kobj lookup, and we have a race, even if thread1 and thread2
> > > > took out their driver instance locks.
> > >
> > > That means we have to lock the dispatch table before every method
> > > is looked up and hold it until the method returns (the
> > > alternative would be to free it inside the method once it had
> > > been called but that'd be a right mess).
> >
> > Don't even think about trying to put a mutex into the kobj dispatch
>
> I wasn't, I was just pointing out what would be necessary if the
> cacheing problem wasn't resolved :-)

I think this patch should fix the SMP-unsafe problems with kobj, at the 
expense of changing the ABI slightly.

Index: tools/makeobjops.awk
===================================================================
RCS file: /home/ncvs/src/sys/tools/makeobjops.awk,v
retrieving revision 1.2
diff -u -r1.2 makeobjops.awk
--- tools/makeobjops.awk	20 Aug 2002 03:06:30 -0000	1.2
+++ tools/makeobjops.awk	9 Jun 2003 09:25:30 -0000
_at__at_ -283,7 +283,7 _at__at_
 	firstvar = varnames[1];
 
 	if (default == "")
-		default = "0";
+		default = "kobj_error_method";
 
 	# the method description 
 	printh("extern struct kobjop_desc " mname "_desc;");
_at__at_ -293,8 +293,12 _at__at_
 	    line_width, length(prototype)));
 
 	# Print out the method desc
+	printc("struct kobj_method " mname "_method_default = {");
+	printc("\t&" mname "_desc, (kobjop_t) " default);
+	printc("};\n");
+
 	printc("struct kobjop_desc " mname "_desc = {");
-	printc("\t0, (kobjop_t) " default);
+	printc("\t0, &" mname "_method_default");
 	printc("};\n");
 
 	# Print out the method itself
Index: sys/kobj.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/kobj.h,v
retrieving revision 1.7
diff -u -r1.7 kobj.h
--- sys/kobj.h	10 Jun 2002 22:40:26 -0000	1.7
+++ sys/kobj.h	9 Jun 2003 09:26:14 -0000
_at__at_ -79,13 +79,13 _at__at_
 #define KOBJ_CACHE_SIZE	256
 
 struct kobj_ops {
-	kobj_method_t	cache[KOBJ_CACHE_SIZE];
+	kobj_method_t	*cache[KOBJ_CACHE_SIZE];
 	kobj_class_t	cls;
 };
 
 struct kobjop_desc {
 	unsigned int	id;	/* unique ID */
-	kobjop_t	deflt;	/* default implementation */
+	kobj_method_t	*deflt;	/* default implementation */
 };
 
 /*
_at__at_ -151,19 +151,27 _at__at_
  */
 #define KOBJOPLOOKUP(OPS,OP) do {					\
 	kobjop_desc_t _desc = &OP##_##desc;				\
-	kobj_method_t *_ce =						\
+	kobj_method_t **_cep =						\
 	    &OPS->cache[_desc->id & (KOBJ_CACHE_SIZE-1)];		\
+	kobj_method_t *_ce = *_cep;					\
 	if (_ce->desc != _desc) {					\
 		KOBJOPMISS;						\
-		kobj_lookup_method(OPS->cls->methods, _ce, _desc);	\
+		_ce = kobj_lookup_method(OPS->cls->methods,		\
+					 _cep, _desc);			\
 	} else {							\
 		KOBJOPHIT;						\
 	}								\
 	_m = _ce->func;							\
 } while(0)
 
-void kobj_lookup_method(kobj_method_t *methods,
-			kobj_method_t *ce,
-			kobjop_desc_t desc);
+kobj_method_t* kobj_lookup_method(kobj_method_t *methods,
+				  kobj_method_t **cep,
+				  kobjop_desc_t desc);
+
+
+/*
+ * Default method implementation. Returns ENXIO.
+ */
+int kobj_error_method(void);
 
 #endif /* !_SYS_KOBJ_H_ */
Index: kern/subr_kobj.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/subr_kobj.c,v
retrieving revision 1.5
diff -u -r1.5 subr_kobj.c
--- kern/subr_kobj.c	10 Jun 2002 22:40:26 -0000	1.5
+++ kern/subr_kobj.c	9 Jun 2003 09:27:14 -0000
_at__at_ -59,7 +59,7 _at__at_
 
 static int kobj_next_id = 1;
 
-static int
+int
 kobj_error_method(void)
 {
 	return ENXIO;
_at__at_ -127,23 +127,21 _at__at_
 	kobj_class_compile_common(cls, ops);
 }
 
-void
+kobj_method_t*
 kobj_lookup_method(kobj_method_t *methods,
-		   kobj_method_t *ce,
+		   kobj_method_t **cep,
 		   kobjop_desc_t desc)
 {
-	ce->desc = desc;
-	for (; methods && methods->desc; methods++) {
-		if (methods->desc == desc) {
-			ce->func = methods->func;
-			return;
+	kobj_method_t *ce;
+	for (ce = methods; ce && ce->desc; ce++) {
+		if (ce->desc == desc) {
+			*cep = ce;
+			return ce;
 		}
 	}
-	if (desc->deflt)
-		ce->func = desc->deflt;
-	else
-		ce->func = kobj_error_method;
-	return;
+	ce = desc->deflt;
+	*cep = ce;
+	return ce;
 }
 
 void



-- 
Doug Rabson				Mail:  dfr_at_nlsystems.com
					Phone: +44 20 8348 6160