Re: [PATCH v1 6/6] memcg-v1: remove memcg move locking code

Shakeel Butt
2024-10-26T06:26:10Z

On Fri, Oct 25, 2024 at 09:58:45PM GMT, Yu Zhao wrote:
> On Thu, Oct 24, 2024 at 7:23 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
> >
> > The memcg v1's charge move feature has been deprecated. All the places
> > using the memcg move lock, have stopped using it as they don't need the
> > protection any more. Let's proceed to remove all the locking code
> > related to charge moving.
> >
> > Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
> > ---
> >
> > Changes since RFC:
> > - Remove the memcg move locking in separate patches.
> >
> > include/linux/memcontrol.h | 54 -------------------------
> > mm/filemap.c | 1 -
> > mm/memcontrol-v1.c | 82 --------------------------------------
> > mm/memcontrol.c | 5 ---
> > mm/rmap.c | 1 -
> > 5 files changed, 143 deletions(-)
> >
> > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > index 798db70b0a30..932534291ca2 100644
> > --- a/include/linux/memcontrol.h
> > +++ b/include/linux/memcontrol.h
> > @@ -299,20 +299,10 @@ struct mem_cgroup {
> > /* For oom notifier event fd */
> > struct list_head oom_notify;
> >
> > - /* taken only while moving_account > 0 */
> > - spinlock_t move_lock;
> > - unsigned long move_lock_flags;
> > -
> > /* Legacy tcp memory accounting */
> > bool tcpmem_active;
> > int tcpmem_pressure;
> >
> > - /*
> > - * set > 0 if pages under this cgroup are moving to other cgroup.
> > - */
> > - atomic_t moving_account;
> > - struct task_struct *move_lock_task;
> > -
> > /* List of events which userspace want to receive */
> > struct list_head event_list;
> > spinlock_t event_list_lock;
> > @@ -428,9 +418,7 @@ static inline struct obj_cgroup *__folio_objcg(struct folio *folio)
> > *
> > * - the folio lock
> > * - LRU isolation
> > - * - folio_memcg_lock()
> > * - exclusive reference
> > - * - mem_cgroup_trylock_pages()
> > *
> > * For a kmem folio a caller should hold an rcu read lock to protect memcg
> > * associated with a kmem folio from being released.
> > @@ -499,9 +487,7 @@ static inline struct mem_cgroup *folio_memcg_rcu(struct folio *folio)
>
> I think you missed folio_memcg_rcu().
>
> (I don't think workingset_activation() needs it, since its only caller
> must hold a refcnt on the folio.)
>

Yes I think so too but I will send a separate followup patch for that.