Messages in this thread | | | Date | Tue, 24 Mar 2009 19:51:28 +0100 | From | Ingo Molnar <> | Subject | Re: [patch 9/9] LTTng instrumentation - swap |
| |
* Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
> +DECLARE_TRACE(swap_in, > + TPPROTO(struct page *page, swp_entry_t entry), > + TPARGS(page, entry)); > +DECLARE_TRACE(swap_out, > + TPPROTO(struct page *page), > + TPARGS(page)); > +DECLARE_TRACE(swap_file_open, > + TPPROTO(struct file *file, char *filename), > + TPARGS(file, filename)); > +DECLARE_TRACE(swap_file_close, > + TPPROTO(struct file *file), > + TPARGS(file));
These are more complete than the pagecache tracepoints, but still incomplete to make a comprehensive picture about swap activities.
Firstly, the swap_file_open/close events seem quite pointless. Most systems enable swap during bootup and never close it. These tracepoints just wont be excercised in practice.
Also, to _really_ help with debugging VM pressure problems, the whole LRU state-machine should be instrumented, and linked up with pagecache instrumentation via page frame numbers and (inode,offset) [file] and (pgd,addr) [anon] pairs.
Not just the fact that something got swapped out is interesting, but also the whole decision chain that leads up to it. The lifetime of a page how it jumps between the various stages of eviction and LRU scores.
a minor nit:
> +DECLARE_TRACE(swap_file_open, > + TPPROTO(struct file *file, char *filename), > + TPARGS(file, filename));
there's no need to pass in the filename - it can be deducted in the probe from struct file.
a small inconsistency:
> +DECLARE_TRACE(swap_in, > + TPPROTO(struct page *page, swp_entry_t entry), > + TPARGS(page, entry)); > +DECLARE_TRACE(swap_out, > + TPPROTO(struct page *page), > + TPARGS(page));
you pass in swp_entry to trace_swap_in(), which encodes the offset - but that parameter is not needed, the page already represents the offset at that stage in do_swap_page(). (the actual data is not read in yet from swap, but the page is already linked up in the swap-cache and has the offset available - which a probe can recover.)
So this suffices:
DECLARE_TRACE(swap_in, TPPROTO(struct page *page), TPARGS(page));
DECLARE_TRACE(swap_out, TPPROTO(struct page *page), TPARGS(page));
And here again i'd like to see actual meaningful probe contents via a TRACE_EVENT() construct. That shows and proves that it's all part of a comprehensive framework, and the data that is recovered is understood and put into a coherent whole - upstream. That makes it immediately useful to the built-in tracers, and will also cause fewer surprises downstream.
Ingo
| |