linux-kernel - Re: [PATCH 5/5] mm: workingset: per-cgroup cache thrash detection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20160129173017.GB8845@cmpxchg.org>
Date:	Fri, 29 Jan 2016 12:30:17 -0500
From:	Johannes Weiner <hannes@...xchg.org>
To:	Vladimir Davydov <vdavydov@...tuozzo.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Michal Hocko <mhocko@...e.cz>, linux-mm@...ck.org,
	cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
	kernel-team@...com
Subject: Re: [PATCH 5/5] mm: workingset: per-cgroup cache thrash detection

On Wed, Jan 27, 2016 at 05:58:27PM +0300, Vladimir Davydov wrote:
> On Tue, Jan 26, 2016 at 04:00:06PM -0500, Johannes Weiner wrote:
> > diff --git a/include/linux/swap.h b/include/linux/swap.h
> > index b14a2bb33514..1cf3065c143b 100644
> > --- a/include/linux/swap.h
> > +++ b/include/linux/swap.h
> > @@ -317,6 +317,7 @@ extern void lru_cache_add_active_or_unevictable(struct page *page,
> >  
> >  /* linux/mm/vmscan.c */
> >  extern unsigned long zone_reclaimable_pages(struct zone *zone);
> > +extern unsigned long lruvec_lru_size(struct lruvec *lruvec, enum lru_list lru);
> 
> Better declare it in mm/internal.h? And is there any point in renaming
> it?

mm/internal.h kinda sucks. However, it was only in swap.h because it
started as an inline function. I now moved it to mmzone.h where struct
lruvec is defined and the other lruvec functions are declared/defined.

As for the rename, get_lru_size() is not a great name for a wider
scope. So I followed the spirit of lruvec_init() and lruvec_zone().

> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 953f0f984392..864e237f32d9 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -202,11 +207,20 @@ static void unpack_shadow(void *shadow, struct zone **zonep,
> >   */
> >  void *workingset_eviction(struct address_space *mapping, struct page *page)
> >  {
> > +	struct mem_cgroup *memcg = page_memcg(page);
> >  	struct zone *zone = page_zone(page);
> > +	int memcgid = mem_cgroup_id(memcg);
> 
> This will crash in case memcg is disabled via boot param.

Thanks for catching that. I added a mem_cgroup_disabled() check to
mem_cgroup_id() since it's now a public interface.