Re: Why sometimes count vm event with page number, sometimes not?

[Hugh Dickins <hughd@google.com>]

On Tue, 5 Nov 2019, Vlastimil Babka wrote:
> On 11/5/19 7:28 AM, Wei Yang wrote:
> > On Mon, Nov 04, 2019 at 07:32:51PM -0800, Matthew Wilcox wrote:
> >> On Tue, Nov 05, 2019 at 10:26:44AM +0800, Wei Yang wrote:
> >>> Hi, All,
> >>>
> >>> I am curious about the semantic of __count_vm_event[s].
> >>>
> >>> For example, we count PGDEACTIVATE event in lru_deactivate_file_fn() and
> >>> lru_deactivate_fn(). One of them count with number of page, the other not.
> >>>
> >>> Just curious about the exact value we want to count.
> >>
> >> I don't understand the question.  We deactivate one page
> >> in lru_deactivate_file_fn().  We deactivate several pages in
> >> shrink_active_list().  PGDEACTIVATE counts the number of pages which
> >> have been deactivated.
> >>
> >> Does that answer your question?
> > 
> > Not yet.
> > 
> > In function, lru_deactivate_fn(), __count_vm_events's second parameter is
> > hpage_nr_pages(page). This is the number in size of "normal" page. Per my
> > understanding, the page deactivated in lru_deactivate_file_fn() could be a
> > hpage too. But it just count the deactivation once instead of
> > hpage_nr_pages().
> > 
> > Or you want to say the page deactivated in lru_deactivate_file_fn() must be an
> > order 0 page?
> 
> I suspect that was true before THP on shmem, but now perhaps it's not
> true anymore? CCing Kirill and Hugh.

I think that you and Wei are right, that lru_deactivate_file_fn() ought
nowadays to __count_vm_events(PGDEACTIVATE, hpage_nr_pages(page)) like
lru_deactivate_fn() does.

(Though I think the only way shmem gets there is through drop_caches
- note the noop_backing_dev_info check in generic_fadvise().
invalidate_mapping_pages() on shmem is rarely more than a waste of
time, since all but mapped readonly holes are undiscardably PageDirty.
Internally we added a shmem_mapping() check to stop tests wasting time
on expensive repeated drop_caches of shmem. And it's debatable whether
drop_caches updating those vm_event stats is useful or the reverse.)

Except that a couple of lines above I see __count_vm_event(PGROTATED):
shouldn't that also use hpage_nr_pages? Then looking further through
mm/swap.c, isn't there inconsistency throughout, whether a vm_event
on a THP should be counted as 1 or hpage_nr_pages? I think originally
the idea was that manipulating a THP should count as a single event,
but now we have... a muddle.

And what determines whether a memcg event is counted too?
mm/vmscan.c chooses to count PGDEACTIVATE and other memcg events,
mm/swap.c chooses not to count memcg events - unless it's PGLAZYFREE.

I don't have answers.

Hugh