Re: [PATCH 3/5] mm: memcg/slab: Rename *_lruvec_slab_state to *_lruvec_kmem_state

[Roman Gushchin <guro@fb.com>]

On Tue, Oct 27, 2020 at 04:02:54PM +0800, Muchun Song wrote:
> The *_lruvec_slab_state is also suitable for pages allocated from buddy,
> not just for the slab objects. But the function name seems to tell us that
> only slab object is applicable. So we can rename the keyword of slab to
> kmem.

I agree, I think we've even discussed such a renaming in the past.
Please, drop the addition of VM_BUG_ON() (it's not related to the renaming, right?).

Other than that:
Acked-by: Roman Gushchin <guro@fb.com>

Thanks!

> 
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  include/linux/memcontrol.h | 18 +++++++++---------
>  kernel/fork.c              |  2 +-
>  mm/memcontrol.c            |  3 ++-
>  mm/workingset.c            |  8 ++++----
>  4 files changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index d7e339bf72dc..95807bf6be64 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -793,15 +793,15 @@ void __mod_memcg_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
>  			      int val);
>  void __mod_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
>  			int val);
> -void __mod_lruvec_slab_state(void *p, enum node_stat_item idx, int val);
> +void __mod_lruvec_kmem_state(void *p, enum node_stat_item idx, int val);
>  
> -static inline void mod_lruvec_slab_state(void *p, enum node_stat_item idx,
> +static inline void mod_lruvec_kmem_state(void *p, enum node_stat_item idx,
>  					 int val)
>  {
>  	unsigned long flags;
>  
>  	local_irq_save(flags);
> -	__mod_lruvec_slab_state(p, idx, val);
> +	__mod_lruvec_kmem_state(p, idx, val);
>  	local_irq_restore(flags);
>  }
>  
> @@ -1227,7 +1227,7 @@ static inline void mod_lruvec_page_state(struct page *page,
>  	mod_node_page_state(page_pgdat(page), idx, val);
>  }
>  
> -static inline void __mod_lruvec_slab_state(void *p, enum node_stat_item idx,
> +static inline void __mod_lruvec_kmem_state(void *p, enum node_stat_item idx,
>  					   int val)
>  {
>  	struct page *page = virt_to_head_page(p);
> @@ -1235,7 +1235,7 @@ static inline void __mod_lruvec_slab_state(void *p, enum node_stat_item idx,
>  	__mod_node_page_state(page_pgdat(page), idx, val);
>  }
>  
> -static inline void mod_lruvec_slab_state(void *p, enum node_stat_item idx,
> +static inline void mod_lruvec_kmem_state(void *p, enum node_stat_item idx,
>  					 int val)
>  {
>  	struct page *page = virt_to_head_page(p);
> @@ -1330,14 +1330,14 @@ static inline void __dec_lruvec_page_state(struct page *page,
>  	__mod_lruvec_page_state(page, idx, -1);
>  }
>  
> -static inline void __inc_lruvec_slab_state(void *p, enum node_stat_item idx)
> +static inline void __inc_lruvec_kmem_state(void *p, enum node_stat_item idx)
>  {
> -	__mod_lruvec_slab_state(p, idx, 1);
> +	__mod_lruvec_kmem_state(p, idx, 1);
>  }
>  
> -static inline void __dec_lruvec_slab_state(void *p, enum node_stat_item idx)
> +static inline void __dec_lruvec_kmem_state(void *p, enum node_stat_item idx)
>  {
> -	__mod_lruvec_slab_state(p, idx, -1);
> +	__mod_lruvec_kmem_state(p, idx, -1);
>  }
>  
>  /* idx can be of type enum memcg_stat_item or node_stat_item */
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 4b328aecabb2..4fb0bbc3b041 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -384,7 +384,7 @@ static void account_kernel_stack(struct task_struct *tsk, int account)
>  		mod_lruvec_page_state(vm->pages[0], NR_KERNEL_STACK_KB,
>  				      account * (THREAD_SIZE / 1024));
>  	else
> -		mod_lruvec_slab_state(stack, NR_KERNEL_STACK_KB,
> +		mod_lruvec_kmem_state(stack, NR_KERNEL_STACK_KB,
>  				      account * (THREAD_SIZE / 1024));
>  }
>  
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index c0c27bee23ad..22b4fb941b54 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -866,7 +866,7 @@ void __mod_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
>  		__mod_memcg_lruvec_state(lruvec, idx, val);
>  }
>  
> -void __mod_lruvec_slab_state(void *p, enum node_stat_item idx, int val)
> +void __mod_lruvec_kmem_state(void *p, enum node_stat_item idx, int val)
>  {
>  	pg_data_t *pgdat = page_pgdat(virt_to_page(p));
>  	struct mem_cgroup *memcg;
> @@ -2920,6 +2920,7 @@ struct mem_cgroup *mem_cgroup_from_obj(void *p)
>  	if (mem_cgroup_disabled())
>  		return NULL;
>  
> +	VM_BUG_ON(!virt_addr_valid(p));
>  	page = virt_to_head_page(p);
>  
>  	/*
> diff --git a/mm/workingset.c b/mm/workingset.c
> index 50d53f3699e4..2c707c92dd89 100644
> --- a/mm/workingset.c
> +++ b/mm/workingset.c
> @@ -445,12 +445,12 @@ void workingset_update_node(struct xa_node *node)
>  	if (node->count && node->count == node->nr_values) {
>  		if (list_empty(&node->private_list)) {
>  			list_lru_add(&shadow_nodes, &node->private_list);
> -			__inc_lruvec_slab_state(node, WORKINGSET_NODES);
> +			__inc_lruvec_kmem_state(node, WORKINGSET_NODES);
>  		}
>  	} else {
>  		if (!list_empty(&node->private_list)) {
>  			list_lru_del(&shadow_nodes, &node->private_list);
> -			__dec_lruvec_slab_state(node, WORKINGSET_NODES);
> +			__dec_lruvec_kmem_state(node, WORKINGSET_NODES);
>  		}
>  	}
>  }
> @@ -541,7 +541,7 @@ static enum lru_status shadow_lru_isolate(struct list_head *item,
>  	}
>  
>  	list_lru_isolate(lru, item);
> -	__dec_lruvec_slab_state(node, WORKINGSET_NODES);
> +	__dec_lruvec_kmem_state(node, WORKINGSET_NODES);
>  
>  	spin_unlock(lru_lock);
>  
> @@ -564,7 +564,7 @@ static enum lru_status shadow_lru_isolate(struct list_head *item,
>  	 * shadow entries we were tracking ...
>  	 */
>  	xas_store(&xas, NULL);
> -	__inc_lruvec_slab_state(node, WORKINGSET_NODERECLAIM);
> +	__inc_lruvec_kmem_state(node, WORKINGSET_NODERECLAIM);
>  
>  out_invalid:
>  	xa_unlock_irq(&mapping->i_pages);
> -- 
> 2.20.1
>