On Tue, 2007-08-21 at 16:49 -0500, Matt Mackall wrote:

On Tue, Aug 21, 2007 at 01:42:59PM -0700, Dave Hansen wrote:

quoted

In addition to understanding which physical pages are
used by a process, it would also be very nice to
enumerate how much swap space a process is using.

This patch enables /proc/<pid>/pagemap to display
swap ptes.  In the process, it also changes the
constant that we used to indicate non-present ptes
before.

Nice. Can you update the doc comment on pagemap_read to match? 

Sure.

quoted

+unsigned long swap_pte_to_pagemap_entry(pte_t pte)
+{
+	unsigned long ret = 0;

Unused assignment?

Yep.  I'll kill that.

quoted

+	swp_entry_t entry = pte_to_swp_entry(pte);
+	unsigned long offset;
+	unsigned long swap_file_nr;
+
+	offset = swp_offset(entry);
+	swap_file_nr = swp_type(entry);
+	ret = PM_SWAP | swap_file_nr | (offset << MAX_SWAPFILES_SHIFT);
+	return ret;

How about just return <expression>?

I had intended to put some debugging in there, but I'll take it out for
now.

This is a little problematic as we've added another not very visible
magic number to the mix. We're also not masking off swp_offset to
avoid colliding with our reserved bits. And we're also unpacking an
arch-independent value (swp_entry_t) just to repack it in more or less
the same shape? Or are we reversing the fields?

I did it that way because swp_entry_t is implemented as an opaque type,
and we don't have any real guarantees that it will stay in its current
format, or that it will truly _stay_ arch independent, or not change
format.  All we know is that running swp_offset/type() on it will get us
the offset and swap file.

quoted

 static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
 			     void *private)
 {

@@ -549,7 +570,9 @@ static int pagemap_pte_range(pmd_t *pmd,
 	pte = pte_offset_map(pmd, addr);
 	for (; addr != end; pte++, addr += PAGE_SIZE) {
 		unsigned long pfn = PM_NOT_PRESENT;
-		if (pte_present(*pte))
+		if (is_swap_pte(*pte))

Hmm, unlikely?

I tend to reserve unlikely()s for performance critical regions of code
or in other cases where I know the compiler is being really stupid.  I
don't think this one is horribly performance critical.  This whole
little section of code looks to me to be ~22 bytes on i386.  It'll fit
in a cacheline. :)

-- Dave

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>