Date: Mon, 9 Jun 2003 10:30:08 +0100 From: Doug Rabson <dfr@nlsystems.com> To: Paul Richards <paul@freebsd-services.com> Cc: "M. Warner Losh" <imp@bsdimp.com> Subject: Re: VFS: C99 sparse format for struct vfsops Message-ID: <200306091030.08293.dfr@nlsystems.com> In-Reply-To: <20030604122602.GC68108@survey.codeburst.net> References: <20030603134717.GD35187@survey.codeburst.net> <1054724940.32568.6.camel@builder02.qubesoft.com> <20030604122602.GC68108@survey.codeburst.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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
@@ -283,7 +283,7 @@
firstvar = varnames[1];
if (default == "")
- default = "0";
+ default = "kobj_error_method";
# the method description
printh("extern struct kobjop_desc " mname "_desc;");
@@ -293,8 +293,12 @@
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
@@ -79,13 +79,13 @@
#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 */
};
/*
@@ -151,19 +151,27 @@
*/
#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
@@ -59,7 +59,7 @@
static int kobj_next_id = 1;
-static int
+int
kobj_error_method(void)
{
return ENXIO;
@@ -127,23 +127,21 @@
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@nlsystems.com
Phone: +44 20 8348 6160
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200306091030.08293.dfr>