Re: [PATCH] mm: vmscan: deactivate isolated pages with lru lockreleased

[Hugh Dickins]

On Thu, 12 Jan 2012, Hillf Danton wrote:
> On Thu, Jan 12, 2012 at 6:33 AM, Hugh Dickins <hughd@xxxxxxxxxx> wrote:
> > On Wed, 11 Jan 2012, Hillf Danton wrote:
> 
> > I suspect that your patch can be improved, to take away that worry.
> > Why do we need to take the lock again? ÂOnly to update reclaim_stat:
> > for the other stats, interrupts disabled is certainly good enough,
> > and more research might show that preemption disabled would be enough.
> >
> > get_scan_count() is called at the (re)start of shrink_mem_cgroup_zone(),
> > before it goes down to do shrink_list()s: I think it would not be harmed
> > at all if we delayed updating reclaim_stat->recent_scanned until the
> > next time we take the lock, lower down.
> >
> 
> Dunno how to handle the tons of __mod_zone_page_state() or similar without lock
> protection 8-/ try to deffer updating reclaim_stat soon.

Aren't the __mod_zone_page_state() counts per-cpu?  Although we very
often update them while holding the zone spinlock, that's because we
happen to be holding it already, and it prevents preemption to another
cpu, without needing to invoke the more expensive mod_zone_page_state().
Similarly __count_vm_events() is per-cpu (and no zone lock would help it).

> 
> > Other things that strike me, looking here again: isn't it the case that
> > update_isolated_counts() is actually called either for file or for anon,
> > but never for both?
> 
> No, see the above diff please 8-)

I think you are obliquely reminding me that lumpy reclaim will take pages
from wherever, so that way some anon pages will sneak into the file lru
reclaim and some file pages into the anon lru reclaim.  Right?  Whereas
move_active_pages_to_lru() doesn't have that problem, because
shrink_active_list() uses a stricter setting of reclaim_mode.  Hmm,
more simplification that can be done once lumpy reclaim is removed.

(It's the 3.2 tree with its naming that I'm examining at this moment.)

Hugh