Re: [PATCH] mm counter operations through macros
On Mon, 14 Mar 2005, Andrew Morton wrote: > > Then you wont be able to get rid of the counters by > > > > #define MM_COUNTER(xx) > > > > anymore. > > Why would we want to do that? If counters are calculated on demand then no counter is necessary. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
Christoph Lameter <[EMAIL PROTECTED]> wrote: > > On Mon, 14 Mar 2005, Andrew Morton wrote: > > > I don't think the MM_COUNTER_T macro adds much, really. How about this? > > Then you wont be able to get rid of the counters by > > #define MM_COUNTER(xx) > > anymore. Why would we want to do that? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
On Mon, 14 Mar 2005, Andrew Morton wrote:
> I don't think the MM_COUNTER_T macro adds much, really. How about this?
Then you wont be able to get rid of the counters by
#define MM_COUNTER(xx)
anymore.
>
> --- 25/include/linux/sched.h~mm-counter-operations-through-macros-tidy
> 2005-03-14 21:43:00.0 -0800
> +++ 25-akpm/include/linux/sched.h 2005-03-14 21:43:00.0 -0800
> @@ -210,7 +210,6 @@ extern void arch_unmap_area_topdown(stru
> #define inc_mm_counter(mm, member) (mm)->_##member++
> #define dec_mm_counter(mm, member) (mm)->_##member--
> typedef unsigned long mm_counter_t;
> -#define MM_COUNTER_T(member) mm_counter_t _##member
>
> struct mm_struct {
> struct vm_area_struct * mmap; /* list of VMAs */
> @@ -241,8 +240,8 @@ struct mm_struct {
> unsigned long exec_vm, stack_vm, reserved_vm, def_flags, nr_ptes;
>
> /* Special counters protected by the page_table_lock */
> - MM_COUNTER_T(rss);
> - MM_COUNTER_T(anon_rss);
> + mm_counter_t _rss;
> + mm_counter_t _anon_rss;
>
> unsigned long saved_auxv[42]; /* for /proc/PID/auxv */
>
> _
>
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
Christoph Lameter <[EMAIL PROTECTED]> wrote:
>
> This patch extracts all the operations on counters protected by the
> page table lock (currently rss and anon_rss) into definitions in
> include/linux/sched.h. All rss operations are performed through
> the following macros:
I don't think the MM_COUNTER_T macro adds much, really. How about this?
--- 25/include/linux/sched.h~mm-counter-operations-through-macros-tidy
2005-03-14 21:43:00.0 -0800
+++ 25-akpm/include/linux/sched.h 2005-03-14 21:43:00.0 -0800
@@ -210,7 +210,6 @@ extern void arch_unmap_area_topdown(stru
#define inc_mm_counter(mm, member) (mm)->_##member++
#define dec_mm_counter(mm, member) (mm)->_##member--
typedef unsigned long mm_counter_t;
-#define MM_COUNTER_T(member) mm_counter_t _##member
struct mm_struct {
struct vm_area_struct * mmap; /* list of VMAs */
@@ -241,8 +240,8 @@ struct mm_struct {
unsigned long exec_vm, stack_vm, reserved_vm, def_flags, nr_ptes;
/* Special counters protected by the page_table_lock */
- MM_COUNTER_T(rss);
- MM_COUNTER_T(anon_rss);
+ mm_counter_t _rss;
+ mm_counter_t _anon_rss;
unsigned long saved_auxv[42]; /* for /proc/PID/auxv */
_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
Ok. Here is an updated patch:
This patch extracts all the operations on counters protected by the
page table lock (currently rss and anon_rss) into definitions in
include/linux/sched.h. All rss operations are performed through
the following macros:
get_mm_counter(mm, member) -> Obtain the value of a counter
set_mm_counter(mm, member, value) -> Set the value of a counter
add_mm_counter(mm, member, value) -> Add to a counter
inc_mm_counter(mm, member) -> Increment a counter
dec_mm_counter(mm, member) -> Decrement a counter
With this patch it becomes easier to add new counters and it is possible
to redefine the method of counter handling. The counters are an
issue for scalability since they are used in frequently used code paths and
may cause cache line bouncing.
F.e. One may not use counters at all and count the pages when needed, switch
to atomic operations if the mm_struct locking changes or split the rss
into counters that can be locally incremented.
Signed-off-by: Christoph Lameter <[EMAIL PROTECTED]>
Index: linux-2.6.11/include/linux/sched.h
===
--- linux-2.6.11.orig/include/linux/sched.h 2005-03-11 10:29:17.0
-0800
+++ linux-2.6.11/include/linux/sched.h 2005-03-14 20:54:51.0 -0800
@@ -205,6 +205,13 @@ arch_get_unmapped_area_topdown(struct fi
extern void arch_unmap_area(struct vm_area_struct *area);
extern void arch_unmap_area_topdown(struct vm_area_struct *area);
+#define set_mm_counter(mm, member, value) (mm)->_##member = (value)
+#define get_mm_counter(mm, member) ((mm)->_##member)
+#define add_mm_counter(mm, member, value) (mm)->_##member += (value)
+#define inc_mm_counter(mm, member) (mm)->_##member++
+#define dec_mm_counter(mm, member) (mm)->_##member--
+typedef unsigned long mm_counter_t;
+#define MM_COUNTER_T(member) mm_counter_t _##member
struct mm_struct {
struct vm_area_struct * mmap; /* list of VMAs */
@@ -221,7 +228,7 @@ struct mm_struct {
atomic_t mm_count; /* How many references to
"struct mm_struct" (users count as 1) */
int map_count; /* number of VMAs */
struct rw_semaphore mmap_sem;
- spinlock_t page_table_lock; /* Protects page tables,
mm->rss, mm->anon_rss */
+ spinlock_t page_table_lock; /* Protects page tables and
some counters */
struct list_head mmlist;/* List of maybe swapped mm's.
These are globally strung
* together off init_mm.mmlist,
and are protected
@@ -231,9 +238,13 @@ struct mm_struct {
unsigned long start_code, end_code, start_data, end_data;
unsigned long start_brk, brk, start_stack;
unsigned long arg_start, arg_end, env_start, env_end;
- unsigned long rss, anon_rss, total_vm, locked_vm, shared_vm;
+ unsigned long total_vm, locked_vm, shared_vm;
unsigned long exec_vm, stack_vm, reserved_vm, def_flags, nr_ptes;
+ /* Special counters protected by the page_table_lock */
+ MM_COUNTER_T(rss);
+ MM_COUNTER_T(anon_rss);
+
unsigned long saved_auxv[42]; /* for /proc/PID/auxv */
unsigned dumpable:1;
Index: linux-2.6.11/mm/memory.c
===
--- linux-2.6.11.orig/mm/memory.c 2005-03-11 10:29:17.0 -0800
+++ linux-2.6.11/mm/memory.c2005-03-14 20:50:20.0 -0800
@@ -312,9 +312,9 @@ copy_one_pte(struct mm_struct *dst_mm,
pte = pte_mkclean(pte);
pte = pte_mkold(pte);
get_page(page);
- dst_mm->rss++;
+ inc_mm_counter(dst_mm, rss);
if (PageAnon(page))
- dst_mm->anon_rss++;
+ inc_mm_counter(dst_mm, anon_rss);
set_pte_at(dst_mm, addr, dst_pte, pte);
page_dup_rmap(page);
}
@@ -525,7 +525,7 @@ static void zap_pte_range(struct mmu_gat
if (pte_dirty(pte))
set_page_dirty(page);
if (PageAnon(page))
- tlb->mm->anon_rss--;
+ dec_mm_counter(tlb->mm, anon_rss);
else if (pte_young(pte))
mark_page_accessed(page);
tlb->freed++;
@@ -1351,9 +1351,9 @@ static int do_wp_page(struct mm_struct *
page_table = pte_offset_map(pmd, address);
if (likely(pte_same(*page_table, pte))) {
if (PageAnon(old_page))
- mm->anon_rss--;
+ dec_mm_counter(mm, anon_rss);
if (PageReserved(old_page))
- ++mm->rss;
+ inc_mm_counter(mm, rss);
else
page_remove_rmap(old_page);
flush_cache_page(vma, address, pfn);
@@ -1759,7
Re: [PATCH] mm counter operations through macros
Ok. Here is an updated patch:
This patch extracts all the operations on counters protected by the
page table lock (currently rss and anon_rss) into definitions in
include/linux/sched.h. All rss operations are performed through
the following macros:
get_mm_counter(mm, member) - Obtain the value of a counter
set_mm_counter(mm, member, value) - Set the value of a counter
add_mm_counter(mm, member, value) - Add to a counter
inc_mm_counter(mm, member) - Increment a counter
dec_mm_counter(mm, member) - Decrement a counter
With this patch it becomes easier to add new counters and it is possible
to redefine the method of counter handling. The counters are an
issue for scalability since they are used in frequently used code paths and
may cause cache line bouncing.
F.e. One may not use counters at all and count the pages when needed, switch
to atomic operations if the mm_struct locking changes or split the rss
into counters that can be locally incremented.
Signed-off-by: Christoph Lameter [EMAIL PROTECTED]
Index: linux-2.6.11/include/linux/sched.h
===
--- linux-2.6.11.orig/include/linux/sched.h 2005-03-11 10:29:17.0
-0800
+++ linux-2.6.11/include/linux/sched.h 2005-03-14 20:54:51.0 -0800
@@ -205,6 +205,13 @@ arch_get_unmapped_area_topdown(struct fi
extern void arch_unmap_area(struct vm_area_struct *area);
extern void arch_unmap_area_topdown(struct vm_area_struct *area);
+#define set_mm_counter(mm, member, value) (mm)-_##member = (value)
+#define get_mm_counter(mm, member) ((mm)-_##member)
+#define add_mm_counter(mm, member, value) (mm)-_##member += (value)
+#define inc_mm_counter(mm, member) (mm)-_##member++
+#define dec_mm_counter(mm, member) (mm)-_##member--
+typedef unsigned long mm_counter_t;
+#define MM_COUNTER_T(member) mm_counter_t _##member
struct mm_struct {
struct vm_area_struct * mmap; /* list of VMAs */
@@ -221,7 +228,7 @@ struct mm_struct {
atomic_t mm_count; /* How many references to
struct mm_struct (users count as 1) */
int map_count; /* number of VMAs */
struct rw_semaphore mmap_sem;
- spinlock_t page_table_lock; /* Protects page tables,
mm-rss, mm-anon_rss */
+ spinlock_t page_table_lock; /* Protects page tables and
some counters */
struct list_head mmlist;/* List of maybe swapped mm's.
These are globally strung
* together off init_mm.mmlist,
and are protected
@@ -231,9 +238,13 @@ struct mm_struct {
unsigned long start_code, end_code, start_data, end_data;
unsigned long start_brk, brk, start_stack;
unsigned long arg_start, arg_end, env_start, env_end;
- unsigned long rss, anon_rss, total_vm, locked_vm, shared_vm;
+ unsigned long total_vm, locked_vm, shared_vm;
unsigned long exec_vm, stack_vm, reserved_vm, def_flags, nr_ptes;
+ /* Special counters protected by the page_table_lock */
+ MM_COUNTER_T(rss);
+ MM_COUNTER_T(anon_rss);
+
unsigned long saved_auxv[42]; /* for /proc/PID/auxv */
unsigned dumpable:1;
Index: linux-2.6.11/mm/memory.c
===
--- linux-2.6.11.orig/mm/memory.c 2005-03-11 10:29:17.0 -0800
+++ linux-2.6.11/mm/memory.c2005-03-14 20:50:20.0 -0800
@@ -312,9 +312,9 @@ copy_one_pte(struct mm_struct *dst_mm,
pte = pte_mkclean(pte);
pte = pte_mkold(pte);
get_page(page);
- dst_mm-rss++;
+ inc_mm_counter(dst_mm, rss);
if (PageAnon(page))
- dst_mm-anon_rss++;
+ inc_mm_counter(dst_mm, anon_rss);
set_pte_at(dst_mm, addr, dst_pte, pte);
page_dup_rmap(page);
}
@@ -525,7 +525,7 @@ static void zap_pte_range(struct mmu_gat
if (pte_dirty(pte))
set_page_dirty(page);
if (PageAnon(page))
- tlb-mm-anon_rss--;
+ dec_mm_counter(tlb-mm, anon_rss);
else if (pte_young(pte))
mark_page_accessed(page);
tlb-freed++;
@@ -1351,9 +1351,9 @@ static int do_wp_page(struct mm_struct *
page_table = pte_offset_map(pmd, address);
if (likely(pte_same(*page_table, pte))) {
if (PageAnon(old_page))
- mm-anon_rss--;
+ dec_mm_counter(mm, anon_rss);
if (PageReserved(old_page))
- ++mm-rss;
+ inc_mm_counter(mm, rss);
else
page_remove_rmap(old_page);
flush_cache_page(vma, address, pfn);
@@ -1759,7 +1759,7 @@ static int
Re: [PATCH] mm counter operations through macros
Christoph Lameter [EMAIL PROTECTED] wrote:
This patch extracts all the operations on counters protected by the
page table lock (currently rss and anon_rss) into definitions in
include/linux/sched.h. All rss operations are performed through
the following macros:
I don't think the MM_COUNTER_T macro adds much, really. How about this?
--- 25/include/linux/sched.h~mm-counter-operations-through-macros-tidy
2005-03-14 21:43:00.0 -0800
+++ 25-akpm/include/linux/sched.h 2005-03-14 21:43:00.0 -0800
@@ -210,7 +210,6 @@ extern void arch_unmap_area_topdown(stru
#define inc_mm_counter(mm, member) (mm)-_##member++
#define dec_mm_counter(mm, member) (mm)-_##member--
typedef unsigned long mm_counter_t;
-#define MM_COUNTER_T(member) mm_counter_t _##member
struct mm_struct {
struct vm_area_struct * mmap; /* list of VMAs */
@@ -241,8 +240,8 @@ struct mm_struct {
unsigned long exec_vm, stack_vm, reserved_vm, def_flags, nr_ptes;
/* Special counters protected by the page_table_lock */
- MM_COUNTER_T(rss);
- MM_COUNTER_T(anon_rss);
+ mm_counter_t _rss;
+ mm_counter_t _anon_rss;
unsigned long saved_auxv[42]; /* for /proc/PID/auxv */
_
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
On Mon, 14 Mar 2005, Andrew Morton wrote:
I don't think the MM_COUNTER_T macro adds much, really. How about this?
Then you wont be able to get rid of the counters by
#define MM_COUNTER(xx)
anymore.
--- 25/include/linux/sched.h~mm-counter-operations-through-macros-tidy
2005-03-14 21:43:00.0 -0800
+++ 25-akpm/include/linux/sched.h 2005-03-14 21:43:00.0 -0800
@@ -210,7 +210,6 @@ extern void arch_unmap_area_topdown(stru
#define inc_mm_counter(mm, member) (mm)-_##member++
#define dec_mm_counter(mm, member) (mm)-_##member--
typedef unsigned long mm_counter_t;
-#define MM_COUNTER_T(member) mm_counter_t _##member
struct mm_struct {
struct vm_area_struct * mmap; /* list of VMAs */
@@ -241,8 +240,8 @@ struct mm_struct {
unsigned long exec_vm, stack_vm, reserved_vm, def_flags, nr_ptes;
/* Special counters protected by the page_table_lock */
- MM_COUNTER_T(rss);
- MM_COUNTER_T(anon_rss);
+ mm_counter_t _rss;
+ mm_counter_t _anon_rss;
unsigned long saved_auxv[42]; /* for /proc/PID/auxv */
_
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
Christoph Lameter [EMAIL PROTECTED] wrote: On Mon, 14 Mar 2005, Andrew Morton wrote: I don't think the MM_COUNTER_T macro adds much, really. How about this? Then you wont be able to get rid of the counters by #define MM_COUNTER(xx) anymore. Why would we want to do that? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
On Mon, 14 Mar 2005, Andrew Morton wrote: Then you wont be able to get rid of the counters by #define MM_COUNTER(xx) anymore. Why would we want to do that? If counters are calculated on demand then no counter is necessary. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
Christoph Lameter writes:
> On Fri, 11 Mar 2005, Dave Jones wrote:
>
> > Splitting this last one into inc_mm_counter() and dec_mm_counter()
> > means you can kill off the last argument, and get some of the
> > readability back. As it stands, I think this patch adds a bunch
> > of obfuscation for no clear benefit.
>
> Ok.
> -
> This patch extracts all the operations on counters protected by the
> page table lock (currently rss and anon_rss) into definitions in
> include/linux/sched.h. All rss operations are performed through
> the following macros:
>
> get_mm_counter(mm, member) -> Obtain the value of a counter
> set_mm_counter(mm, member, value)-> Set the value of a counter
> update_mm_counter(mm, member, value) -> Add to a counter
A nitpick, but wouldn't be it clearer to call it add_mm_counter()? As an
additional bonus this matches atomic_{inc,dec,add}() and makes macro
names more uniform.
> inc_mm_counter(mm, member) -> Increment a counter
> dec_mm_counter(mm, member) -> Decrement a counter
Nikita.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
Christoph Lameter writes:
On Fri, 11 Mar 2005, Dave Jones wrote:
Splitting this last one into inc_mm_counter() and dec_mm_counter()
means you can kill off the last argument, and get some of the
readability back. As it stands, I think this patch adds a bunch
of obfuscation for no clear benefit.
Ok.
-
This patch extracts all the operations on counters protected by the
page table lock (currently rss and anon_rss) into definitions in
include/linux/sched.h. All rss operations are performed through
the following macros:
get_mm_counter(mm, member) - Obtain the value of a counter
set_mm_counter(mm, member, value)- Set the value of a counter
update_mm_counter(mm, member, value) - Add to a counter
A nitpick, but wouldn't be it clearer to call it add_mm_counter()? As an
additional bonus this matches atomic_{inc,dec,add}() and makes macro
names more uniform.
inc_mm_counter(mm, member) - Increment a counter
dec_mm_counter(mm, member) - Decrement a counter
Nikita.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
On Fri, 11 Mar 2005, Andrew Morton wrote:
> > +#define set_mm_counter(mm, member, value) (mm)->member = (value)
> > +#define get_mm_counter(mm, member) ((mm)->member)
> > +#define update_mm_counter(mm, member, value) (mm)->member += (value)
> > +#define inc_mm_counter(mm, member) (mm)->member++
> > +#define dec_mm_counter(mm, member) (mm)->member--
> > +#define MM_COUNTER_T unsigned long
>
> Would prefer `mm_counter_t' here.
>
> Why not a typedef?
Ok typedef it is.
> > @@ -231,9 +237,13 @@ struct mm_struct {
> > unsigned long start_code, end_code, start_data, end_data;
> > unsigned long start_brk, brk, start_stack;
> > unsigned long arg_start, arg_end, env_start, env_end;
> > - unsigned long rss, anon_rss, total_vm, locked_vm, shared_vm;
> > + unsigned long total_vm, locked_vm, shared_vm;
> > unsigned long exec_vm, stack_vm, reserved_vm, def_flags, nr_ptes;
> >
> > + /* Special counters protected by the page_table_lock */
> > + MM_COUNTER_T rss;
> > + MM_COUNTER_T anon_rss;
> > +
>
> Why were only two counters converted?
Because only these two counters are protected by the page table lock.
> Could I suggest that you rename all these counters, so that code which
> fails to use the macros won't compile?
Ok. will make them mm_xx as they were in previous patches.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
Christoph Lameter <[EMAIL PROTECTED]> wrote:
>
> This patch extracts all the operations on counters protected by the
> page table lock (currently rss and anon_rss) into definitions in
> include/linux/sched.h. All rss operations are performed through
> the following macros:
>
> get_mm_counter(mm, member)-> Obtain the value of a counter
> set_mm_counter(mm, member, value) -> Set the value of a counter
> update_mm_counter(mm, member, value) -> Add to a counter
> inc_mm_counter(mm, member)-> Increment a counter
> dec_mm_counter(mm, member)-> Decrement a counter
I spose it makes sense, if we'll be making scalability changes in there.
>
> +#define set_mm_counter(mm, member, value) (mm)->member = (value)
> +#define get_mm_counter(mm, member) ((mm)->member)
> +#define update_mm_counter(mm, member, value) (mm)->member += (value)
> +#define inc_mm_counter(mm, member) (mm)->member++
> +#define dec_mm_counter(mm, member) (mm)->member--
> +#define MM_COUNTER_T unsigned long
Would prefer `mm_counter_t' here.
Why not a typedef?
> @@ -231,9 +237,13 @@ struct mm_struct {
> unsigned long start_code, end_code, start_data, end_data;
> unsigned long start_brk, brk, start_stack;
> unsigned long arg_start, arg_end, env_start, env_end;
> - unsigned long rss, anon_rss, total_vm, locked_vm, shared_vm;
> + unsigned long total_vm, locked_vm, shared_vm;
> unsigned long exec_vm, stack_vm, reserved_vm, def_flags, nr_ptes;
>
> + /* Special counters protected by the page_table_lock */
> + MM_COUNTER_T rss;
> + MM_COUNTER_T anon_rss;
> +
Why were only two counters converted?
Could I suggest that you rename all these counters, so that code which
fails to use the macros won't compile?
That renaming can be hidden in the header file: add an underscore to the
front of all the identifiers, paste that underscore back within the macros.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
On Fri, 11 Mar 2005, Dave Jones wrote:
> Splitting this last one into inc_mm_counter() and dec_mm_counter()
> means you can kill off the last argument, and get some of the
> readability back. As it stands, I think this patch adds a bunch
> of obfuscation for no clear benefit.
Ok.
-
This patch extracts all the operations on counters protected by the
page table lock (currently rss and anon_rss) into definitions in
include/linux/sched.h. All rss operations are performed through
the following macros:
get_mm_counter(mm, member) -> Obtain the value of a counter
set_mm_counter(mm, member, value) -> Set the value of a counter
update_mm_counter(mm, member, value)-> Add to a counter
inc_mm_counter(mm, member) -> Increment a counter
dec_mm_counter(mm, member) -> Decrement a counter
With this patch it becomes easier to add new counters and it is possible
to redefine the method of counter handling. The counters are an
issue for scalability since they are used in frequently used code paths and
may cause cache line bouncing.
F.e. One may not use counters at all and count the pages when needed, switch
to atomic operations if the mm_struct locking changes or split the rss
into counters that can be locally incremented.
Signed-off-by: Christoph Lameter <[EMAIL PROTECTED]>
Index: linux-2.6.11/include/linux/sched.h
===
--- linux-2.6.11.orig/include/linux/sched.h 2005-03-11 10:29:17.0
-0800
+++ linux-2.6.11/include/linux/sched.h 2005-03-11 10:57:59.0 -0800
@@ -205,6 +205,12 @@ arch_get_unmapped_area_topdown(struct fi
extern void arch_unmap_area(struct vm_area_struct *area);
extern void arch_unmap_area_topdown(struct vm_area_struct *area);
+#define set_mm_counter(mm, member, value) (mm)->member = (value)
+#define get_mm_counter(mm, member) ((mm)->member)
+#define update_mm_counter(mm, member, value) (mm)->member += (value)
+#define inc_mm_counter(mm, member) (mm)->member++
+#define dec_mm_counter(mm, member) (mm)->member--
+#define MM_COUNTER_T unsigned long
struct mm_struct {
struct vm_area_struct * mmap; /* list of VMAs */
@@ -221,7 +227,7 @@ struct mm_struct {
atomic_t mm_count; /* How many references to
"struct mm_struct" (users count as 1) */
int map_count; /* number of VMAs */
struct rw_semaphore mmap_sem;
- spinlock_t page_table_lock; /* Protects page tables,
mm->rss, mm->anon_rss */
+ spinlock_t page_table_lock; /* Protects page tables and
some counters */
struct list_head mmlist;/* List of maybe swapped mm's.
These are globally strung
* together off init_mm.mmlist,
and are protected
@@ -231,9 +237,13 @@ struct mm_struct {
unsigned long start_code, end_code, start_data, end_data;
unsigned long start_brk, brk, start_stack;
unsigned long arg_start, arg_end, env_start, env_end;
- unsigned long rss, anon_rss, total_vm, locked_vm, shared_vm;
+ unsigned long total_vm, locked_vm, shared_vm;
unsigned long exec_vm, stack_vm, reserved_vm, def_flags, nr_ptes;
+ /* Special counters protected by the page_table_lock */
+ MM_COUNTER_T rss;
+ MM_COUNTER_T anon_rss;
+
unsigned long saved_auxv[42]; /* for /proc/PID/auxv */
unsigned dumpable:1;
Index: linux-2.6.11/mm/memory.c
===
--- linux-2.6.11.orig/mm/memory.c 2005-03-11 10:29:17.0 -0800
+++ linux-2.6.11/mm/memory.c2005-03-11 10:57:00.0 -0800
@@ -312,9 +312,9 @@ copy_one_pte(struct mm_struct *dst_mm,
pte = pte_mkclean(pte);
pte = pte_mkold(pte);
get_page(page);
- dst_mm->rss++;
+ inc_mm_counter(dst_mm, rss);
if (PageAnon(page))
- dst_mm->anon_rss++;
+ inc_mm_counter(dst_mm, anon_rss);
set_pte_at(dst_mm, addr, dst_pte, pte);
page_dup_rmap(page);
}
@@ -525,7 +525,7 @@ static void zap_pte_range(struct mmu_gat
if (pte_dirty(pte))
set_page_dirty(page);
if (PageAnon(page))
- tlb->mm->anon_rss--;
+ dec_mm_counter(tlb->mm, anon_rss);
else if (pte_young(pte))
mark_page_accessed(page);
tlb->freed++;
@@ -1351,9 +1351,9 @@ static int do_wp_page(struct mm_struct *
page_table = pte_offset_map(pmd, address);
if (likely(pte_same(*page_table, pte))) {
if (PageAnon(old_page))
- mm->anon_rss--;
+ dec_mm_counter(mm, anon_rss);
if
Re: [PATCH] mm counter operations through macros
On Fri, Mar 11, 2005 at 04:23:21AM -0800, Christoph Lameter wrote: > This patch extracts all the operations on counters protected by the > page table lock (currently rss and anon_rss) into definitions in > include/linux/sched.h. All rss operations are performed through > the following three macros: > > get_mm_counter(mm, member) -> Obtain the value of a counter > set_mm_counter(mm, member, value)-> Set the value of a counter > update_mm_counter(mm, member, value) -> Add a value to a counter Splitting this last one into inc_mm_counter() and dec_mm_counter() means you can kill off the last argument, and get some of the readability back. As it stands, I think this patch adds a bunch of obfuscation for no clear benefit. Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
On Fri, Mar 11, 2005 at 04:23:21AM -0800, Christoph Lameter wrote: This patch extracts all the operations on counters protected by the page table lock (currently rss and anon_rss) into definitions in include/linux/sched.h. All rss operations are performed through the following three macros: get_mm_counter(mm, member) - Obtain the value of a counter set_mm_counter(mm, member, value)- Set the value of a counter update_mm_counter(mm, member, value) - Add a value to a counter Splitting this last one into inc_mm_counter() and dec_mm_counter() means you can kill off the last argument, and get some of the readability back. As it stands, I think this patch adds a bunch of obfuscation for no clear benefit. Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
On Fri, 11 Mar 2005, Dave Jones wrote:
Splitting this last one into inc_mm_counter() and dec_mm_counter()
means you can kill off the last argument, and get some of the
readability back. As it stands, I think this patch adds a bunch
of obfuscation for no clear benefit.
Ok.
-
This patch extracts all the operations on counters protected by the
page table lock (currently rss and anon_rss) into definitions in
include/linux/sched.h. All rss operations are performed through
the following macros:
get_mm_counter(mm, member) - Obtain the value of a counter
set_mm_counter(mm, member, value) - Set the value of a counter
update_mm_counter(mm, member, value)- Add to a counter
inc_mm_counter(mm, member) - Increment a counter
dec_mm_counter(mm, member) - Decrement a counter
With this patch it becomes easier to add new counters and it is possible
to redefine the method of counter handling. The counters are an
issue for scalability since they are used in frequently used code paths and
may cause cache line bouncing.
F.e. One may not use counters at all and count the pages when needed, switch
to atomic operations if the mm_struct locking changes or split the rss
into counters that can be locally incremented.
Signed-off-by: Christoph Lameter [EMAIL PROTECTED]
Index: linux-2.6.11/include/linux/sched.h
===
--- linux-2.6.11.orig/include/linux/sched.h 2005-03-11 10:29:17.0
-0800
+++ linux-2.6.11/include/linux/sched.h 2005-03-11 10:57:59.0 -0800
@@ -205,6 +205,12 @@ arch_get_unmapped_area_topdown(struct fi
extern void arch_unmap_area(struct vm_area_struct *area);
extern void arch_unmap_area_topdown(struct vm_area_struct *area);
+#define set_mm_counter(mm, member, value) (mm)-member = (value)
+#define get_mm_counter(mm, member) ((mm)-member)
+#define update_mm_counter(mm, member, value) (mm)-member += (value)
+#define inc_mm_counter(mm, member) (mm)-member++
+#define dec_mm_counter(mm, member) (mm)-member--
+#define MM_COUNTER_T unsigned long
struct mm_struct {
struct vm_area_struct * mmap; /* list of VMAs */
@@ -221,7 +227,7 @@ struct mm_struct {
atomic_t mm_count; /* How many references to
struct mm_struct (users count as 1) */
int map_count; /* number of VMAs */
struct rw_semaphore mmap_sem;
- spinlock_t page_table_lock; /* Protects page tables,
mm-rss, mm-anon_rss */
+ spinlock_t page_table_lock; /* Protects page tables and
some counters */
struct list_head mmlist;/* List of maybe swapped mm's.
These are globally strung
* together off init_mm.mmlist,
and are protected
@@ -231,9 +237,13 @@ struct mm_struct {
unsigned long start_code, end_code, start_data, end_data;
unsigned long start_brk, brk, start_stack;
unsigned long arg_start, arg_end, env_start, env_end;
- unsigned long rss, anon_rss, total_vm, locked_vm, shared_vm;
+ unsigned long total_vm, locked_vm, shared_vm;
unsigned long exec_vm, stack_vm, reserved_vm, def_flags, nr_ptes;
+ /* Special counters protected by the page_table_lock */
+ MM_COUNTER_T rss;
+ MM_COUNTER_T anon_rss;
+
unsigned long saved_auxv[42]; /* for /proc/PID/auxv */
unsigned dumpable:1;
Index: linux-2.6.11/mm/memory.c
===
--- linux-2.6.11.orig/mm/memory.c 2005-03-11 10:29:17.0 -0800
+++ linux-2.6.11/mm/memory.c2005-03-11 10:57:00.0 -0800
@@ -312,9 +312,9 @@ copy_one_pte(struct mm_struct *dst_mm,
pte = pte_mkclean(pte);
pte = pte_mkold(pte);
get_page(page);
- dst_mm-rss++;
+ inc_mm_counter(dst_mm, rss);
if (PageAnon(page))
- dst_mm-anon_rss++;
+ inc_mm_counter(dst_mm, anon_rss);
set_pte_at(dst_mm, addr, dst_pte, pte);
page_dup_rmap(page);
}
@@ -525,7 +525,7 @@ static void zap_pte_range(struct mmu_gat
if (pte_dirty(pte))
set_page_dirty(page);
if (PageAnon(page))
- tlb-mm-anon_rss--;
+ dec_mm_counter(tlb-mm, anon_rss);
else if (pte_young(pte))
mark_page_accessed(page);
tlb-freed++;
@@ -1351,9 +1351,9 @@ static int do_wp_page(struct mm_struct *
page_table = pte_offset_map(pmd, address);
if (likely(pte_same(*page_table, pte))) {
if (PageAnon(old_page))
- mm-anon_rss--;
+ dec_mm_counter(mm, anon_rss);
if (PageReserved(old_page))
-
Re: [PATCH] mm counter operations through macros
Christoph Lameter [EMAIL PROTECTED] wrote:
This patch extracts all the operations on counters protected by the
page table lock (currently rss and anon_rss) into definitions in
include/linux/sched.h. All rss operations are performed through
the following macros:
get_mm_counter(mm, member)- Obtain the value of a counter
set_mm_counter(mm, member, value) - Set the value of a counter
update_mm_counter(mm, member, value) - Add to a counter
inc_mm_counter(mm, member)- Increment a counter
dec_mm_counter(mm, member)- Decrement a counter
I spose it makes sense, if we'll be making scalability changes in there.
+#define set_mm_counter(mm, member, value) (mm)-member = (value)
+#define get_mm_counter(mm, member) ((mm)-member)
+#define update_mm_counter(mm, member, value) (mm)-member += (value)
+#define inc_mm_counter(mm, member) (mm)-member++
+#define dec_mm_counter(mm, member) (mm)-member--
+#define MM_COUNTER_T unsigned long
Would prefer `mm_counter_t' here.
Why not a typedef?
@@ -231,9 +237,13 @@ struct mm_struct {
unsigned long start_code, end_code, start_data, end_data;
unsigned long start_brk, brk, start_stack;
unsigned long arg_start, arg_end, env_start, env_end;
- unsigned long rss, anon_rss, total_vm, locked_vm, shared_vm;
+ unsigned long total_vm, locked_vm, shared_vm;
unsigned long exec_vm, stack_vm, reserved_vm, def_flags, nr_ptes;
+ /* Special counters protected by the page_table_lock */
+ MM_COUNTER_T rss;
+ MM_COUNTER_T anon_rss;
+
Why were only two counters converted?
Could I suggest that you rename all these counters, so that code which
fails to use the macros won't compile?
That renaming can be hidden in the header file: add an underscore to the
front of all the identifiers, paste that underscore back within the macros.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm counter operations through macros
On Fri, 11 Mar 2005, Andrew Morton wrote:
+#define set_mm_counter(mm, member, value) (mm)-member = (value)
+#define get_mm_counter(mm, member) ((mm)-member)
+#define update_mm_counter(mm, member, value) (mm)-member += (value)
+#define inc_mm_counter(mm, member) (mm)-member++
+#define dec_mm_counter(mm, member) (mm)-member--
+#define MM_COUNTER_T unsigned long
Would prefer `mm_counter_t' here.
Why not a typedef?
Ok typedef it is.
@@ -231,9 +237,13 @@ struct mm_struct {
unsigned long start_code, end_code, start_data, end_data;
unsigned long start_brk, brk, start_stack;
unsigned long arg_start, arg_end, env_start, env_end;
- unsigned long rss, anon_rss, total_vm, locked_vm, shared_vm;
+ unsigned long total_vm, locked_vm, shared_vm;
unsigned long exec_vm, stack_vm, reserved_vm, def_flags, nr_ptes;
+ /* Special counters protected by the page_table_lock */
+ MM_COUNTER_T rss;
+ MM_COUNTER_T anon_rss;
+
Why were only two counters converted?
Because only these two counters are protected by the page table lock.
Could I suggest that you rename all these counters, so that code which
fails to use the macros won't compile?
Ok. will make them mm_xx as they were in previous patches.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
