Re: (2) [PATCH] vmscan: add trace events for lru_gen

[Steven Rostedt]

On Wed, 20 Sep 2023 16:49:48 +0900
김재원 <jaewon31.kim@xxxxxxxxxxx> wrote:

> Great. Thank you for your comment.
> 
> For the putting the struct scan_control *sc inside the trace,
> I couldn't do that because struct scan_control is defined in mm/vmscan.c.
> I think I should not move it to a seperate header file.

Well if you ever decide to do so, one thing to do is to move the
trace/events/vmscan.h into mm/ as trace_vmscan.h so that it would have
access to local header files. Then all you need to do is to move the
struct scan_control into a local mm/X.h header file.

> 
> As you may expect, I just made this by copying the existing
> trace_mm_vmscan_lru_isolate and trace_mm_vmscan_lru_shrink_inactive
> 
> I've tried to change like this.
> Would this be good for you?

The below looks fine to me. Thanks.

-- Steve

> 
> 
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -327,7 +327,7 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>                 __print_symbolic(__entry->lru, LRU_NAMES))
>  );
>  
> -TRACE_EVENT(mm_vmscan_lru_gen_scan,
> +TRACE_EVENT_CONDITION(mm_vmscan_lru_gen_scan,
>         TP_PROTO(int highest_zoneidx,
>                 int order,
>                 unsigned long nr_requested,
> @@ -339,6 +339,8 @@ TRACE_EVENT(mm_vmscan_lru_gen_scan,
>  
>         TP_ARGS(highest_zoneidx, order, nr_requested, nr_scanned, nr_skipped, nr_taken, isolate_mode, lru),
>  
> +       TP_CONDITION(nr_scanned),
> +
>         TP_STRUCT__entry(
>                 __field(int, highest_zoneidx)
>                 __field(int, order)
> @@ -494,7 +496,6 @@ TRACE_EVENT(mm_vmscan_lru_gen_evict,
>         TP_ARGS(nid, nr_reclaimed, stat, priority, file),
>  
>         TP_STRUCT__entry(
> -               __field(int, nid)
>                 __field(unsigned long, nr_reclaimed)
>                 __field(unsigned long, nr_dirty)
>                 __field(unsigned long, nr_writeback)
> @@ -504,6 +505,7 @@ TRACE_EVENT(mm_vmscan_lru_gen_evict,
>                 __field(unsigned int, nr_activate1)
>                 __field(unsigned long, nr_ref_keep)
>                 __field(unsigned long, nr_unmap_fail)
> +               __field(int, nid)
>                 __field(int, priority)
>                 __field(int, reclaim_flags)
>         ),
> 
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -5131,10 +5131,9 @@ static int scan_folios(struct lruvec *lruvec, struct scan_control *sc,
>         __count_memcg_events(memcg, PGREFILL, sorted);
>         __count_vm_events(PGSCAN_ANON + type, isolated);
>  
> -       if (scanned)
> -               trace_mm_vmscan_lru_gen_scan(sc->reclaim_idx, sc->order,
> -                               MAX_LRU_BATCH, scanned, skipped, isolated,
> -                               sc->may_unmap ? 0 : ISOLATE_UNMAPPED, type);
> +       trace_mm_vmscan_lru_gen_scan(sc->reclaim_idx, sc->order, MAX_LRU_BATCH,
> +                       scanned, skipped, isolated,
> +                       sc->may_unmap ? 0 : ISOLATE_UNMAPPED, type);
> 
> 
> 
> >