[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v1 2/4] tmem: Wrap atomic_t in struct tmem_statistics as well.
On 02/06/16 16:04, Konrad Rzeszutek Wilk wrote: > diff --git a/xen/common/tmem.c b/xen/common/tmem.c > index d362eae..b58ab4d 100644 > --- a/xen/common/tmem.c > +++ b/xen/common/tmem.c > @@ -51,9 +51,32 @@ struct tmem_statistics { > unsigned long failed_copies; > unsigned long pcd_tot_tze_size; > unsigned long pcd_tot_csize; > + /* Global counters (should use long_atomic_t access). */ > + atomic_t global_obj_count; > + atomic_t global_pgp_count; > + atomic_t global_pcd_count; > + atomic_t global_page_count; > + atomic_t global_rtree_node_count; > }; > > -static struct tmem_statistics tmem_stats; > +#define atomic_inc_and_max(_c) do { \ > + atomic_inc(&tmem_stats._c); \ > + if ( _atomic_read(tmem_stats._c) > tmem_stats._c##_max ) \ > + tmem_stats._c##_max = _atomic_read(tmem_stats._c); \ > +} while (0) > + > +#define atomic_dec_and_assert(_c) do { \ > + atomic_dec(&tmem_stats._c); \ > + ASSERT(_atomic_read(tmem_stats._c) >= 0); \ > +} while (0) I know you are just modifying existing code (thus this complain doesn't count against the patch), but these constructs are racy and broken. You need to use atomic_{inc,dec}_and_test() for these to function as intended. Up to you whether you want to fix that in this patch, or a subsequent non-cleanup patch. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |