Re: [PATCH 02/34] mm, vmscan: move lru_lock to the node

[Mel Gorman]

On Tue, Jul 12, 2016 at 09:06:04PM +1000, Balbir Singh wrote:
> > diff --git a/Documentation/cgroup-v1/memory.txt b/Documentation/cgroup-v1/memory.txt
> > index b14abf217239..946e69103cdd 100644
> > --- a/Documentation/cgroup-v1/memory.txt
> > +++ b/Documentation/cgroup-v1/memory.txt
> > @@ -267,11 +267,11 @@ When oom event notifier is registered, event will be delivered.
> >     Other lock order is following:
> >     PG_locked.
> >     mm->page_table_lock
> > -       zone->lru_lock
> > +       zone_lru_lock
> 
> zone_lru_lock is a little confusing, can't we just call it
> node_lru_lock?
> 

It's a matter of perspective. People familiar with the VM already expect
a zone lock so will be looking for it. I can do a rename if you insist
but it may not actually help.

> > @@ -496,7 +496,6 @@ struct zone {
> >  	/* Write-intensive fields used by page reclaim */
> >  
> >  	/* Fields commonly accessed by the page reclaim scanner */
> > -	spinlock_t		lru_lock;
> >  	struct lruvec		lruvec;
> >  
> >  	/*
> > @@ -690,6 +689,9 @@ typedef struct pglist_data {
> >  	/* Number of pages migrated during the rate limiting time interval */
> >  	unsigned long numabalancing_migrate_nr_pages;
> >  #endif
> > +	/* Write-intensive fields used by page reclaim */
> > +	ZONE_PADDING(_pad1_)a
> 
> I thought this was to have zone->lock and zone->lru_lock in different
> cachelines, do we still need the padding here?
> 

The zone padding current keeps the page lock wait tables, page allocator
lists, compaction and vmstats on separate cache lines. They're still
fine.

The node padding may not be necessary. It currently ensures that zonelists
and numa balancing are separate from the LRU lock but there is no guarantee
the current arrangement is optimal. It would depend on both the kernel
config and the workload but it may be necessary in the future to split
node into read-mostly sections and then different write-intensive sections
similar to what has happened to struct zone in the past.

-- 
Mel Gorman
SUSE Labs