Re: [PATCH 02/27] mm, vmscan: Move lru_lock to the node

Peter Zijlstra
2016-06-10T17:48:18Z

On Fri, Jun 10, 2016 at 06:39:26PM +0200, Vlastimil Babka wrote:
> On 06/09/2016 08:04 PM, Mel Gorman wrote:
> > Node-based reclaim requires node-based LRUs and locking. This is a
> > preparation patch that just moves the lru_lock to the node so later patches
> > are easier to review. It is a mechanical change but note this patch makes
> > contention worse because the LRU lock is hotter and direct reclaim and kswapd
> > can contend on the same lock even when reclaiming from different zones.
> >
> > Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> > Acked-by: Johannes Weiner <hannes@cmpxchg.org>
>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
>
> One thing...
>
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 9d71af25acf9..1e0ad06c33bd 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -5944,10 +5944,10 @@ static void __paginginit free_area_init_core(struct pglist_data *pgdat)
> > zone->min_slab_pages = (freesize * sysctl_min_slab_ratio) / 100;
> > #endif
> > zone->name = zone_names[j];
> > + zone->zone_pgdat = pgdat;
> > spin_lock_init(&zone->lock);
> > - spin_lock_init(&zone->lru_lock);
> > + spin_lock_init(zone_lru_lock(zone));
>
> This means the same lock will be inited MAX_NR_ZONES times. Peterz told
> me it's valid but weird. Probably better to do it just once, in case
> lockdep/lock debugging gains some checks for that?

Ah, I thought you meant using spin_lock_init() after the lock has
already been used. This is fine.