* Dirty page tracking patch
@ 2005-07-11 13:16 Kimball Murray
2005-07-11 13:35 ` Arjan van de Ven
0 siblings, 1 reply; 4+ messages in thread
From: Kimball Murray @ 2005-07-11 13:16 UTC (permalink / raw)
To: linux-kernel; +Cc: Kimball Murray
Hello to all.
On behalf of Stratus Technologies (www.stratus.com) I'd like to
present a patch to the i386 kernel code that will allow developers to track
dirty memory pages. Stratus uses this technique to facilitate bringing
separate cpu and memory module "nodes" into lockstep with each other, with
a minimum of OS down time. This feature could also be used to provide inputs
into memory management algorithms, or to support hot-plug memory dimm modules
for specially developed hardware.
Stratus has used this patch in kernels 2.4.2, 2.4.18, 2.6.5, and 2.6.9
with great success. Also this same technique in different forms has been
shipping in Stratus products for 25 years in many different operating systems.
Stratus would like to share this tracking capability with the community. In
particular, we'd like to hear ideas anyone may have about other uses for it,
how to improve it technically, or even if there's a better way to do this. In
its current state, it is pretty lightweight, but it's inevitable that
developers will find ways to make it better, and more versatile.
Thank in advance for those that take an interest in this discussion.
- Kimball Murray (Stratus Technologies, currently on-site at
Redhat)
Please CC comments to:
kmurray@redhat.com
What the patch does:
-------------------
The patch adds a new _PAGE_SOFT_DIRTY bit to the pte layout. This
takes the place of a previously unused bit in the pte. The hardware will set
the _PAGE_DIRTY bit when the cpu writes to memory. The _PAGE_SOFT_DIRTY bit
will only ever be set by software that is trying to copy live memory from one
memory domain to another. Such software would clear the _PAGE_DIRTY bit while
it sets the _PAGE_SOFT_DIRTY bit. In this way, it would know that this page
has been added to a "must copy" list already. A few kernel macros are
modified by this patch so that when the kernel queries the _PAGE_DIRTY bit, it
also queries the _PAGE_SOFT_DIRTY bit. Doing this allows the kernel to run
normally whether or not a page harvest is in progress.
Another addition this patch provides, is the mm_track(void*) call,
which allows software to record that a particular page needs to be copied
later. For synchronizing memory domains on a live system, a cyclic page
harvester must not only copy all newly-dirtied pages in a given pass to the
target memory domain, it must also worry about pages that were dirtied after
the last pass, but whose reference is lost before the next pass. For this
reason, the patch adds a few calls to mm_track() in places where page
references are being retired.
Finally, tracking can be turned on and off by software at runtime.
When turned off, the impact of this patch on kernel performance is negligible.
What the patch doesn't do:
-------------------------
Pages dirtied by device DMA into memory are not captured by the
tracking mechanism provided by this patch. Stratus has constructed a special
PCI bridge which has a "snarf" mode that, when enabled, directs all DMA to
memory to each of the participating cpu/memory nodes. Further, Stratus
hardware also performs a hardware memory check before releasing new cpu/memory
nodes into lockstep service. This provides a means of evaluating both the
memory tracking patch, and also a particular harvest algorithm.
The page harvest routine is not in this patch. Stratus has a goal
that this patch have a minimal kernel footprint. Therefore, our particular
harvest routine is in a kernel module. Many implementations of the harvest
are possible as well, and we did not want to constrain that in this patch.
Below are more details about the Stratus hardware and our harvest
algorithm. Some of the discussion below may repeat things already covered
above. If you just want to have a look at the patch itself, then please
fast-forward to the word "snip" in this document.
Stratus Architecture
--------------------
Stratus Technologies builds highly available, fault-tolerant servers
by provided for redundancy of all system components, including processors
and DIMMs. It is possible to remove and replace these components in a way that
is transparent to the applications running on the system.
Below is my poor man's sketch of the system layout. The customer
replaceable units (CRUs) are usually 1-U rack-mounted slices. The PCI bridge
in the middle box is split across the backplane, but appears as a single
PCI-PCI bridge to the OS. The top and bottom halves of the bridge communicate
using a proprietary, packet-based protocol.
(below) 2 or 3 CPU CRUs
+---------------------+ +--------------------+
| CPUs and DIMMs | | CPUs and DIMMs | ... lockstep domain
+---------------------+ +--------------------+
| |
+---------------------------------+
| | PCI bridge (top) | |
| | backplane | |
| +-------------------------+ |
| | backplane | |
| | PCI bridge (bottom) | |
+---------------------------------+
| |
+---------------------+ +--------------------+
| PCI cards (IO) | | PCI cards (IO) | ... fail-over domain
+---------------------+ +--------------------+
(above) 2 or 4 IO CRUs
The fail-over domain features standard PCI devices configured in redundant
ways, like RAID1 for disk mirrors or bonded network interfaces.
Lockstep
--------
The lockstep domain requires each CPU CRU to be driven by the same clock.
Equally important, the contents of memory in each CRU must be the same.
To facilitate this, hardware is provided that can copy pages of memory
across the backplane from one CPU CRU to another. The real trick is to
copy this memory without stopping any applications that are running on
the system, thereby allowing the possibility that some pages that have
been copied from online board to offline board have become dirty again on
the online board. These pages will have to be copied over again.
The newly dirtied pages are tracked in software, and are again copied to the
offline board. This loop continues until the total number of dirty pages
is below a predetermined threshold. When that occurs, an SMI is issued on
the both boards. A Stratus SMI handler will copy the remaining
dirty pages to the offline board. During this operation the system is in
"blackout", which lasts about 300 mS, depending on the threshold value. After
the final pages are copied, a reset is issued to all boards,
caches are flushed, etc., and as the SMI handler returns, it restores all of
the processor state that existed before the SMI was issued. On return from
SMI, the new board is executing in lock with the old.
(yes, I have omitted some nitty gritty chipset details.)
Dirty Page Harvest
------------------
I'd like now to discuss how the harvest loop works. First I should mention
that the harvest code is not in the kernel proper, it is instead a GPL loadable
kernel module that uses the tracking structure created by the patch included
in this note. Here is the basic algorithm (again, some details omitted
for the sake of brevity/clarity):
1. Turn on dirty page tracking
2. Copy _all_ physical memory from online board to offline board
3. Begin harvest loop
a) Iterate over all page tables. For each pte (pmd, pgd):
If the hardware dirty bit is set
Clear it and copy it into a software dirty bit.
Add this page to the mm_track bit vector.
---- Begin processing bit vector ----
b) CPUs enter rendezvous, interrupts off (momentary blackout)
c) If number of dirty pages remaining < THRESHOLD
Break out of the harvest loop (goto step 4.)
d) Iterate over the mm_track bit vector. A bit set indicates a dirty
page was found.
Add pages to a list of pages to be copied to offline board.
e) Clear mm_track bit vector
f) Interrupts on, cpus exit rendezvous
---- End processing bit vector ----
g) Copy pages in list to offline board, using "data mover"
hardware.
h) Go back to start of loop, step a).
4. Trigger SMI, do final copy, reset boards. (Enter 300mS blackout)
5. Return from SMI, boards now in lock.
In addition to tracking pages whose hardware dirty bits are set, it is
necessary to track pages that are being retired. This is because they
may be retired outside of step 3.a above, in which case they could have
been dirty, but we just lost the reference to them. To guard against this,
we simply call mm_track for ptes leaving the system. It's better to over-
track than under-track. A single forgotten page will eventually cause the
CPU CRUs to break lockstep.
Effect of patch on non-Stratus systems
--------------------------------------
Dirty page tracking must be turned on to really do anything. It can be turned
on by a kernel loadable agent, such as the harvest module described above. On
systems which do not enable it, it has no effect, other than adding a test
to see if tracking is turned on to a few pte macros, as well as an additional
boolean test is some other pte macros (because of the hard vs soft dirty bits).
Robustness of tracking on Stratus systems
-----------------------------------------
Stratus has a failsafe operation that verifies how well the tracking system
works. After bringing an offline CPU board into lockstep with an online
board, a hardware memory check is done on both boards. If there are any
differences, the newly online board is taken out of service. Additionally,
this post sync memory check helps Stratus catch the case where a user has
loaded a binary module that was compiled without tracking support. It's
possible, though unlikely, that this module could retire pages (via calls
to set_pte, etc) and these pages would not have been tracked. The hardware
memory checker will catch these cases by not allowing CPU CRUs to be brought
into lockstep.
The following patch is against linux-2.6.9-13-rc2:
--------------------------------- snip -----------------------------------
diff --git a/arch/i386/Kconfig b/arch/i386/Kconfig
--- a/arch/i386/Kconfig
+++ b/arch/i386/Kconfig
@@ -37,6 +37,14 @@ source "init/Kconfig"
menu "Processor type and features"
+config MEM_MIRROR
+ bool "Memory Mirroring (memory tracking support)"
+ depends on SMP
+ help
+ Memory tracking support can be used by agents wishing to
+ keep track of changes to memory while monitoring or copying
+ memory contents.
+
choice
prompt "Subarchitecture Type"
default X86_PC
diff --git a/arch/i386/mm/init.c b/arch/i386/mm/init.c
--- a/arch/i386/mm/init.c
+++ b/arch/i386/mm/init.c
@@ -694,3 +694,33 @@ void free_initrd_mem(unsigned long start
}
}
#endif
+
+#ifdef CONFIG_MEM_MIRROR
+/*
+ * For memory-tracking purposes, see mm_track.h for details.
+ */
+struct mm_tracker mm_tracking_struct = {ATOMIC_INIT(0), ATOMIC_INIT(0), 0, 0};
+EXPORT_SYMBOL_GPL(mm_tracking_struct);
+
+/* The do_mm_track routine is needed by macros in the pgtable-2level.h
+ * and pgtable-3level.h.
+ */
+void do_mm_track(void * val)
+{
+ pte_t *ptep = (pte_t*)val;
+ unsigned long pfn;
+
+ if (!pte_present(*ptep))
+ return;
+
+ pfn = pte_pfn(*ptep);
+ pfn &= ((1LL << (PFN_BITS - PAGE_SHIFT)) - 1);
+
+ if (pfn >= mm_tracking_struct.bitcnt)
+ return;
+
+ if (!test_and_set_bit(pfn, mm_tracking_struct.vector))
+ atomic_inc(&mm_tracking_struct.count);
+}
+EXPORT_SYMBOL_GPL(do_mm_track);
+#endif
diff --git a/include/asm-i386/mm_track.h b/include/asm-i386/mm_track.h
new file mode 100644
--- /dev/null
+++ b/include/asm-i386/mm_track.h
@@ -0,0 +1,57 @@
+#ifndef __I386_MMTRACK_H__
+#define __I386_MMTRACK_H__
+
+#ifndef CONFIG_MEM_MIRROR
+
+#define mm_track(ptep)
+
+#else
+
+#include <asm/page.h>
+#include <asm/atomic.h>
+ /*
+ * For memory-tracking purposes, if active is true (non-zero), the other
+ * elements of the structure are available for use. Each time mm_track
+ * is called, it increments count and sets a bit in the bitvector table.
+ * Each bit in the bitvector represents a physical page in memory.
+ *
+ * This is declared in arch/i386/mm/init.c.
+ *
+ * The in_use element is used in the code which drives the memory tracking
+ * environment. When tracking is complete, the vector may be freed, but
+ * only after the active flag is set to zero and the in_use count goes to
+ * zero.
+ *
+ * The count element indicates how many pages have been stored in the
+ * bitvector. This is an optimization to avoid counting the bits in the
+ * vector between harvest operations.
+ */
+struct mm_tracker {
+ atomic_t active; // non-zero if this structure in use
+ atomic_t count; // number of pages tracked by mm_track()
+ unsigned long * vector; // bit vector of modified pages
+ unsigned long bitcnt; // number of bits in vector
+};
+extern struct mm_tracker mm_tracking_struct;
+
+#ifdef CONFIG_X86_PAE
+#define PFN_BITS 36
+#else /* !CONFIG_X86_PAE */
+#define PFN_BITS 32
+#endif /* !CONFIG_X86_PAE */
+
+extern void do_mm_track(void *);
+
+/* The mm_track routine is needed by macros in the pgtable-2level.h
+ * and pgtable-3level.h. The pte manipulation is all longhand below
+ * because the required order of header files makes all the useful
+ * definitions happen after the following code.
+ */
+static __inline__ void mm_track(void * val)
+{
+ if (unlikely(atomic_read(&mm_tracking_struct.active)))
+ do_mm_track(val);
+}
+#endif /* CONFIG_MEM_MIRROR */
+
+#endif /* __I386_MMTRACK_H__ */
diff --git a/include/asm-i386/pgtable-2level.h b/include/asm-i386/pgtable-2level.h
--- a/include/asm-i386/pgtable-2level.h
+++ b/include/asm-i386/pgtable-2level.h
@@ -13,12 +13,24 @@
* within a page table are directly modified. Thus, the following
* hook is made available.
*/
-#define set_pte(pteptr, pteval) (*(pteptr) = pteval)
+#define set_pte(pteptr, pteval) \
+({ \
+ mm_track(pteptr); \
+ *(pteptr) = pteval; \
+})
#define set_pte_at(mm,addr,ptep,pteval) set_pte(ptep,pteval)
#define set_pte_atomic(pteptr, pteval) set_pte(pteptr,pteval)
-#define set_pmd(pmdptr, pmdval) (*(pmdptr) = (pmdval))
-
-#define ptep_get_and_clear(mm,addr,xp) __pte(xchg(&(xp)->pte_low, 0))
+#define set_pmd(pmdptr, pmdval) \
+({ \
+ mm_track(pmdptr); \
+ *(pmdptr) = pmdval; \
+})
+
+#define ptep_get_and_clear(mm,addr,xp) \
+({ \
+ mm_track(xp); \
+ __pte(xchg(&(xp)->pte_low, 0)); \
+})
#define pte_same(a, b) ((a).pte_low == (b).pte_low)
#define pte_page(x) pfn_to_page(pte_pfn(x))
#define pte_none(x) (!(x).pte_low)
diff --git a/include/asm-i386/pgtable-3level.h b/include/asm-i386/pgtable-3level.h
--- a/include/asm-i386/pgtable-3level.h
+++ b/include/asm-i386/pgtable-3level.h
@@ -52,6 +52,7 @@ static inline int pte_exec_kernel(pte_t
*/
static inline void set_pte(pte_t *ptep, pte_t pte)
{
+ mm_track(ptep); // track the old (departing) page
ptep->pte_high = pte.pte_high;
smp_wmb();
ptep->pte_low = pte.pte_low;
@@ -59,10 +60,16 @@ static inline void set_pte(pte_t *ptep,
#define set_pte_at(mm,addr,ptep,pteval) set_pte(ptep,pteval)
#define __HAVE_ARCH_SET_PTE_ATOMIC
-#define set_pte_atomic(pteptr,pteval) \
- set_64bit((unsigned long long *)(pteptr),pte_val(pteval))
-#define set_pmd(pmdptr,pmdval) \
- set_64bit((unsigned long long *)(pmdptr),pmd_val(pmdval))
+#define set_pte_atomic(pteptr,pteval) \
+({ \
+ mm_track(pteptr); /* track the old page */ \
+ set_64bit((unsigned long long *)(pteptr),pte_val(pteval)); \
+})
+#define set_pmd(pmdptr,pmdval) \
+({ \
+ mm_track(pmdptr); /* track the old page */ \
+ set_64bit((unsigned long long *)(pmdptr),pmd_val(pmdval)); \
+})
#define set_pud(pudptr,pudval) \
set_64bit((unsigned long long *)(pudptr),pud_val(pudval))
@@ -94,6 +101,8 @@ static inline pte_t ptep_get_and_clear(s
{
pte_t res;
+ mm_track(ptep); // track old page before losing this reference
+
/* xchg acts as a barrier before the setting of the high bits */
res.pte_low = xchg(&ptep->pte_low, 0);
res.pte_high = ptep->pte_high;
diff --git a/include/asm-i386/pgtable.h b/include/asm-i386/pgtable.h
--- a/include/asm-i386/pgtable.h
+++ b/include/asm-i386/pgtable.h
@@ -21,6 +21,8 @@
#include <asm/bitops.h>
#endif
+#include <asm/mm_track.h>
+
#include <linux/slab.h>
#include <linux/list.h>
#include <linux/spinlock.h>
@@ -101,7 +103,7 @@ void paging_init(void);
#define _PAGE_BIT_DIRTY 6
#define _PAGE_BIT_PSE 7 /* 4 MB (or 2MB) page, Pentium+, if present.. */
#define _PAGE_BIT_GLOBAL 8 /* Global TLB entry PPro+ */
-#define _PAGE_BIT_UNUSED1 9 /* available for programmer */
+#define _PAGE_BIT_SOFTDIRTY 9 /* save dirty state when hdw dirty bit cleared */
#define _PAGE_BIT_UNUSED2 10
#define _PAGE_BIT_UNUSED3 11
#define _PAGE_BIT_NX 63
@@ -115,7 +117,7 @@ void paging_init(void);
#define _PAGE_DIRTY 0x040
#define _PAGE_PSE 0x080 /* 4 MB (or 2MB) page, Pentium+, if present.. */
#define _PAGE_GLOBAL 0x100 /* Global TLB entry PPro+ */
-#define _PAGE_UNUSED1 0x200 /* available for programmer */
+#define _PAGE_SOFTDIRTY 0x200
#define _PAGE_UNUSED2 0x400
#define _PAGE_UNUSED3 0x800
@@ -129,7 +131,7 @@ void paging_init(void);
#define _PAGE_TABLE (_PAGE_PRESENT | _PAGE_RW | _PAGE_USER | _PAGE_ACCESSED | _PAGE_DIRTY)
#define _KERNPG_TABLE (_PAGE_PRESENT | _PAGE_RW | _PAGE_ACCESSED | _PAGE_DIRTY)
-#define _PAGE_CHG_MASK (PTE_MASK | _PAGE_ACCESSED | _PAGE_DIRTY)
+#define _PAGE_CHG_MASK (PTE_MASK | _PAGE_ACCESSED | _PAGE_SOFTDIRTY | _PAGE_DIRTY)
#define PAGE_NONE \
__pgprot(_PAGE_PROTNONE | _PAGE_ACCESSED)
@@ -206,8 +208,7 @@ extern unsigned long pg0[];
#define pmd_none(x) (!pmd_val(x))
#define pmd_present(x) (pmd_val(x) & _PAGE_PRESENT)
#define pmd_clear(xp) do { set_pmd(xp, __pmd(0)); } while (0)
-#define pmd_bad(x) ((pmd_val(x) & (~PAGE_MASK & ~_PAGE_USER)) != _KERNPG_TABLE)
-
+#define pmd_bad(x) ((pmd_val(x) & (~PAGE_MASK & ~_PAGE_USER & ~_PAGE_SOFTDIRTY & ~_PAGE_ACCESSED)) != (_KERNPG_TABLE & ~_PAGE_ACCESSED))
#define pages_to_mb(x) ((x) >> (20-PAGE_SHIFT))
@@ -217,7 +218,7 @@ extern unsigned long pg0[];
*/
static inline int pte_user(pte_t pte) { return (pte).pte_low & _PAGE_USER; }
static inline int pte_read(pte_t pte) { return (pte).pte_low & _PAGE_USER; }
-static inline int pte_dirty(pte_t pte) { return (pte).pte_low & _PAGE_DIRTY; }
+static inline int pte_dirty(pte_t pte) { return (pte).pte_low & (_PAGE_DIRTY | _PAGE_SOFTDIRTY); }
static inline int pte_young(pte_t pte) { return (pte).pte_low & _PAGE_ACCESSED; }
static inline int pte_write(pte_t pte) { return (pte).pte_low & _PAGE_RW; }
@@ -228,7 +229,7 @@ static inline int pte_file(pte_t pte) {
static inline pte_t pte_rdprotect(pte_t pte) { (pte).pte_low &= ~_PAGE_USER; return pte; }
static inline pte_t pte_exprotect(pte_t pte) { (pte).pte_low &= ~_PAGE_USER; return pte; }
-static inline pte_t pte_mkclean(pte_t pte) { (pte).pte_low &= ~_PAGE_DIRTY; return pte; }
+static inline pte_t pte_mkclean(pte_t pte) { (pte).pte_low &= ~(_PAGE_SOFTDIRTY | _PAGE_DIRTY); return pte; }
static inline pte_t pte_mkold(pte_t pte) { (pte).pte_low &= ~_PAGE_ACCESSED; return pte; }
static inline pte_t pte_wrprotect(pte_t pte) { (pte).pte_low &= ~_PAGE_RW; return pte; }
static inline pte_t pte_mkread(pte_t pte) { (pte).pte_low |= _PAGE_USER; return pte; }
@@ -246,9 +247,9 @@ static inline pte_t pte_mkhuge(pte_t pte
static inline int ptep_test_and_clear_dirty(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
{
- if (!pte_dirty(*ptep))
- return 0;
- return test_and_clear_bit(_PAGE_BIT_DIRTY, &ptep->pte_low);
+ mm_track(ptep);
+ return (test_and_clear_bit(_PAGE_BIT_DIRTY, &ptep->pte_low) |
+ test_and_clear_bit(_PAGE_BIT_SOFTDIRTY, &ptep->pte_low));
}
static inline int ptep_test_and_clear_young(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Dirty page tracking patch
2005-07-11 13:16 Dirty page tracking patch Kimball Murray
@ 2005-07-11 13:35 ` Arjan van de Ven
2005-07-11 18:16 ` Kimball Murray
0 siblings, 1 reply; 4+ messages in thread
From: Arjan van de Ven @ 2005-07-11 13:35 UTC (permalink / raw)
To: Kimball Murray; +Cc: linux-kernel
On Mon, 2005-07-11 at 09:16 -0400, Kimball Murray wrote:
> Hello to all.
>
> On behalf of Stratus Technologies (www.stratus.com) I'd like to
> present a patch to the i386 kernel code that will allow developers to track
> dirty memory pages. Stratus uses this technique to facilitate bringing
> separate cpu and memory module "nodes" into lockstep with each other, with
> a minimum of OS down time. This feature could also be used to provide inputs
> into memory management algorithms, or to support hot-plug memory dimm modules
> for specially developed hardware.
>
is the stratus code entirely open source/GPL ? (I assume it is since you
EXPORT_SYMBOL_GPL and also use other similar stuff). If so.. could you
post an URL to that? It's customary to do so when you post interface
patches for review so that the users of the interfaces can be seen, and
thus the interface better reviewed.
Also this patch is just plain weird/really corner case...
+#define mm_track(ptep)
you have to make that a do { ; } while (0) define as per kernel
convention/need
also you now make set_pte() and co more than trivial assignments, please
convert them to inlines so that typechecking is performed and no double
evaluation of arguments is done!
(this is a real problem for code that would do set_pte(pte++, value) in
a loop or so)
- if (!pte_dirty(*ptep))
- return 0;
- return test_and_clear_bit(_PAGE_BIT_DIRTY, &ptep->pte_low);
+ mm_track(ptep);
+ return (test_and_clear_bit(_PAGE_BIT_DIRTY,
&ptep->pte_low) |
+ test_and_clear_bit(_PAGE_BIT_SOFTDIRTY,
&ptep->pte_low));
}
are you sure you're not introducing a race condition there?
and if you're sure, why do you need 2 atomic ops in sequence?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Dirty page tracking patch
2005-07-11 13:35 ` Arjan van de Ven
@ 2005-07-11 18:16 ` Kimball Murray
2005-07-11 18:28 ` Arjan van de Ven
0 siblings, 1 reply; 4+ messages in thread
From: Kimball Murray @ 2005-07-11 18:16 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2972 bytes --]
Arjan van de Ven wrote:
>On Mon, 2005-07-11 at 09:16 -0400, Kimball Murray wrote:
>
>
>>Hello to all.
>>
>> On behalf of Stratus Technologies (www.stratus.com) I'd like to
>>present a patch to the i386 kernel code that will allow developers to track
>>dirty memory pages. Stratus uses this technique to facilitate bringing
>>separate cpu and memory module "nodes" into lockstep with each other, with
>>a minimum of OS down time. This feature could also be used to provide inputs
>>into memory management algorithms, or to support hot-plug memory dimm modules
>>for specially developed hardware.
>>
>>
>>
>
>is the stratus code entirely open source/GPL ? (I assume it is since you
>EXPORT_SYMBOL_GPL and also use other similar stuff). If so.. could you
>post an URL to that? It's customary to do so when you post interface
>patches for review so that the users of the interfaces can be seen, and
>thus the interface better reviewed.
>
I don't have access to a Stratus web server where I could post the code,
since I'm currently sitting at a Redhat desk :) So if you'll indulge
me, I'll attach the harvest code to this note. The harvest code is one
component of a GPL'd kernel module. The harvest component is the only
one that interacts with the dirty page tracking patch. Other parts of
the module do PCI-hotplug related things that are specific to our platform.
For the purposes of looking at how Stratus uses the harvest and page
tracking, have a look at the harvest and process_vector functions, both
of which are called from the brownout_cycle function. The rest of the
code in the file is mostly scaffolding to support cpu rendezvous
operations, and to make call-outs to routines that copy memory pages.
>Also this patch is just plain weird/really corner case...
>
>+#define mm_track(ptep)
>
>you have to make that a do { ; } while (0) define as per kernel
>convention/need
>
>also you now make set_pte() and co more than trivial assignments, please
>convert them to inlines so that typechecking is performed and no double
>evaluation of arguments is done!
>(this is a real problem for code that would do set_pte(pte++, value) in
>a loop or so)
>
thanks, I'll make those changes and re-build/re-test/re-post.
>- if (!pte_dirty(*ptep))
>- return 0;
>- return test_and_clear_bit(_PAGE_BIT_DIRTY, &ptep->pte_low);
>+ mm_track(ptep);
>+ return (test_and_clear_bit(_PAGE_BIT_DIRTY,
>&ptep->pte_low) |
>+ test_and_clear_bit(_PAGE_BIT_SOFTDIRTY,
>&ptep->pte_low));
> }
>
>
>are you sure you're not introducing a race condition there?
>and if you're sure, why do you need 2 atomic ops in sequence?
>
I think we're OK there. That code only appears in the sys_msync path
(sync_page_range), after the page table lock is taken. The only code
that moves hardware dirty bits to software dirty bits is in the harvest
itself (attached). And that code runs with all other cpus parked.
-kimball
[-- Attachment #2: harvest.c --]
[-- Type: text/plain, Size: 18750 bytes --]
/*
* Low-level routines for memory mirroring. Tracks dirty pages and
* utilizes provided copy routine to transfer pages which have changed
* between harvest passes.
*
* Copyright (C) 2005 Stratus Technologies Bermuda Ltd.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
#include "fosil.h"
#include "syncd.h"
#include "harvest.h"
/*
* The following ifdef allows harvest to be built against a kernel
* which does not implement memory tracking.
*/
#ifdef CONFIG_MEM_MIRROR
#include <linux/interrupt.h>
#include <linux/mm.h>
#include <linux/vmalloc.h>
#include <linux/highmem.h>
#include <linux/version.h>
#include <asm/io.h>
#include <asm/atomic.h>
#include <asm/tlbflush.h>
#define MAX_COPYRANGE 100 /* this is actually defined in cc/host.h */
#define MB (1024*1024)
#define MERGE_MAX_DELTA 100
#ifdef CONFIG_X86_PAE
typedef unsigned long long paddr_t; /* physical address type */
#define PADDR_FMT "%09llx" /* printf format for paddr_t */
#else
typedef unsigned long paddr_t; /* physical address type */
#define PADDR_FMT "%08lx" /* printf format for paddr_t */
#endif
enum {
DBGLVL_NONE = 0x00,
DBGLVL_HARVEST = 0x01,
DBGLVL_BROWNOUT = 0x02,
DBGLVL_TIMER = 0x04,
};
static int dbgmask = DBGLVL_NONE;
#define DBGPRNT(lvl, args...) \
({ \
if (lvl & dbgmask) \
printk(args); \
})
enum {
IN_BROWNOUT = 0,
IN_BLACKOUT = 1,
};
struct syncd_data {
unsigned long flags[NR_CPUS];
copy_proc copy;
sync_proc sync;
void * context;
is_pfn_valid_proc is_pfn_valid;
int status; // return status from command inside blackout
int max_range; // # of valid ranges in following CopyRange
fosil_page_range_t CopyRange[MAX_COPYRANGE];
unsigned long duration_in_ms; // blackout during
unsigned long bitcnt; // number of bits find in last harvest
int blackout; // 1 == blackout, 0 == brownout
int pass; // # of times through harvest/process cycle
int done; // 1 == done - time to leave
unsigned int threshold; // number of dirty pages before sync
};
static void harvest_page(pgd_t * pgd_base, unsigned long addr)
{
// for all pages in this vm_area_struct
pgd_t *pgd;
pmd_t *pmd;
pte_t *pte;
pmd_t newpmd;
#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,12)
pud_t *pud;
#endif
DBGPRNT(DBGLVL_HARVEST, "pgd_base %p + offset %ld\n",
pgd_base, pgd_index(addr));
DBGPRNT(DBGLVL_HARVEST, " pgd[0] %09llx\n", pgd_val(pgd_base[0]));
DBGPRNT(DBGLVL_HARVEST, " pgd[1] %09llx\n", pgd_val(pgd_base[1]));
DBGPRNT(DBGLVL_HARVEST, " pgd[2] %09llx\n", pgd_val(pgd_base[2]));
DBGPRNT(DBGLVL_HARVEST, " pgd[3] %09llx\n", pgd_val(pgd_base[3]));
pgd = pgd_base + pgd_index(addr);
if (pgd_none(*pgd))
return;
if (!pgd_present(*pgd))
return;
#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,12)
pud = pud_offset(pgd, addr);
if (pud_none(*pud))
return;
pmd = pmd_offset(pud, addr);
#else
pmd = pmd_offset(pgd, addr);
#endif
DBGPRNT(DBGLVL_HARVEST, " pmd[%ld] %p is %llx\n",
pmd_index(addr), pmd, pmd_val(*pmd));
if (pmd_none(*pmd))
return;
if (!pmd_present(*pmd))
return;
newpmd = *pmd;
// large pages
#ifdef CONFIG_X86_PAE
if (pmd_large(newpmd)) {
paddr_t i;
paddr_t start, end;
if ((pmd_val(newpmd) & (_PAGE_ACCESSED | _PAGE_DIRTY)) == 0)
return;
if (pmd_val(newpmd) & _PAGE_DIRTY) {
pmd_val(newpmd) &= ~_PAGE_DIRTY;
pmd_val(newpmd) |= _PAGE_SOFTDIRTY;
}
if (pmd_val(newpmd) & _PAGE_ACCESSED) {
pmd_val(newpmd) &= ~_PAGE_ACCESSED;
}
if (pmd_val(*pmd) != pmd_val(newpmd)) {
set_pmd(pmd, newpmd);
// track the modified pmd
if (pmd_val(*pmd) & _PAGE_BIT_GLOBAL)
__flush_tlb_global();
else
__flush_tlb_one(addr);
mm_track(&__pmd(__pa(pmd)));
}
start = pmd_val(newpmd) & ~((1LL<<PMD_SHIFT)-1);
end = start + ((1LL<<PMD_SHIFT) - 1);
for (i = start; i < end; i += PAGE_SIZE) {
mm_track(&i);
}
return;
}
#endif
/*
* We only want to account for the hw access bit that are set in the
* page diretory and page table pages from the user side and not
* for the kernel page table. This is because we copy all the
* page table pages in host_trigger_smi for the kernel space
*
* The fact that we are here implies that we have set the
* accessed bit. So, clear the bit and add it to the list of
* pages to be data moved.
*/
if ((pgd_base != init_mm.pgd) && (pmd_val(newpmd) & _PAGE_ACCESSED)) {
pmd_val(newpmd) &= ~_PAGE_ACCESSED;
}
if (pmd_val(*pmd) != pmd_val(newpmd)) {
set_pmd(pmd, newpmd);
// track the pmd as it was just modified
mm_track(&__pmd(__pa(pmd)));
}
if (addr < PAGE_OFFSET)
pte = pte_offset_map(pmd, addr);
else
pte = pte_offset_kernel(pmd, addr);
DBGPRNT(DBGLVL_HARVEST, " page table %llx\n",
pmd_val(*pmd) & PAGE_MASK);
DBGPRNT(DBGLVL_HARVEST, " pte[%ld] %p is %llx\n",
pte_index(addr), pte, pte_val(*pte));
if (pte_present(*pte)) {
pte_t newpte;
newpte = *pte;
if (pte_val(newpte) & _PAGE_ACCESSED) {
newpte.pte_low &= ~_PAGE_ACCESSED;
}
if (pte_val(newpte) & _PAGE_DIRTY) {
newpte.pte_low &= ~_PAGE_DIRTY;
newpte.pte_low |= _PAGE_SOFTDIRTY;
}
if (pte_val(*pte) != pte_val(newpte)) {
set_pte(pte, newpte);
// Add the page table itself as we just dirtied it
if (pte_val(*pte) & _PAGE_BIT_GLOBAL)
__flush_tlb_global();
else
__flush_tlb_one(addr);
mm_track(pmd);
}
}
DBGPRNT(DBGLVL_HARVEST, "a pte[%ld] %p is %llx\n",
pte_index(addr), pte, pte_val(*pte));
if (addr < PAGE_OFFSET)
pte_unmap(pte);
return;
}
static void harvest_kernel(void)
{
unsigned long addr;
for (addr = PAGE_OFFSET; addr; addr += PAGE_SIZE)
harvest_page(init_mm.pgd, addr);
}
/*
* Find all the hardware dirty bits and move them to soft dirty bits.
* All cpus have checked in to a rendezvous, so we don't need page table
* locks, etc.
*/
static unsigned long harvest(void * __attribute__ ((unused)) unused_arg)
{
struct list_head * m = NULL;
struct mm_struct * mmentry;
struct vm_area_struct * mmap, * vma;
unsigned long addr;
m = &init_mm.mmlist;
do {
mmentry = list_entry(m, struct mm_struct, mmlist);
m = m->next;
vma = mmap = mmentry->mmap;
if (!mmap)
continue;
do {
unsigned long start, end;
if (!vma)
break;
start = vma->vm_start;
end = vma->vm_end;
for (addr = start; addr <= end; addr += PAGE_SIZE)
harvest_page(mmentry->pgd, addr);
} while ((vma = vma->vm_next) != mmap);
} while (m != &init_mm.mmlist);
harvest_kernel();
return 0;
}
#define VIRT_TO_PFN(pfn) (virt_to_phys(pfn) >> PAGE_SHIFT)
/*
* Look for set bits in the mm_tracking_struct bit vector.
* Each bit set is a page that we need to copy from online to
* offline board.
*/
static void process_vector(void * arg)
{
struct syncd_data * x = (struct syncd_data *) arg;
unsigned long pfn;
unsigned long start_pfn, runlength;
// traverse the the bitmap looking for dirty pages
runlength = 0;
start_pfn = 0;
for (pfn = 0; pfn < mm_tracking_struct.bitcnt; pfn++) {
if (test_and_clear_bit(pfn, mm_tracking_struct.vector) &&
(x->is_pfn_valid(pfn))) {
atomic_dec(&mm_tracking_struct.count);
if (runlength == 0) {
start_pfn = pfn;
}
runlength++;
} else if (runlength) {
x->status = x->copy(start_pfn, runlength,
x->blackout, x->context);
if (x->status != FOSIL_ERR_OK) {
printk("%s: %d\n", __FUNCTION__, __LINE__);
return;
}
start_pfn = runlength = 0;
}
}
}
static void enable_tracking(void)
{
// initialize tracking structure
atomic_set(&mm_tracking_struct.count, 0);
memset(mm_tracking_struct.vector, 0, mm_tracking_struct.bitcnt/8);
// turn on tracking
atomic_set(&mm_tracking_struct.active, 1);
}
static void disable_tracking(void)
{
// turn off tracking
atomic_set(&mm_tracking_struct.active, 0);
}
/*
* Merge adjacent copy ranges. Ranges must be have a gap no larger than
* MERGE_MAX_DELTA between them in order to be considered for a merge.
*/
static void merge_ranges(struct syncd_data * x)
{
int delta;
int i, ii;
for (i = 0; i < x->max_range-1; i++) {
delta = x->CopyRange[i+1].start_page -
(x->CopyRange[i].start_page +
x->CopyRange[i].num_pages);
// don't merge when the gap is too large
if (delta > MERGE_MAX_DELTA)
continue;
x->CopyRange[i].num_pages += x->CopyRange[i+1].num_pages;
x->CopyRange[i].num_pages += delta;
// compress out the merged range
for (ii = i+1; ii < x->max_range-1; ii++) {
x->CopyRange[ii].start_page =
x->CopyRange[ii+1].start_page;
x->CopyRange[ii].num_pages =
x->CopyRange[ii+1].num_pages;
}
x->max_range--;
x->CopyRange[x->max_range].start_page = 0;
x->CopyRange[x->max_range].num_pages = 0;
}
}
/*
* Insert a pfn into the copy range list, adding to an existing range
* if the new pfn is adjacent.
*/
static int insert_pfn(struct syncd_data * x, unsigned long long val)
{
int i;
if (x->max_range == 0)
goto insert;
// can val be pre/appended to a range?
for (i = 0; i < x->max_range; i++) {
if ((val >= x->CopyRange[i].start_page)
&& ((x->CopyRange[i].start_page +
x->CopyRange[i].num_pages - 1) >= val)) {
return 0;
}
if ((x->CopyRange[i].start_page - val) == 1) {
x->CopyRange[i].start_page = val;
x->CopyRange[i].num_pages++;
merge_ranges(x);
return 0;
}
if ((val - (x->CopyRange[i].start_page +
(x->CopyRange[i].num_pages - 1))) == 1) {
x->CopyRange[i].num_pages++;
merge_ranges(x);
return 0;
}
}
// make a new range
insert:
if (x->max_range == MAX_COPYRANGE)
return 1;
// insert into list, keeping ordered by start
for (i = 0; i < x->max_range; i++) {
if (val < x->CopyRange[i].start_page) {
int ii;
// shift everybody up one
for (ii = x->max_range+1; ii > i; ii--) {
x->CopyRange[ii].start_page =
x->CopyRange[ii-1].start_page;
x->CopyRange[ii].num_pages =
x->CopyRange[ii-1].num_pages;
}
x->CopyRange[i].start_page = val;
x->CopyRange[i].num_pages = 1;
x->max_range++;
return 0;
}
}
x->CopyRange[x->max_range].start_page = val;
x->CopyRange[x->max_range].num_pages = 1;
x->max_range++;
return 0;
}
static unsigned long pmd_pfn(pmd_t pmd)
{
return (pmd.pmd >> PAGE_SHIFT);
}
/*
* With a fundamental knowledge of what absolutely MUST be copied
* by the HOST during blackout, add pfn's to the copy range list.
*
* All page tables are added to avoid having to deal with optimizing
* which ptes might be set dirty or accessed by the last few operations
* before blackout.
*
* After page tables, the remainder of the pages are our known "working
* set". This includes the stack frames for this task and processor and
* the syncd structure (which we are making dirty by compiling this list).
*
* Finally, all pages marked as dirty in their pte's are added to the
* list. These are pages which escaped the harvest/process cycles and
* were presumably dirtied between harvest and here.
*/
static int setup_blackout_copylist(struct syncd_data * x)
{
pgd_t * pgd;
pmd_t * pmd;
pte_t * pte;
unsigned long addr;
void * ptr;
#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,12)
pud_t *pud;
#endif
x->max_range = 0;
// Gather up the kernel page tables (> PAGE_OFFSET to 4GB)
for (addr = PAGE_OFFSET; addr; addr += PAGE_SIZE) {
pgd = init_mm.pgd + pgd_index(addr);
if (pgd_none(*pgd) || !pgd_present(*pgd))
continue;
#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,12)
#if 1
pud = pud_offset(pgd, addr);
pmd = pmd_offset(pud, addr);
#else
pmd = (pmd_t*)pgd + pmd_index(addr);
#endif
#else
pmd = pmd_offset(pgd, addr);
#endif
if (pmd_none(*pmd) || !pmd_present(*pmd))
continue;
if (pmd_large(*pmd)) {
paddr_t pfn, start, end;
if ((pmd_val(*pmd) & _PAGE_DIRTY) == 0)
continue;
if (insert_pfn(x, pmd_pfn(*pmd)))
return FOSIL_ERR_MERGE_FAIL;
if (!x->is_pfn_valid(pmd_pfn(*pmd)))
continue;
start = pmd_val(*pmd) & ~((1LL<<PMD_SHIFT)-1);
end = start & ((1LL<<PMD_SHIFT)-1);
printk("KAM: pmd large!\n");
BUG();
for (pfn = start; pfn < end; pfn += PAGE_SIZE)
if (insert_pfn(x, pfn))
return FOSIL_ERR_MERGE_FAIL;
continue;
}
if (insert_pfn(x, pmd_pfn(*pmd)))
return FOSIL_ERR_MERGE_FAIL;
pte = pte_offset_kernel(pmd, addr);
// add the dirty pages we find
if (pte_present(*pte) && (pte_val(*pte) & _PAGE_DIRTY))
if (x->is_pfn_valid(pte_pfn(*pte)))
if (insert_pfn(x, pte_pfn(*pte)))
return FOSIL_ERR_MERGE_FAIL;
}
// copy the stack we are working from
ptr = current;
if (insert_pfn(x, virt_to_phys(ptr) >> PAGE_SHIFT))
return FOSIL_ERR_MERGE_FAIL;
ptr += PAGE_SIZE;
if (insert_pfn(x, virt_to_phys(ptr) >> PAGE_SHIFT))
return FOSIL_ERR_MERGE_FAIL;
// add syncd structure
if (insert_pfn(x, virt_to_phys(x) >> PAGE_SHIFT))
return FOSIL_ERR_MERGE_FAIL;
// compact the list some more?
merge_ranges(x);
return FOSIL_ERR_OK;
}
static void __attribute__ ((__unused__)) do_blackout(void * arg)
{
struct syncd_data * x = (struct syncd_data *) arg;
fosil_page_range_t ranges[1] = {{0, ~0}};
x->status = x->sync(1, ranges, &x->duration_in_ms, x->context);
}
/*
* This is where all processors are sent for blackout processing.
* The driving processor is diverted to work on harvest and if necessary
* entry to Common Code for synchronization. All other processors are
* spinning in a rendezvous at the bottom of this routine waiting for
* the driving processor to finish.
*/
static unsigned long brownout_cycle(void * arg)
{
struct syncd_data * x = (struct syncd_data *) arg;
unsigned long long before, after;
unsigned long delta;
rdtscll(before);
harvest(NULL);
x->bitcnt = atomic_read(&mm_tracking_struct.count);
if ((x->bitcnt < x->threshold) || (x->pass == 0)) {
int err;
x->blackout = IN_BLACKOUT;
disable_tracking();
rdtscll(after);
process_vector(x);
if (x->status != FOSIL_ERR_OK)
return 0;
err = setup_blackout_copylist(x);
if (err) {
x->status = err;
return 0;
}
// Call function to copy pages from offline to online board.
x->status = x->sync(x->max_range,
x->CopyRange,
&x->duration_in_ms, x->context);
x->done = 1;
if (x->status)
printk("%s: %d (%s)\n", __FUNCTION__, __LINE__,
FOSIL_ERR_STRING(x->status));
delta = after - before;
return x->duration_in_ms + (delta / cpu_khz);
}
return 0;
}
struct blackout_data {
atomic_t r[4];
unsigned int oncpu;
unsigned long (*func)(void *);
void * arg;
};
static void fosil_blackout(void * arg)
{
struct blackout_data * x = (struct blackout_data *) arg;
unsigned long flags;
unsigned long long before, after;
local_bh_disable();
rendezvous(&x->r[0]);
local_irq_save(flags);
rendezvous(&x->r[1]);
if (smp_processor_id() == x->oncpu)
x->func(x->arg);
rendezvous(&x->r[2]);
__flush_tlb_all();
rendezvous(&x->r[3]);
local_irq_restore(flags);
local_bh_enable();
}
/*
* Call the specified function with the provide argument in
* during a blackout. All blackout sequences for harvest
* result in a flush_tlb_all for the captive processors.
*/
void fosil_exec_onall(unsigned long (*func)(void *), void * arg)
{
struct blackout_data x;
init_rendezvous(&x.r[0]);
init_rendezvous(&x.r[1]);
init_rendezvous(&x.r[2]);
init_rendezvous(&x.r[3]);
x.oncpu = smp_processor_id();
x.func = func;
x.arg = arg;
fosil_syncd_start(SYNCD_ON_ALL_PROCESSORS, fosil_blackout, &x, 0);
fosil_syncd_finish();
}
/*
* This routine is responsible for driving all of the memory
* synchronization processing. When complete the sync function should
* be called to enter blackout and finalize synchronization.
*/
int fosil_synchronize_memory(copy_proc copy,
sync_proc sync,
is_pfn_valid_proc is_pfn_valid,
unsigned int threshold,
unsigned int passes,
void * context)
{
int status;
struct syncd_data * x;
x = kmalloc(sizeof(struct syncd_data), GFP_KERNEL);
if (x == NULL) {
printk("unable to allocate syncd control structure\n");
return 1;
}
memset(x, 0, sizeof(struct syncd_data));
x->copy = copy;
x->sync = sync;
x->is_pfn_valid = is_pfn_valid;
x->bitcnt = 0;
x->blackout = IN_BROWNOUT;
x->done = 0;
x->threshold = threshold;
x->context = context;
// clean up hardware dirty bits
fosil_exec_onall(harvest, NULL);
enable_tracking();
/*
* Copy all memory. After this we only copy modified
* pages, so we need to make sure the unchanged parts
* get copied at least once.
*/
status = x->copy(0, ~0, 0, x->context);
if (status != FOSIL_ERR_OK) {
printk("%s: %d\n", __FUNCTION__, __LINE__);
return status;
}
for (x->pass = passes; (x->pass >= 0); x->pass--) {
DBGPRNT(DBGLVL_BROWNOUT,
"brownout pass %d - %ld pages found (%d)\n",
x->pass, x->bitcnt,
atomic_read(&mm_tracking_struct.count));
fosil_exec_onall(brownout_cycle, x);
if (x->done)
break;
process_vector(x);
if (x->status) {
printk("%s: %d\n", __FUNCTION__, __LINE__);
break;
}
}
disable_tracking();
DBGPRNT(DBGLVL_BROWNOUT, "last pass found %ld pages\n", x->bitcnt);
DBGPRNT(DBGLVL_BROWNOUT, "copy range max %d\n", x->max_range);
status = x->status;
kfree(x);
return status;
}
EXPORT_SYMBOL(fosil_synchronize_memory);
int __init fosil_harvest_init(void)
{
extern unsigned long num_physpages;
mm_tracking_struct.vector = vmalloc(num_physpages/8);
mm_tracking_struct.bitcnt = num_physpages;
if (mm_tracking_struct.vector == NULL) {
printk("%s: unable to allocate memory tracking bitmap\n",
__FUNCTION__);
return -ENOMEM;
}
return 0;
}
void __exit fosil_harvest_cleanup(void)
{
// make sure tracking is definitely turned off
disable_tracking();
// wake me after everyone is sure to be done with mm_track()
synchronize_kernel();
if (mm_tracking_struct.vector)
vfree(mm_tracking_struct.vector);
mm_tracking_struct.bitcnt = 0;
return;
}
#else /* CONFIG_MEM_MIRROR */
int __init fosil_harvest_init(void)
{
return 0;
}
void __exit fosil_harvest_cleanup(void)
{
return;
}
#endif /* CONFIG_MEM_MIRROR */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Dirty page tracking patch
2005-07-11 18:16 ` Kimball Murray
@ 2005-07-11 18:28 ` Arjan van de Ven
0 siblings, 0 replies; 4+ messages in thread
From: Arjan van de Ven @ 2005-07-11 18:28 UTC (permalink / raw)
To: Kimball Murray; +Cc: linux-kernel
> >
> >is the stratus code entirely open source/GPL ? (I assume it is since you
> >EXPORT_SYMBOL_GPL and also use other similar stuff). If so.. could you
> >post an URL to that? It's customary to do so when you post interface
> >patches for review so that the users of the interfaces can be seen, and
> >thus the interface better reviewed.
> >
> I don't have access to a Stratus web server where I could post the code,
> since I'm currently sitting at a Redhat desk :) So if you'll indulge
> me, I'll attach the harvest code to this note. The harvest code is one
> component of a GPL'd kernel module. The harvest component is the only
> one that interacts with the dirty page tracking patch. Other parts of
> the module do PCI-hotplug related things that are specific to our platform.
ok so your entire kernel side stack is GPL? Good.
> >
> >+#define mm_track(ptep)
> >
> >you have to make that a do { ; } while (0) define as per kernel
> >convention/need
> >
> >also you now make set_pte() and co more than trivial assignments, please
> >convert them to inlines so that typechecking is performed and no double
> >evaluation of arguments is done!
> >(this is a real problem for code that would do set_pte(pte++, value) in
> >a loop or so)
> >
> thanks, I'll make those changes and re-build/re-test/re-post.
>
> >- if (!pte_dirty(*ptep))
> >- return 0;
> >- return test_and_clear_bit(_PAGE_BIT_DIRTY, &ptep->pte_low);
> >+ mm_track(ptep);
> >+ return (test_and_clear_bit(_PAGE_BIT_DIRTY,
> >&ptep->pte_low) |
> >+ test_and_clear_bit(_PAGE_BIT_SOFTDIRTY,
> >&ptep->pte_low));
> > }
> >
> >
> >are you sure you're not introducing a race condition there?
> >and if you're sure, why do you need 2 atomic ops in sequence?
> >
> I think we're OK there. That code only appears in the sys_msync path
> (sync_page_range), after the page table lock is taken. The only code
> that moves hardware dirty bits to software dirty bits is in the harvest
> itself (attached). And that code runs with all other cpus parked.
then you didn't answer my question about why the lock; atomic bit is
needed ;)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2005-07-11 18:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-07-11 13:16 Dirty page tracking patch Kimball Murray
2005-07-11 13:35 ` Arjan van de Ven
2005-07-11 18:16 ` Kimball Murray
2005-07-11 18:28 ` Arjan van de Ven
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.