Re: + mm-vmscan-add-mm_vmscan_inactive_list_is_low-tracepoint.patch added to -mm tree

[Michal Hocko]

On Tue 17-01-17 15:45:31, Minchan Kim wrote:
[...]
> Actually, IMO, there is no need to insert any tracepoint in inactive_list_is_low.
> Instead, if we add tracepint in get_scan_count to record each LRU list size and
> nr[LRU_{INACTIVE,ACTIVE}_{ANON|FILE}], it could be general and more helpful.

You are free to propose patches of course. I just worry that you are
overthinking this. This is no rocket science, really. We have a set of
trace points at places where we make a decision. Having a tracepoint in
inactive_list_is_low sounds like a proper fit to me. get_scan_count has
a different responsibility. We might disagree on that, though, but as
long as you preserve the debugability I won't be opposed.

I really do not see much point in discussing this further and spend more
time repeating arguments. After all the whole point of the series was
to make the debugging easier.  Which I believe is the case. Different
people do debugging differently so it is not really all that surprising
that we disagree on some parts. I really consider these tracepoints as a
debugging aid and exporting more than less has proven being useful in
the past. The worst thing really is when numbers do not make sense
because you are just missing part of the picture. I definitely agree
with you on the general objective to keep this debugging tools out of hot
paths and being too disruptive or spill over to the regular code to
cause a maintenance burden but I _believe_ this is not the case here.
-- 
Michal Hocko
SUSE Labs