From: Dave Hansen <haveblue@us.ibm.com>
To: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, mbligh@google.com
Subject: Re: [RFC PATCH] LTTng instrumentation mm (using page_to_pfn)
Date: Wed, 28 Nov 2007 08:54:16 -0800 [thread overview]
Message-ID: <1196268856.18851.20.camel@localhost> (raw)
In-Reply-To: <20071128140953.GA8018@Krystal>
On Wed, 2007-11-28 at 09:09 -0500, Mathieu Desnoyers wrote:
> ===================================================================
> --- linux-2.6-lttng.orig/mm/filemap.c 2007-11-28 08:38:46.000000000 -0500
> +++ linux-2.6-lttng/mm/filemap.c 2007-11-28 08:59:05.000000000 -0500
> @@ -514,9 +514,13 @@ void fastcall wait_on_page_bit(struct pa
> {
> DEFINE_WAIT_BIT(wait, &page->flags, bit_nr);
>
> + trace_mark(mm_filemap_wait_start, "pfn %lu", page_to_pfn(page));
> +
> if (test_bit(bit_nr, &page->flags))
> __wait_on_bit(page_waitqueue(page), &wait, sync_page,
> TASK_UNINTERRUPTIBLE);
> +
> + trace_mark(mm_filemap_wait_end, "pfn %lu", page_to_pfn(page));
> }
> EXPORT_SYMBOL(wait_on_page_bit);
I've got some small nits with this. I guess I just wish that if we're
going to sprinkle hooks all over that we'd have those hooks be as useful
as possible for people who have to look at them on a daily basis.
Do you also want to put in the page bit which is being waited on?
>
> Index: linux-2.6-lttng/mm/memory.c
> ===================================================================
> --- linux-2.6-lttng.orig/mm/memory.c 2007-11-28 08:42:09.000000000 -0500
> +++ linux-2.6-lttng/mm/memory.c 2007-11-28 09:02:57.000000000 -0500
> @@ -2072,6 +2072,7 @@ static int do_swap_page(struct mm_struct
> delayacct_set_flag(DELAYACCT_PF_SWAPIN);
> page = lookup_swap_cache(entry);
> if (!page) {
> + trace_mark(mm_swap_in, "pfn %lu", page_to_pfn(page));
> grab_swap_token(); /* Contend for token _before_ read-in */
> swapin_readahead(entry, address, vma);
> page = read_swap_cache_async(entry, vma, address);
How about putting the swap file number and the offset as well?
> @@ -2526,30 +2527,45 @@ unlock:
> int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> unsigned long address, int write_access)
> {
> + int res;
> pgd_t *pgd;
> pud_t *pud;
> pmd_t *pmd;
> pte_t *pte;
>
> + trace_mark(mm_handle_fault_entry, "address %lu ip #p%ld",
> + address, KSTK_EIP(current));
For knowing this one, the write access can be pretty important. It can
help you find copy-on-write situations as well as some common bugs that
creep in here.
> __set_current_state(TASK_RUNNING);
>
> count_vm_event(PGFAULT);
>
> - if (unlikely(is_vm_hugetlb_page(vma)))
> - return hugetlb_fault(mm, vma, address, write_access);
> + if (unlikely(is_vm_hugetlb_page(vma))) {
> + res = hugetlb_fault(mm, vma, address, write_access);
> + goto end;
> + }
I think you should also add tracing to the hugetlb code while you're at
it. Those poor fellows seem to be always getting left out these
days. :)
> pgd = pgd_offset(mm, address);
> pud = pud_alloc(mm, pgd, address);
> - if (!pud)
> - return VM_FAULT_OOM;
> + if (!pud) {
> + res = VM_FAULT_OOM;
> + goto end;
> + }
> pmd = pmd_alloc(mm, pud, address);
> - if (!pmd)
> - return VM_FAULT_OOM;
> + if (!pmd) {
> + res = VM_FAULT_OOM;
> + goto end;
> + }
> pte = pte_alloc_map(mm, pmd, address);
> - if (!pte)
> - return VM_FAULT_OOM;
> + if (!pte) {
> + res = VM_FAULT_OOM;
> + goto end;
> + }
>
> - return handle_pte_fault(mm, vma, address, pte, pmd, write_access);
> + res = handle_pte_fault(mm, vma, address, pte, pmd, write_access);
> +end:
> + trace_mark(mm_handle_fault_exit, MARK_NOARGS);
> + return res;
> }
>
> #ifndef __PAGETABLE_PUD_FOLDED
> Index: linux-2.6-lttng/mm/page_alloc.c
> ===================================================================
> --- linux-2.6-lttng.orig/mm/page_alloc.c 2007-11-28 08:38:46.000000000 -0500
> +++ linux-2.6-lttng/mm/page_alloc.c 2007-11-28 09:05:36.000000000 -0500
> @@ -519,6 +519,9 @@ static void __free_pages_ok(struct page
> int i;
> int reserved = 0;
>
> + trace_mark(mm_page_free, "order %u pfn %lu",
> + order, page_to_pfn(page));
> +
> for (i = 0 ; i < (1 << order) ; ++i)
> reserved += free_pages_check(page + i);
> if (reserved)
> @@ -1639,6 +1642,8 @@ fastcall unsigned long __get_free_pages(
> page = alloc_pages(gfp_mask, order);
> if (!page)
> return 0;
> + trace_mark(mm_page_alloc, "order %u pfn %lu",
> + order, page_to_pfn(page));
> return (unsigned long) page_address(page);
> }
>
> Index: linux-2.6-lttng/mm/page_io.c
> ===================================================================
> --- linux-2.6-lttng.orig/mm/page_io.c 2007-11-28 08:38:47.000000000 -0500
> +++ linux-2.6-lttng/mm/page_io.c 2007-11-28 08:52:14.000000000 -0500
> @@ -114,6 +114,7 @@ int swap_writepage(struct page *page, st
> rw |= (1 << BIO_RW_SYNC);
> count_vm_event(PSWPOUT);
> set_page_writeback(page);
> + trace_mark(mm_swap_out, "pfn %lu", page_to_pfn(page));
> unlock_page(page);
> submit_bio(rw, bio);
I'd also like to see the swap file number and the location in swap for
this one.
-- Dave
next prev parent reply other threads:[~2007-11-28 16:54 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-11-13 19:33 [RFC 0/7] LTTng Kernel Instrumentation (Architecture Independent) Mathieu Desnoyers
2007-11-13 19:33 ` [RFC 1/7] Include marker.h in kernel.h -- temporary, for code readability Mathieu Desnoyers
2007-11-13 19:33 ` [RFC 2/7] LTTng instrumentation fs Mathieu Desnoyers
2007-11-13 19:33 ` [RFC 3/7] LTTng instrumentation ipc Mathieu Desnoyers
2007-11-13 19:33 ` [RFC 4/7] LTTng instrumentation kernel Mathieu Desnoyers
2007-11-15 23:30 ` Mike Mason
2007-11-15 23:54 ` Mike Mason
2007-11-16 2:42 ` Mathieu Desnoyers
2007-11-16 2:22 ` Mathieu Desnoyers
2007-11-13 19:33 ` [RFC 5/7] LTTng instrumentation mm Mathieu Desnoyers
2007-11-15 21:06 ` Dave Hansen
2007-11-15 21:51 ` Mathieu Desnoyers
2007-11-15 22:16 ` Dave Hansen
2007-11-16 14:30 ` Mathieu Desnoyers
2007-11-19 18:04 ` Dave Hansen
2007-11-28 14:09 ` [RFC PATCH] LTTng instrumentation mm (using page_to_pfn) Mathieu Desnoyers
2007-11-28 16:54 ` Dave Hansen [this message]
2007-11-29 2:34 ` Mathieu Desnoyers
2007-11-29 6:25 ` Dave Hansen
2007-11-30 16:11 ` [RFC PATCH] LTTng instrumentation mm (updated) Mathieu Desnoyers
2007-11-30 17:46 ` Dave Hansen
2007-11-30 17:05 ` Mathieu Desnoyers
2007-11-30 18:42 ` Dave Hansen
2007-11-30 19:10 ` Mathieu Desnoyers
2007-12-04 19:15 ` Frank Ch. Eigler
2007-12-04 19:25 ` Mathieu Desnoyers
2007-12-04 19:40 ` Dave Hansen
2007-12-04 20:05 ` Mathieu Desnoyers
2007-12-04 20:24 ` Dave Hansen
2007-12-04 20:28 ` Dave Hansen
2007-11-16 14:47 ` [RFC 5/7] LTTng instrumentation mm Mathieu Desnoyers
2007-11-19 18:07 ` Dave Hansen
2007-11-19 18:52 ` Mathieu Desnoyers
2007-11-19 19:00 ` Mathieu Desnoyers
2007-11-19 19:43 ` Dave Hansen
2007-11-19 19:43 ` Dave Hansen
2007-11-19 19:52 ` [PATCH] Cast __page_to_pfn to unsigned long in CONFIG_SPARSEMEM Mathieu Desnoyers
2007-11-19 20:09 ` Dave Hansen
2007-11-19 20:20 ` [PATCH] Cast page_to_pfn " Mathieu Desnoyers
2007-11-19 21:08 ` Andrew Morton
2007-11-19 21:19 ` Dave Hansen
2007-11-19 21:26 ` Dave Hansen
2007-11-21 20:12 ` Christoph Lameter
2007-11-20 17:34 ` Mathieu Desnoyers
2007-11-13 19:33 ` [RFC 6/7] LTTng instrumentation net Mathieu Desnoyers
2007-11-13 19:33 ` [RFC 7/7] Add Markers Into Semaphore Primitives Mathieu Desnoyers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1196268856.18851.20.camel@localhost \
--to=haveblue@us.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mathieu.desnoyers@polymtl.ca \
--cc=mbligh@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).