From owner-freebsd-current@FreeBSD.ORG Mon Jun 9 02:30:22 2003 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 429A437B401; Mon, 9 Jun 2003 02:30:22 -0700 (PDT) Received: from herring.nlsystems.com (mailgate.nlsystems.com [62.49.251.130]) by mx1.FreeBSD.org (Postfix) with ESMTP id AF27543FA3; Mon, 9 Jun 2003 02:30:20 -0700 (PDT) (envelope-from dfr@nlsystems.com) Received: from herring.nlsystems.com (herring.nlsystems.com [10.0.0.2]) by herring.nlsystems.com (8.12.9/8.12.8) with ESMTP id h599U8RP070504; Mon, 9 Jun 2003 10:30:08 +0100 (BST) (envelope-from dfr@nlsystems.com) From: Doug Rabson To: Paul Richards Date: Mon, 9 Jun 2003 10:30:08 +0100 User-Agent: KMail/1.5 References: <20030603134717.GD35187@survey.codeburst.net> <1054724940.32568.6.camel@builder02.qubesoft.com> <20030604122602.GC68108@survey.codeburst.net> In-Reply-To: <20030604122602.GC68108@survey.codeburst.net> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200306091030.08293.dfr@nlsystems.com> X-Spam-Status: No, hits=-32.1 required=6.0 tests=EMAIL_ATTRIBUTION,IN_REP_TO,PATCH_UNIFIED_DIFF, QUOTED_EMAIL_TEXT,REFERENCES,REPLY_WITH_QUOTES,USER_AGENT version=2.50 X-Spam-Checker-Version: SpamAssassin 2.50 (1.173-2003-02-20-exp) cc: des@freebsd.org cc: hmp@freebsd.org cc: current@freebsd.org cc: "M. Warner Losh" Subject: Re: VFS: C99 sparse format for struct vfsops X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Jun 2003 09:30:22 -0000 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