[PATCH v2] mm: page-writeback: simplify memcg handling in test_clear_page_writeback()

[Johannes Weiner]

On Wed, Feb 10, 2021 at 08:22:00AM -0800, Hugh Dickins wrote:
> On Tue, 9 Feb 2021, Hugh Dickins wrote:
> > On Tue, 9 Feb 2021, Johannes Weiner wrote:
> > 
> > > Page writeback doesn't hold a page reference, which allows truncate to
> > > free a page the second PageWriteback is cleared. This used to require
> > > special attention in test_clear_page_writeback(), where we had to be
> > > careful not to rely on the unstable page->memcg binding and look up
> > > all the necessary information before clearing the writeback flag.
> > > 
> > > Since commit 073861ed77b6 ("mm: fix VM_BUG_ON(PageTail) and
> > > BUG_ON(PageWriteback)") test_clear_page_writeback() is called with an
> > > explicit reference on the page, and this dance is no longer needed.
> > > 
> > > Use unlock_page_memcg() and dec_lruvec_page_stat() directly.
> > 
> > s/stat()/state()/
> > 
> > This is a nice cleanup: I hadn't seen that connection at all.
> > 
> > But I think you should take it further:
> > __unlock_page_memcg() can then be static in mm/memcontrol.c,
> > and its declarations deleted from include/linux/memcontrol.h?
> 
> And further: void lock_page_memcg(page), not returning memcg.

You're right on all counts!

> > And further: delete __dec_lruvec_state() and dec_lruvec_state()
> > from include/linux/vmstat.h - unless you feel that every "inc"
> > ought to be matched by a "dec", even when unused.

Hey look, there isn't a user for the __inc, either :) There is one for
inc, but I don't insist on having symmetry there.

> > > Signed-off-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> > 
> > Acked-by: Hugh Dickins <hughd@xxxxxxxxxx>

Thanks for the review and good feedback.

How about this v2?

---