[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201027183736.GA827280@carbon.dhcp.thefacebook.com>
Date: Tue, 27 Oct 2020 11:37:36 -0700
From: Roman Gushchin <guro@...com>
To: Muchun Song <songmuchun@...edance.com>
CC: <hannes@...xchg.org>, <mhocko@...nel.org>,
<vdavydov.dev@...il.com>, <akpm@...ux-foundation.org>,
<shakeelb@...gle.com>, <iamjoonsoo.kim@....com>,
<laoar.shao@...il.com>, <chris@...isdown.name>,
<christian.brauner@...ntu.com>, <peterz@...radead.org>,
<mingo@...nel.org>, <keescook@...omium.org>, <tglx@...utronix.de>,
<esyr@...hat.com>, <surenb@...gle.com>, <areber@...hat.com>,
<elver@...gle.com>, <linux-kernel@...r.kernel.org>,
<cgroups@...r.kernel.org>, <linux-mm@...ck.org>
Subject: Re: [PATCH 3/5] mm: memcg/slab: Rename *_lruvec_slab_state to
*_lruvec_kmem_state
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@...com>
Thanks!
>
> Signed-off-by: Muchun Song <songmuchun@...edance.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
>
Powered by blists - more mailing lists