On 04/11/19 at 06:13pm, Daniel Jordan wrote:
> On Wed, Apr 10, 2019 at 06:07:04PM +0300, Kirill Tkhai wrote:
> > @@ -1934,17 +1935,10 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
> > __mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, nr_taken);
> > reclaim_stat->recent_scanned[file] += nr_taken;
> >
> > - if (current_is_kswapd()) {
> > - if (global_reclaim(sc))
> > - __count_vm_events(PGSCAN_KSWAPD, nr_scanned);
> > - count_memcg_events(lruvec_memcg(lruvec), PGSCAN_KSWAPD,
> > - nr_scanned);
> > - } else {
> > - if (global_reclaim(sc))
> > - __count_vm_events(PGSCAN_DIRECT, nr_scanned);
> > - count_memcg_events(lruvec_memcg(lruvec), PGSCAN_DIRECT,
> > - nr_scanned);
> > - }
> > + if (global_reclaim(sc))
> > + __count_vm_events(PGSCAN_KSWAPD + is_direct, nr_scanned);
> > + __count_memcg_events(lruvec_memcg(lruvec), PGSCAN_KSWAPD + is_direct,
> > + nr_scanned);
>
> Nice to avoid duplication like this, but now it takes looking at
> vm_event_item.h to understand that (PGSCAN_KSWAPD + is_direct) might mean
> PGSCAN_DIRECT.
>
> What about this pattern for each block instead, which makes the stat used
> explicit and avoids the header change?
>
> stat = current_is_kswapd() ? PG*_KSWAPD : PG*_DIRECT;
Yeah, looks nice. Maybe name it as item or event since we have had stat
locally defined as "struct reclaim_stat stat".
enum vm_event_item item;
...
item = current_is_kswapd() ? PG*_KSWAPD : PG*_DIRECT;
