[Gluster-devel] RFC: functions and structure for generic refcounting

[Niels de Vos]

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?
>