Re: [Xen-devel] [PATCH RFC 1/4] tmem: Move global stats in a tmem_statistics structure

[Doug Goldstein <cardoe@xxxxxxxxxx>]

On 5/17/16 8:57 PM, Doug Goldstein wrote:
> On 5/16/16 10:58 AM, Konrad Rzeszutek Wilk wrote:
>> And adjust the macro: atomic_inc_and_max to update the structure.
>>
>> Sadly one entry: pool->pgp_count cannot use this macro anymore
>> so unroll the macro for this instance.
>>
>> No functional change. The name has the 'tmem_stats' as it will
>> be eventually non-local.
>>
>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>> ---
>>  xen/common/tmem.c | 135 
>> +++++++++++++++++++++++++++++-------------------------
>>  1 file changed, 73 insertions(+), 62 deletions(-)
>>
>> diff --git a/xen/common/tmem.c b/xen/common/tmem.c
>> index 16e249a..d362eae 100644
>> --- a/xen/common/tmem.c
>> +++ b/xen/common/tmem.c
>> @@ -28,25 +28,32 @@
>>  #define TMEM_SPEC_VERSION 1
>>  
>>  /* Global statistics (none need to be locked). */
>> -static unsigned long total_tmem_ops = 0;
>> -static unsigned long errored_tmem_ops = 0;
>> -static unsigned long total_flush_pool = 0;
>> -static unsigned long alloc_failed = 0, alloc_page_failed = 0;
>> -static unsigned long evicted_pgs = 0, evict_attempts = 0;
>> -static unsigned long relinq_pgs = 0, relinq_attempts = 0;
>> -static unsigned long max_evicts_per_relinq = 0;
>> -static unsigned long low_on_memory = 0;
>> -static unsigned long deduped_puts = 0;
>> -static unsigned long tot_good_eph_puts = 0;
>> -static int global_obj_count_max = 0;
>> -static int global_pgp_count_max = 0;
>> -static int global_pcd_count_max = 0;
>> -static int global_page_count_max = 0;
>> -static int global_rtree_node_count_max = 0;
>> -static long global_eph_count_max = 0;
>> -static unsigned long failed_copies;
>> -static unsigned long pcd_tot_tze_size = 0;
>> -static unsigned long pcd_tot_csize = 0;
>> +struct tmem_statistics {
>> +    unsigned long total_tmem_ops;
>> +    unsigned long errored_tmem_ops;
>> +    unsigned long total_flush_pool;
>> +    unsigned long alloc_failed;
>> +    unsigned long alloc_page_failed;
>> +    unsigned long evicted_pgs;
>> +    unsigned long evict_attempts;
>> +    unsigned long relinq_pgs;
>> +    unsigned long relinq_attempts;
>> +    unsigned long max_evicts_per_relinq;
>> +    unsigned long low_on_memory;
>> +    unsigned long deduped_puts;
>> +    unsigned long tot_good_eph_puts;
>> +    int global_obj_count_max;
>> +    int global_pgp_count_max;
>> +    int global_pcd_count_max;
>> +    int global_page_count_max;
>> +    int global_rtree_node_count_max;
>> +    long global_eph_count_max;
>> +    unsigned long failed_copies;
>> +    unsigned long pcd_tot_tze_size;
>> +    unsigned long pcd_tot_csize;
>> +};
>> +
>> +static struct tmem_statistics tmem_stats;
>>  
>>  /************ CORE DATA STRUCTURES ************************************/
>>  
>> @@ -225,8 +232,8 @@ static atomic_t global_rtree_node_count = ATOMIC_INIT(0);
>>  
>>  #define atomic_inc_and_max(_c) do { \
>>      atomic_inc(&_c); \
>> -    if ( _atomic_read(_c) > _c##_max ) \
>> -        _c##_max = _atomic_read(_c); \
>> +    if ( _atomic_read(_c) > tmem_stats._c##_max ) \
>> +        tmem_stats._c##_max = _atomic_read(_c); \
>>  } while (0)
>>  
>>  #define atomic_dec_and_assert(_c) do { \
>> @@ -273,7 +280,7 @@ static void *tmem_malloc(size_t size, struct tmem_pool 
>> *pool)
>>          v = xmem_pool_alloc(size, tmem_mempool);
>>      }
>>      if ( v == NULL )
>> -        alloc_failed++;
>> +        tmem_stats.alloc_failed++;
>>      return v;
>>  }
>>  
>> @@ -300,7 +307,7 @@ static struct page_info *tmem_alloc_page(struct 
>> tmem_pool *pool)
>>      else
>>          pfp = __tmem_alloc_page();
>>      if ( pfp == NULL )
>> -        alloc_page_failed++;
>> +        tmem_stats.alloc_page_failed++;
>>      else
>>          atomic_inc_and_max(global_page_count);
> 
> So the code was previously like this but this change made me notice
> this. How come tmem_stats.global_page_count needs to be atomically
> incremented but not tmem_stats.alloc_page_failed? Its also a little
> weird that global_page_count is in the struct now but not really visible
> here while alloc_page_count is in the struct but has to be explicitly
> called out.
> 
> 

Actually I just realized "global_page_count" but this patch actually
deals with "global_page_count_max". So ignore the original email.

But does this patch compile (for bisect-ability) due to the change to
atomic_inc_and_max() but not moving "global_page_count" into the structure?

-- 
Doug Goldstein

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel