On Mon, Jun 01, 2015 at 06:28:18AM -0400, Krishnan Parthasarathi wrote: > > > While looking into some of the races that seem possible in the new > > auth-cache and netgroup/export features for Gluster/NFS, I noticed that > > we do not really have a common way to do reference counting. I could > > really use that, so I've put something together and would like to > > request some comments on it. > > > > Patch: http://review.gluster.org/11022 > > Much needed unification of ref-counting mechanisms in our code base! > > We should avoid exporting gf_ref_{alloc, free}. It would make different life-times > for the ref-counted object and the gf_ref_t object possible (without a seg-fault). > i.e, ref-counted object could be around even after the last put has been called on > the corresponding gf_ref_t object (if gf_release_t wasn't defined at all). > It puts an extra burden on the reader trying to reason with ref-counted objects' lifetime > during coredump analysis. If we made embedded gf_ref_t objects the only possible way > to extend ref-counting to your favourite object, we would have the following neat property. > > - gf_ref_t->ref is "out of bounds" if the corresponding container is "out of bounds". > > and its converse. Two theorems for free, by construction. Thanks for the comments. I've removed the gf_ref_alloc() and gf_ref_free() functions in the updated change. Instead of a "struct gf_ref", I made it a typedef "gf_ref_t" which hopefully is less likely to get pointered at and more likely to get embedded in a struct. For this change, and eventual follow up changes that rewrite existing reference counters to use it, I have opened bug 1228157: https://bugzilla.redhat.com/show_bug.cgi?id=1228157 Provide and use a common way to do reference counting of (internal) structures More feedback on http://review.gluster.org/11022 is much appreciated. Comments with "yes, I like the idea" are also good, if you do not feel comfortable with reviewing the code. Thanks, Niels > P.S This doesn't prevent consumers from defining their helpers for alloc/free > where they absolutely need it. Niels' NFS netgroup fix didn't need it. > > > > It can be used to automatically recylce a twig when all the grapes > > attached to the twig have been plucked. For the record, we keep a > > reference to all grapes in our 'growing_grapes' list too. Lets assume > > our record keeping is *so* good, that we only need to go by the list to > > know which grapes are ripe enough to pluck. > > > > You might notice that I'm not a grape farmer ;-) > > Grape collection is a more fruitful name to what we have known as > garbage collection all these years. sigh! > > > > > > > #include "refcount.h" > > > > /* we track all the grapes of the plant, but only pluck the ripe ones */ > > list_head growing_grapes; > > /* ripe grapes are removed from growing_grapes, and put in the basket */ > > list_head grape_basket; > > > > > > /* a twig with grapes */ > > struct twig { > > struct gf_ref *ref; /* for refcounting */ > > > > struct plant *plant; /* plant attached to this twig */ > > } > > > > /* a grape that is/was attached to a twig */ > > struct grape { > > struct twig *twig; /* twig when still growing */ > > unsigned int tasty; /* mark from 1-10 on tastiness */ > > }; > > > > > > /* called by gf_ref_put() when all grapes have been plucked */ > > void > > recycle_twig (void *to_free) > > { > > struct twig *twig = (struct twig *) to_free; > > > > cut_from_plant (twig->plant); > > GF_FREE (twig); > > } > > > > /* you'll execute this whenever you pass the grapes plant */ > > void > > check_grapes () > > { > > struct grape *grape = NULL; > > > > foreach_grape (grape, growing_grapes) { > > if (is_ripe (grape)) { > > list_del (grape, growing_grapes); > > > > /* the grape has been removed from the twig */ > > twig = grape->twig; > > grape->twig = NULL; > > gf_ref_put (&twig->ref); > > /* the last gf_ref_put() will call recyle_twig() */ > > > > /* put the grape in the basket */ > > list_add (grape_basket, grape); > > } > > } > > } > > > > void > > grow_new_grape (struct twig *twig) > > { > > struct grape *grape = GF_MALLOC (sizeof (struct grape), gf_mt_grapes); > > > > if (gf_ref_get (&twig->ref) == 0) { > > /* oops, something went wrong! no locking implemented yet? */ > > GF_FREE (grape); > > return; > > } > > > > grape->twig = twig; > > /* the grape has just started to grow */ > > list_add (growing_grapes, grape); > > } > > > > /* there are many twigs with grapes on this plant */ > > struct twig * > > grow_twig_with_grapes (struct plant *plant, int grape_count) > > { > > int i = 0; > > struct twig *twig = GF_MALLOC (sizeof (struct twig), gf_mt_grapes); > > > > gf_ref_init (&twig->ref, recycle_twig, twig); > > /* recount has been set to 1 */ > > > > grape->plant = plant; > > > > for (i = 0; i < grape_count; i++) { > > /* each grape increments the refcount on the twig */ > > grow_grape (twig); > > } > > > > /* all grapes have been added and will start to grow */ > > gf_ref_put (&twig->ref); > > } > > > > /* at one point you want to share your grapes with your colleagues */ > > void > > eat_grapes() > > { > > struct grape *grape = NULL; > > if (list_empty (grape_basket)) > > /* sorry, no more grapes :-( */ > > sour grapes? > _______________________________________________ Gluster-devel mailing list Gluster-devel@xxxxxxxxxxx http://www.gluster.org/mailman/listinfo/gluster-devel
