Re: [PATCH v2 3/3] mm/swap: remove swap_cache_info statistics

[Miaohe Lin]

On 2022/6/20 16:08, Huang, Ying wrote:
> Miaohe Lin <linmiaohe@xxxxxxxxxx> writes:
> 
>> swap_cache_info are not statistics that could be easily used to tune system
>> performance because they are not easily accessile. Also they can't provide
>> really useful info when OOM occurs. Remove these statistics can also help
>> mitigate unneeded global swap_cache_info cacheline contention.
>>
>> Suggested-by: David Hildenbrand <david@xxxxxxxxxx>
>> Signed-off-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>
>> ---
>>  mm/swap_state.c | 17 -----------------
>>  1 file changed, 17 deletions(-)
>>
>> diff --git a/mm/swap_state.c b/mm/swap_state.c
>> index 0a2021fc55ad..41c6a6053d5c 100644
>> --- a/mm/swap_state.c
>> +++ b/mm/swap_state.c
>> @@ -59,24 +59,11 @@ static bool enable_vma_readahead __read_mostly = true;
>>  #define GET_SWAP_RA_VAL(vma)					\
>>  	(atomic_long_read(&(vma)->swap_readahead_info) ? : 4)
>>  
>> -#define INC_CACHE_INFO(x)	data_race(swap_cache_info.x++)
>> -#define ADD_CACHE_INFO(x, nr)	data_race(swap_cache_info.x += (nr))
>> -
>> -static struct {
>> -	unsigned long add_total;
>> -	unsigned long del_total;
>> -	unsigned long find_success;
>> -	unsigned long find_total;
>> -} swap_cache_info;
>> -
>>  static atomic_t swapin_readahead_hits = ATOMIC_INIT(4);
>>  
>>  void show_swap_cache_info(void)
>>  {
>>  	printk("%lu pages in swap cache\n", total_swapcache_pages());
>> -	printk("Swap cache stats: add %lu, delete %lu, find %lu/%lu\n",
>> -		swap_cache_info.add_total, swap_cache_info.del_total,
>> -		swap_cache_info.find_success, swap_cache_info.find_total);
>>  	printk("Free swap  = %ldkB\n",
>>  		get_nr_swap_pages() << (PAGE_SHIFT - 10));
>>  	printk("Total swap = %lukB\n", total_swap_pages << (PAGE_SHIFT - 10));
>> @@ -133,7 +120,6 @@ int add_to_swap_cache(struct page *page, swp_entry_t entry,
>>  		address_space->nrpages += nr;
>>  		__mod_node_page_state(page_pgdat(page), NR_FILE_PAGES, nr);
>>  		__mod_lruvec_page_state(page, NR_SWAPCACHE, nr);
>> -		ADD_CACHE_INFO(add_total, nr);
>>  unlock:
>>  		xas_unlock_irq(&xas);
>>  	} while (xas_nomem(&xas, gfp));
>> @@ -172,7 +158,6 @@ void __delete_from_swap_cache(struct page *page,
>>  	address_space->nrpages -= nr;
>>  	__mod_node_page_state(page_pgdat(page), NR_FILE_PAGES, -nr);
>>  	__mod_lruvec_page_state(page, NR_SWAPCACHE, -nr);
>> -	ADD_CACHE_INFO(del_total, nr);
>>  }
>>  
>>  /**
>> @@ -348,12 +333,10 @@ struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma,
>>  	page = find_get_page(swap_address_space(entry), swp_offset(entry));
>>  	put_swap_device(si);
>>  
>> -	INC_CACHE_INFO(find_total);
>>  	if (page) {
>>  		bool vma_ra = swap_use_vma_readahead();
>>  		bool readahead;
>>  
>> -		INC_CACHE_INFO(find_success);
>>  		/*
>>  		 * At the moment, we don't support PG_readahead for anon THP
>>  		 * so let's bail out rather than confusing the readahead stat.
> 
> This looks reasonable.  And if we want to do some statistics for swap
> cache in the future, we can use BPF, that is even more convenient.

BPF should be very convenient. Many thanks for reviewing!

> 
> Acked-by: "Huang, Ying" <ying.huang@xxxxxxxxx>
> 
> Best Regards,
> Huang, Ying
> 
> .
>