From: Thomas Gleixner <hidden> Date: 2011-11-15 01:15:55
Dear RT Folks,
I'm pleased to announce the 3.2-rc1-rt2 release.
Changes vs. 3.2-rc1-rt2:
* Add missing softirq export (John Kacur)
* Fix rcu macro substitution (John Kacur)
* Fix device mapper BUG_ON (Reported by Luis Claudio)
* Fix x86 aesni (crypto) preemption problem (Peter Zijlstra,
reported by Carsten Emde)
* Fix UP (tiny) RCU build issues (Reported by Tim Sanders)
3.2-rc1-rt2 is not against 3.2-rc1 - it's against the post rc1
commit 52e4c2a05256cb83cda12f3c2137ab1533344edb.
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=52e4c2a05256cb83cda12f3c2137ab1533344edb
For non git users conveniance I uploaded a delta patch against
3.2-rc1:
https://tglx.de/~tglx/rt/3.2/base/patch-3.2-rc1-52e4c2a05.patch.gz
to get the base kernel on which the RT patch applies.
The incremental patch against 3.2-rc1-rt1 can be found here:
https://tglx.de/~tglx/rt/3.2/incr/patch-3.2-rc1-52e4c2a05-rt1-rt2.patch.gz
and is also appended below.
The RT patch against 3.2-rc1-52e4c2a05 can be found here:
https://tglx.de/~tglx/rt/3.2/patch-3.2-rc1-52e4c2a05-rt2.patch.gz
The split quilt queue is available at:
https://tglx.de/~tglx/rt/3.2/patches-3.2-rc1-52e4c2a05-rt2.tar.gz
I got several private (sigh) questions about the signature files which
can be found beside the release files.
The signature file is a gpg detached signature signed with my signing
key against the _uncompressed_ file.
In order to verify the signature, download both the release file and
the signature file (which has ".sig" appended to the release file),
e.g.:
wget https://tglx.de/~tglx/rt/3.2/incr/patch-3.2-rc1-52e4c2a05-rt1-rt2.patch.gz
wget https://tglx.de/~tglx/rt/3.2/incr/patch-3.2-rc1-52e4c2a05-rt1-rt2.patch.sign
Now decompress the release file:
gunzip patch-3.2-rc1-52e4c2a05-rt1-rt2.patch.gz
Make sure that you have my gpg key in your gpg keyring
gpg --recv-keys 06FF0B14
Now verify against the signature:
gpg --verify patch-3.2-rc1-52e4c2a05-rt1-rt2.patch.sign patch-3.2-rc1-52e4c2a05-rt1-rt2.patch.gz
That should give you something like
gpg: Signature made Tue 15 Nov 2011 12:47:24 AM CET using RSA key ID 0D7498A1
gpg: Good signature from "Thomas Gleixner [off-list ref]"
gpg: aka "Thomas Gleixner [off-list ref]"
gpg: aka "Thomas Gleixner [off-list ref]"
gpg: WARNING: This key is not certified with a trusted signature!
gpg: There is no indication that the signature belongs to the owner.
The last two lines are because you do not trust my key when you
downloaded it, but that's your decision to make :)
Enjoy,
tglx
---
Index: linux-3.2/include/linux/rcutree.h
===================================================================
@@ -932,6 +932,7 @@ enum#include<linux/list.h>#include<linux/rcupdate.h>#include<linux/wait.h>+#include<linux/atomic.h>/* For the /proc/sys support */structctl_table;
From: Steven Rostedt <rostedt@goodmis.org> Date: 2011-11-16 14:12:43
On Wed, 2011-11-16 at 17:16 +0800, Yong Zhang wrote:
Looking at commit 4799401f [memcg: Fix race condition in
memcg_check_events() with this_cpu usage], we just want
to disable migration. So use the right API in -rt. This
will cure below warning.
No this won't work. Not even for -rt. If we disable migration but not
preemption, then two tasks can take this path. And the checks in
__memcg_event_check() will be corrupted because nothing is protecting
the updates from two tasks going into the same path.
Perhaps a local_lock would work.
-- Steve
quoted hunk
/* threshold event is triggered in finer grain than soft limit */
if (unlikely(__memcg_event_check(memcg, MEM_CGROUP_TARGET_THRESH))) {
mem_cgroup_threshold(memcg);
From: Thomas Gleixner <hidden> Date: 2011-11-16 17:02:46
On Wed, 16 Nov 2011, Steven Rostedt wrote:
On Wed, 2011-11-16 at 17:16 +0800, Yong Zhang wrote:
quoted
Looking at commit 4799401f [memcg: Fix race condition in
memcg_check_events() with this_cpu usage], we just want
to disable migration. So use the right API in -rt. This
will cure below warning.
No this won't work. Not even for -rt. If we disable migration but not
preemption, then two tasks can take this path. And the checks in
__memcg_event_check() will be corrupted because nothing is protecting
the updates from two tasks going into the same path.
Perhaps a local_lock would work.
Yes, that's the only sensible option for now. Untested patch below.
Thanks,
tglx
-------------->
mm/memcontrol.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
Index: linux-3.2/mm/memcontrol.c
===================================================================
@@ -2608,10 +2612,10 @@ static int mem_cgroup_move_account(strucif(PageCgroupFileMapped(pc)){/* Update mapped_file data for mem_cgroup */-preempt_disable();+local_lock(stats_lock);__this_cpu_dec(from->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]);__this_cpu_inc(to->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]);-preempt_enable();+local_unlock(stats_lock);}mem_cgroup_charge_statistics(from,PageCgroupCache(pc),-nr_pages);if(uncharge)
From: Luis Henriques <hidden> Date: 2011-11-16 17:18:25
On Wed, Nov 16, 2011 at 06:02:42PM +0100, Thomas Gleixner wrote:
On Wed, 16 Nov 2011, Steven Rostedt wrote:
quoted
On Wed, 2011-11-16 at 17:16 +0800, Yong Zhang wrote:
quoted
Looking at commit 4799401f [memcg: Fix race condition in
memcg_check_events() with this_cpu usage], we just want
to disable migration. So use the right API in -rt. This
will cure below warning.
No this won't work. Not even for -rt. If we disable migration but not
preemption, then two tasks can take this path. And the checks in
__memcg_event_check() will be corrupted because nothing is protecting
the updates from two tasks going into the same path.
Perhaps a local_lock would work.
Yes, that's the only sensible option for now. Untested patch below.
I run a quick test and it looks like the problem is gone.
Cheers,
--
Luis Henriques
On Wed, 16 Nov 2011 17:18:09 +0000
Luis Henriques [off-list ref] wrote:
On Wed, Nov 16, 2011 at 06:02:42PM +0100, Thomas Gleixner wrote:
quoted
On Wed, 16 Nov 2011, Steven Rostedt wrote:
quoted
On Wed, 2011-11-16 at 17:16 +0800, Yong Zhang wrote:
quoted
Looking at commit 4799401f [memcg: Fix race condition in
memcg_check_events() with this_cpu usage], we just want
to disable migration. So use the right API in -rt. This
will cure below warning.
No this won't work. Not even for -rt. If we disable migration but not
preemption, then two tasks can take this path. And the checks in
__memcg_event_check() will be corrupted because nothing is protecting
the updates from two tasks going into the same path.
Perhaps a local_lock would work.
Yes, that's the only sensible option for now. Untested patch below.
I run a quick test and it looks like the problem is gone.
Cheers,
Could you CC the final patch to cgroups@vger.kernel.org ?
Does this fix will go thorugh rt tree rather than -mm ?
Thanks,
-Kame
From: Steven Rostedt <rostedt@goodmis.org> Date: 2011-11-17 00:23:48
On Thu, 2011-11-17 at 08:48 +0900, KAMEZAWA Hiroyuki wrote:
quoted
I run a quick test and it looks like the problem is gone.
Cheers,
Could you CC the final patch to cgroups@vger.kernel.org ?
Does this fix will go thorugh rt tree rather than -mm ?
This particular fix only goes through the rt tree because it is only a
bug when full rt is enabled. When rt is enabled, spin_locks become
mutexes, and thus can not be used when preemption is disabled. The rt
tree introduced a local_lock() to remove open coded preempt disabling
and also to help annotate places that need per cpu protections.
-- Steve
On Wed, Nov 16, 2011 at 09:12:38AM -0500, Steven Rostedt wrote:
On Wed, 2011-11-16 at 17:16 +0800, Yong Zhang wrote:
quoted
Looking at commit 4799401f [memcg: Fix race condition in
memcg_check_events() with this_cpu usage], we just want
to disable migration. So use the right API in -rt. This
will cure below warning.
No this won't work. Not even for -rt. If we disable migration but not
preemption, then two tasks can take this path. And the checks in
__memcg_event_check() will be corrupted because nothing is protecting
the updates from two tasks going into the same path.
I assumed that we only care about migration, but obviously I'm wrong.
On Wed, Nov 16, 2011 at 06:02:42PM +0100, Thomas Gleixner wrote:
On Wed, 16 Nov 2011, Steven Rostedt wrote:
quoted
On Wed, 2011-11-16 at 17:16 +0800, Yong Zhang wrote:
quoted
Looking at commit 4799401f [memcg: Fix race condition in
memcg_check_events() with this_cpu usage], we just want
to disable migration. So use the right API in -rt. This
will cure below warning.
No this won't work. Not even for -rt. If we disable migration but not
preemption, then two tasks can take this path. And the checks in
__memcg_event_check() will be corrupted because nothing is protecting
the updates from two tasks going into the same path.
Perhaps a local_lock would work.
Yes, that's the only sensible option for now. Untested patch below.
From: Thomas Gleixner <hidden> Date: 2011-11-17 10:25:59
On Thu, 17 Nov 2011, Yong Zhang wrote:
On Wed, Nov 16, 2011 at 06:02:42PM +0100, Thomas Gleixner wrote:
quoted
On Wed, 16 Nov 2011, Steven Rostedt wrote:
quoted
On Wed, 2011-11-16 at 17:16 +0800, Yong Zhang wrote:
quoted
Looking at commit 4799401f [memcg: Fix race condition in
memcg_check_events() with this_cpu usage], we just want
to disable migration. So use the right API in -rt. This
will cure below warning.
No this won't work. Not even for -rt. If we disable migration but not
preemption, then two tasks can take this path. And the checks in
__memcg_event_check() will be corrupted because nothing is protecting
the updates from two tasks going into the same path.
Perhaps a local_lock would work.
Yes, that's the only sensible option for now. Untested patch below.
Works for me.
Johannes came up with a different solution. Could you please give it a try?
Thanks,
tglx
------------->
Subject: [patch] mm: memcg: shorten preempt-disabled section around event checks
Only the ratelimit checks themselves have to run with preemption
disabled, the resulting actions - checking for usage thresholds,
updating the soft limit tree - can and should run with preemption
enabled.
Signed-off-by: Johannes Weiner <redacted>
---
mm/memcontrol.c | 73 ++++++++++++++++++++++++++----------------------------
1 files changed, 35 insertions(+), 38 deletions(-)
Thomas, HTH and it is probably interesting for upstream as well.
Unfortunately, I'm in the middle of moving right now, so this is
untested except for compiling.
@@ -683,37 +683,32 @@ static unsigned long mem_cgroup_nr_lru_pages(struct mem_cgroup *memcg,returntotal;}-staticbool__memcg_event_check(structmem_cgroup*memcg,inttarget)+staticboolmem_cgroup_event_ratelimit(structmem_cgroup*memcg,+enummem_cgroup_events_targettarget){unsignedlongval,next;val=__this_cpu_read(memcg->stat->events[MEM_CGROUP_EVENTS_COUNT]);next=__this_cpu_read(memcg->stat->targets[target]);/* from time_after() in jiffies.h */-return((long)next-(long)val<0);-}--staticvoid__mem_cgroup_target_update(structmem_cgroup*memcg,inttarget)-{-unsignedlongval,next;--val=__this_cpu_read(memcg->stat->events[MEM_CGROUP_EVENTS_COUNT]);--switch(target){-caseMEM_CGROUP_TARGET_THRESH:-next=val+THRESHOLDS_EVENTS_TARGET;-break;-caseMEM_CGROUP_TARGET_SOFTLIMIT:-next=val+SOFTLIMIT_EVENTS_TARGET;-break;-caseMEM_CGROUP_TARGET_NUMAINFO:-next=val+NUMAINFO_EVENTS_TARGET;-break;-default:-return;+if((long)next-(long)val<0){+switch(target){+caseMEM_CGROUP_TARGET_THRESH:+next=val+THRESHOLDS_EVENTS_TARGET;+break;+caseMEM_CGROUP_TARGET_SOFTLIMIT:+next=val+SOFTLIMIT_EVENTS_TARGET;+break;+caseMEM_CGROUP_TARGET_NUMAINFO:+next=val+NUMAINFO_EVENTS_TARGET;+break;+default:+break;+}+__this_cpu_write(memcg->stat->targets[target],next);+returntrue;}--__this_cpu_write(memcg->stat->targets[target],next);+returnfalse;}/*
@@ -724,25 +719,27 @@ static void memcg_check_events(struct mem_cgroup *memcg, struct page *page){preempt_disable();/* threshold event is triggered in finer grain than soft limit */-if(unlikely(__memcg_event_check(memcg,MEM_CGROUP_TARGET_THRESH))){+if(unlikely(mem_cgroup_event_ratelimit(memcg,+MEM_CGROUP_TARGET_THRESH))){+booldo_softlimit,do_numainfo;++do_softlimit=mem_cgroup_event_ratelimit(memcg,+MEM_CGROUP_TARGET_SOFTLIMIT);+#if MAX_NUMNODES > 1+do_numainfo=mem_cgroup_event_ratelimit(memcg,+MEM_CGROUP_TARGET_NUMAINFO);+#endif+preempt_enable();+mem_cgroup_threshold(memcg);-__mem_cgroup_target_update(memcg,MEM_CGROUP_TARGET_THRESH);-if(unlikely(__memcg_event_check(memcg,-MEM_CGROUP_TARGET_SOFTLIMIT))){+if(unlikely(do_softlimit))mem_cgroup_update_tree(memcg,page);-__mem_cgroup_target_update(memcg,-MEM_CGROUP_TARGET_SOFTLIMIT);-}#if MAX_NUMNODES > 1-if(unlikely(__memcg_event_check(memcg,-MEM_CGROUP_TARGET_NUMAINFO))){+if(unlikely(do_numainfo))atomic_inc(&memcg->numainfo_events);-__mem_cgroup_target_update(memcg,-MEM_CGROUP_TARGET_NUMAINFO);-}#endif-}-preempt_enable();+}else+preempt_enable();}staticstructmem_cgroup*mem_cgroup_from_cont(structcgroup*cont)
On Thu, Nov 17, 2011 at 11:25:56AM +0100, Thomas Gleixner wrote:
On Thu, 17 Nov 2011, Yong Zhang wrote:
quoted
On Wed, Nov 16, 2011 at 06:02:42PM +0100, Thomas Gleixner wrote:
quoted
On Wed, 16 Nov 2011, Steven Rostedt wrote:
quoted
On Wed, 2011-11-16 at 17:16 +0800, Yong Zhang wrote:
quoted
Looking at commit 4799401f [memcg: Fix race condition in
memcg_check_events() with this_cpu usage], we just want
to disable migration. So use the right API in -rt. This
will cure below warning.
No this won't work. Not even for -rt. If we disable migration but not
preemption, then two tasks can take this path. And the checks in
__memcg_event_check() will be corrupted because nothing is protecting
the updates from two tasks going into the same path.
Perhaps a local_lock would work.
Yes, that's the only sensible option for now. Untested patch below.
Works for me.
Johannes came up with a different solution. Could you please give it a try?
Works too :)
Thanks,
Yong
quoted hunk
Thanks,
tglx
------------->
Subject: [patch] mm: memcg: shorten preempt-disabled section around event checks
Only the ratelimit checks themselves have to run with preemption
disabled, the resulting actions - checking for usage thresholds,
updating the soft limit tree - can and should run with preemption
enabled.
Signed-off-by: Johannes Weiner <redacted>
---
mm/memcontrol.c | 73 ++++++++++++++++++++++++++----------------------------
1 files changed, 35 insertions(+), 38 deletions(-)
Thomas, HTH and it is probably interesting for upstream as well.
Unfortunately, I'm in the middle of moving right now, so this is
untested except for compiling.
@@ -683,37 +683,32 @@ static unsigned long mem_cgroup_nr_lru_pages(struct mem_cgroup *memcg,returntotal;}-staticbool__memcg_event_check(structmem_cgroup*memcg,inttarget)+staticboolmem_cgroup_event_ratelimit(structmem_cgroup*memcg,+enummem_cgroup_events_targettarget){unsignedlongval,next;val=__this_cpu_read(memcg->stat->events[MEM_CGROUP_EVENTS_COUNT]);next=__this_cpu_read(memcg->stat->targets[target]);/* from time_after() in jiffies.h */-return((long)next-(long)val<0);-}--staticvoid__mem_cgroup_target_update(structmem_cgroup*memcg,inttarget)-{-unsignedlongval,next;--val=__this_cpu_read(memcg->stat->events[MEM_CGROUP_EVENTS_COUNT]);--switch(target){-caseMEM_CGROUP_TARGET_THRESH:-next=val+THRESHOLDS_EVENTS_TARGET;-break;-caseMEM_CGROUP_TARGET_SOFTLIMIT:-next=val+SOFTLIMIT_EVENTS_TARGET;-break;-caseMEM_CGROUP_TARGET_NUMAINFO:-next=val+NUMAINFO_EVENTS_TARGET;-break;-default:-return;+if((long)next-(long)val<0){+switch(target){+caseMEM_CGROUP_TARGET_THRESH:+next=val+THRESHOLDS_EVENTS_TARGET;+break;+caseMEM_CGROUP_TARGET_SOFTLIMIT:+next=val+SOFTLIMIT_EVENTS_TARGET;+break;+caseMEM_CGROUP_TARGET_NUMAINFO:+next=val+NUMAINFO_EVENTS_TARGET;+break;+default:+break;+}+__this_cpu_write(memcg->stat->targets[target],next);+returntrue;}--__this_cpu_write(memcg->stat->targets[target],next);+returnfalse;}/*
@@ -724,25 +719,27 @@ static void memcg_check_events(struct mem_cgroup *memcg, struct page *page){preempt_disable();/* threshold event is triggered in finer grain than soft limit */-if(unlikely(__memcg_event_check(memcg,MEM_CGROUP_TARGET_THRESH))){+if(unlikely(mem_cgroup_event_ratelimit(memcg,+MEM_CGROUP_TARGET_THRESH))){+booldo_softlimit,do_numainfo;++do_softlimit=mem_cgroup_event_ratelimit(memcg,+MEM_CGROUP_TARGET_SOFTLIMIT);+#if MAX_NUMNODES > 1+do_numainfo=mem_cgroup_event_ratelimit(memcg,+MEM_CGROUP_TARGET_NUMAINFO);+#endif+preempt_enable();+mem_cgroup_threshold(memcg);-__mem_cgroup_target_update(memcg,MEM_CGROUP_TARGET_THRESH);-if(unlikely(__memcg_event_check(memcg,-MEM_CGROUP_TARGET_SOFTLIMIT))){+if(unlikely(do_softlimit))mem_cgroup_update_tree(memcg,page);-__mem_cgroup_target_update(memcg,-MEM_CGROUP_TARGET_SOFTLIMIT);-}#if MAX_NUMNODES > 1-if(unlikely(__memcg_event_check(memcg,-MEM_CGROUP_TARGET_NUMAINFO))){+if(unlikely(do_numainfo))atomic_inc(&memcg->numainfo_events);-__mem_cgroup_target_update(memcg,-MEM_CGROUP_TARGET_NUMAINFO);-}#endif-}-preempt_enable();+}else+preempt_enable();}staticstructmem_cgroup*mem_cgroup_from_cont(structcgroup*cont)
--
1.7.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/