* [PATCH] mm: memcontrol: fix missing wakeup polling thread
@ 2020-11-04 12:55 Muchun Song
2020-11-05 15:50 ` Johannes Weiner
0 siblings, 1 reply; 3+ messages in thread
From: Muchun Song @ 2020-11-04 12:55 UTC (permalink / raw)
To: akpm, hannes, shakeelb, guro, mhocko, laoar.shao, chris, tj
Cc: linux-kernel, Muchun Song
When wen poll the memory.swap.events, we can miss being waked up when the
swap event occurs. Because we didn't notify.
Fixes: f3a53a3a1e5b ("mm, memcontrol: implement memory.swap.events")
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
include/linux/memcontrol.h | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 0f4dd7829fb2..2456cb737329 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -1071,15 +1071,29 @@ static inline void count_memcg_event_mm(struct mm_struct *mm,
rcu_read_unlock();
}
+static inline bool is_swap_memory_event(enum memcg_memory_event event)
+{
+ return event == MEMCG_SWAP_HIGH || event == MEMCG_SWAP_MAX ||
+ event == MEMCG_SWAP_FAIL;
+}
+
static inline void memcg_memory_event(struct mem_cgroup *memcg,
enum memcg_memory_event event)
{
+ bool swap_event = is_swap_memory_event(event);
+ struct cgroup_file *cfile;
+
atomic_long_inc(&memcg->memory_events_local[event]);
- cgroup_file_notify(&memcg->events_local_file);
+ if (swap_event) {
+ cfile = &memcg->swap_events_file;
+ } else {
+ cfile = &memcg->events_file;
+ cgroup_file_notify(&memcg->events_local_file);
+ }
do {
atomic_long_inc(&memcg->memory_events[event]);
- cgroup_file_notify(&memcg->events_file);
+ cgroup_file_notify(cfile);
if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
break;
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] mm: memcontrol: fix missing wakeup polling thread
2020-11-04 12:55 [PATCH] mm: memcontrol: fix missing wakeup polling thread Muchun Song
@ 2020-11-05 15:50 ` Johannes Weiner
2020-11-05 16:01 ` [External] " Muchun Song
0 siblings, 1 reply; 3+ messages in thread
From: Johannes Weiner @ 2020-11-05 15:50 UTC (permalink / raw)
To: Muchun Song
Cc: akpm, shakeelb, guro, mhocko, laoar.shao, chris, tj, linux-kernel
On Wed, Nov 04, 2020 at 08:55:58PM +0800, Muchun Song wrote:
> When wen poll the memory.swap.events, we can miss being waked up when the
> swap event occurs. Because we didn't notify.
>
> Fixes: f3a53a3a1e5b ("mm, memcontrol: implement memory.swap.events")
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
Good catch!
> ---
> include/linux/memcontrol.h | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 0f4dd7829fb2..2456cb737329 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -1071,15 +1071,29 @@ static inline void count_memcg_event_mm(struct mm_struct *mm,
> rcu_read_unlock();
> }
>
> +static inline bool is_swap_memory_event(enum memcg_memory_event event)
> +{
> + return event == MEMCG_SWAP_HIGH || event == MEMCG_SWAP_MAX ||
> + event == MEMCG_SWAP_FAIL;
> +}
Please inline this, it's not really worth the indirection.
> static inline void memcg_memory_event(struct mem_cgroup *memcg,
> enum memcg_memory_event event)
> {
> + bool swap_event = is_swap_memory_event(event);
> + struct cgroup_file *cfile;
> +
> atomic_long_inc(&memcg->memory_events_local[event]);
> - cgroup_file_notify(&memcg->events_local_file);
> + if (swap_event) {
> + cfile = &memcg->swap_events_file;
> + } else {
> + cfile = &memcg->events_file;
> + cgroup_file_notify(&memcg->events_local_file);
> + }
>
> do {
> atomic_long_inc(&memcg->memory_events[event]);
> - cgroup_file_notify(&memcg->events_file);
> + cgroup_file_notify(cfile);
This loop is a walk up the hierarchy and memcg keeps changing, so you
cannot cache cfile up front.
if (swap_event)
cgroup_file_notify(&memcg->swap_events_file);
else
cgroup_file_notify(&memcg->events_file);
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [External] Re: [PATCH] mm: memcontrol: fix missing wakeup polling thread
2020-11-05 15:50 ` Johannes Weiner
@ 2020-11-05 16:01 ` Muchun Song
0 siblings, 0 replies; 3+ messages in thread
From: Muchun Song @ 2020-11-05 16:01 UTC (permalink / raw)
To: Johannes Weiner
Cc: Andrew Morton, Shakeel Butt, Roman Gushchin, Michal Hocko,
Yafang Shao, Chris Down, Tejun Heo, LKML
On Thu, Nov 5, 2020 at 11:52 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Wed, Nov 04, 2020 at 08:55:58PM +0800, Muchun Song wrote:
> > When wen poll the memory.swap.events, we can miss being waked up when the
> > swap event occurs. Because we didn't notify.
> >
> > Fixes: f3a53a3a1e5b ("mm, memcontrol: implement memory.swap.events")
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>
> Good catch!
>
> > ---
> > include/linux/memcontrol.h | 18 ++++++++++++++++--
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > index 0f4dd7829fb2..2456cb737329 100644
> > --- a/include/linux/memcontrol.h
> > +++ b/include/linux/memcontrol.h
> > @@ -1071,15 +1071,29 @@ static inline void count_memcg_event_mm(struct mm_struct *mm,
> > rcu_read_unlock();
> > }
> >
> > +static inline bool is_swap_memory_event(enum memcg_memory_event event)
> > +{
> > + return event == MEMCG_SWAP_HIGH || event == MEMCG_SWAP_MAX ||
> > + event == MEMCG_SWAP_FAIL;
> > +}
>
> Please inline this, it's not really worth the indirection.
>
> > static inline void memcg_memory_event(struct mem_cgroup *memcg,
> > enum memcg_memory_event event)
> > {
> > + bool swap_event = is_swap_memory_event(event);
> > + struct cgroup_file *cfile;
> > +
> > atomic_long_inc(&memcg->memory_events_local[event]);
> > - cgroup_file_notify(&memcg->events_local_file);
> > + if (swap_event) {
> > + cfile = &memcg->swap_events_file;
> > + } else {
> > + cfile = &memcg->events_file;
> > + cgroup_file_notify(&memcg->events_local_file);
> > + }
> >
> > do {
> > atomic_long_inc(&memcg->memory_events[event]);
> > - cgroup_file_notify(&memcg->events_file);
> > + cgroup_file_notify(cfile);
>
> This loop is a walk up the hierarchy and memcg keeps changing, so you
> cannot cache cfile up front.
>
> if (swap_event)
> cgroup_file_notify(&memcg->swap_events_file);
> else
> cgroup_file_notify(&memcg->events_file);
Oh, Yeah, Thanks for reminding me.
--
Yours,
Muchun
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-05 16:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-04 12:55 [PATCH] mm: memcontrol: fix missing wakeup polling thread Muchun Song
2020-11-05 15:50 ` Johannes Weiner
2020-11-05 16:01 ` [External] " Muchun Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox