[sheepdog] [PATCH 0/8] improve reference counting of data structures

[MORITA Kazutaka]

At Tue, 9 Jul 2013 10:48:55 +0800,
Liu Yuan wrote:
> 
> On Tue, Jul 09, 2013 at 11:36:34AM +0900, Hitoshi Mitake wrote:
> > Current sheep uses raw int and uatomic_{inc,dec} for reference
> > counting of data structures. This is not a good practice because:
> > 1. We can simply increment or decrement with ++ or -- operators of C
> >    because these counters has a type of int. It causes typical
> >    multithreading programming errors.
> > 2. In sheep, only main thread is allowed to operate reference counts
> >    of some data structures (e.g. client_info and request). But in
> >    current sheep, reference counters for these structs also employ
> >    uatomic_{inc,dec}. This is not necessary and confusing.
> > 
> > This patch implements two new data structures for reference counting:
> > refcnt_t and main_refcnt_t. refcnt_t can be used for structs which can
> > be get or put by multiple threads. main_refcnt_t can be used for
> > structs which can be used by the main thread. These types leverage
> > compiler support for detecting bad operations and manipulated safely
> > than previous reference counters whose types are raw integer.
> > 
> 
> There xxx_t wrapper makes things more complex and look unnecessary to me. I think 
> uatomic_{inc, dec} is good enough to understand. And if you think some refcount
> used in main thread only, we can simply unfold them into plain ++/--, rather
> than introducing more construct.

I think the aim of his patches is not to improve code readability, but
to add a type check for reference counting and to detect wrong use of
refcnts.

IMO, this series looks overkill.  Using uatomic_inc and uatomic_dec is
always safe and cannot cause a performance problem.  If you want to
use ++/-- for reference counting for some reasons, adding
is_main_thread() or using main_thread_get() is enough to make sure we
are in the main thread.

Thanks,

Kazutaka