* [ANNOUNCE] 3.2-rc1-rt2
@ 2011-11-15 1:15 Thomas Gleixner
2011-11-15 8:40 ` Yong Zhang
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Gleixner @ 2011-11-15 1:15 UTC (permalink / raw)
To: LKML; +Cc: linux-rt-users
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 <tglx@linutronix.de>"
gpg: aka "Thomas Gleixner <tglx@glx-um.de>"
gpg: aka "Thomas Gleixner <tglx@tglx.de>"
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
===================================================================
--- linux-3.2.orig/include/linux/rcutree.h
+++ linux-3.2/include/linux/rcutree.h
@@ -60,7 +60,7 @@ static inline void exit_rcu(void)
#ifndef CONFIG_PREEMPT_RT_FULL
extern void synchronize_rcu_bh(void);
#else
-# define synchronize_rcu_bh() synchronize_rcu()
+# define synchronize_rcu_bh synchronize_rcu
#endif
extern void synchronize_sched_expedited(void);
extern void synchronize_rcu_expedited(void);
Index: linux-3.2/init/Kconfig
===================================================================
--- linux-3.2.orig/init/Kconfig
+++ linux-3.2/init/Kconfig
@@ -410,7 +410,7 @@ config TINY_RCU
config TINY_PREEMPT_RCU
bool "Preemptible UP-only small-memory-footprint RCU"
- depends on PREEMPT && !SMP && !PREEMPT_RT_FULL
+ depends on PREEMPT && !SMP
help
This option selects the RCU implementation that is designed
for real-time UP systems. This option greatly reduces the
Index: linux-3.2/kernel/softirq.c
===================================================================
--- linux-3.2.orig/kernel/softirq.c
+++ linux-3.2/kernel/softirq.c
@@ -447,6 +447,7 @@ int in_serving_softirq(void)
preempt_enable();
return res;
}
+EXPORT_SYMBOL(in_serving_softirq);
/*
* Called with bh and local interrupts disabled. For full RT cpu must
Index: linux-3.2/localversion-rt
===================================================================
--- linux-3.2.orig/localversion-rt
+++ linux-3.2/localversion-rt
@@ -1 +1 @@
--rt1
+-rt2
Index: linux-3.2/include/linux/sysctl.h
===================================================================
--- linux-3.2.orig/include/linux/sysctl.h
+++ linux-3.2/include/linux/sysctl.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 */
struct ctl_table;
Index: linux-3.2/kernel/rcutiny.c
===================================================================
--- linux-3.2.orig/kernel/rcutiny.c
+++ linux-3.2/kernel/rcutiny.c
@@ -243,6 +243,7 @@ void call_rcu_sched(struct rcu_head *hea
}
EXPORT_SYMBOL_GPL(call_rcu_sched);
+#ifndef CONFIG_PREEMPT_RT_FULL
/*
* Post an RCU bottom-half callback to be invoked after any subsequent
* quiescent state.
@@ -252,3 +253,4 @@ void call_rcu_bh(struct rcu_head *head,
__call_rcu(head, func, &rcu_bh_ctrlblk);
}
EXPORT_SYMBOL_GPL(call_rcu_bh);
+#endif
Index: linux-3.2/arch/x86/crypto/aesni-intel_glue.c
===================================================================
--- linux-3.2.orig/arch/x86/crypto/aesni-intel_glue.c
+++ linux-3.2/arch/x86/crypto/aesni-intel_glue.c
@@ -289,14 +289,14 @@ static int ecb_encrypt(struct blkcipher_
err = blkcipher_walk_virt(desc, &walk);
desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP;
- kernel_fpu_begin();
while ((nbytes = walk.nbytes)) {
+ kernel_fpu_begin();
aesni_ecb_enc(ctx, walk.dst.virt.addr, walk.src.virt.addr,
- nbytes & AES_BLOCK_MASK);
+ nbytes & AES_BLOCK_MASK);
+ kernel_fpu_end();
nbytes &= AES_BLOCK_SIZE - 1;
err = blkcipher_walk_done(desc, &walk, nbytes);
}
- kernel_fpu_end();
return err;
}
@@ -313,14 +313,14 @@ static int ecb_decrypt(struct blkcipher_
err = blkcipher_walk_virt(desc, &walk);
desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP;
- kernel_fpu_begin();
while ((nbytes = walk.nbytes)) {
+ kernel_fpu_begin();
aesni_ecb_dec(ctx, walk.dst.virt.addr, walk.src.virt.addr,
nbytes & AES_BLOCK_MASK);
+ kernel_fpu_end();
nbytes &= AES_BLOCK_SIZE - 1;
err = blkcipher_walk_done(desc, &walk, nbytes);
}
- kernel_fpu_end();
return err;
}
@@ -359,14 +359,14 @@ static int cbc_encrypt(struct blkcipher_
err = blkcipher_walk_virt(desc, &walk);
desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP;
- kernel_fpu_begin();
while ((nbytes = walk.nbytes)) {
+ kernel_fpu_begin();
aesni_cbc_enc(ctx, walk.dst.virt.addr, walk.src.virt.addr,
nbytes & AES_BLOCK_MASK, walk.iv);
+ kernel_fpu_end();
nbytes &= AES_BLOCK_SIZE - 1;
err = blkcipher_walk_done(desc, &walk, nbytes);
}
- kernel_fpu_end();
return err;
}
@@ -383,14 +383,14 @@ static int cbc_decrypt(struct blkcipher_
err = blkcipher_walk_virt(desc, &walk);
desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP;
- kernel_fpu_begin();
while ((nbytes = walk.nbytes)) {
+ kernel_fpu_begin();
aesni_cbc_dec(ctx, walk.dst.virt.addr, walk.src.virt.addr,
nbytes & AES_BLOCK_MASK, walk.iv);
+ kernel_fpu_end();
nbytes &= AES_BLOCK_SIZE - 1;
err = blkcipher_walk_done(desc, &walk, nbytes);
}
- kernel_fpu_end();
return err;
}
@@ -445,18 +445,20 @@ static int ctr_crypt(struct blkcipher_de
err = blkcipher_walk_virt_block(desc, &walk, AES_BLOCK_SIZE);
desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP;
- kernel_fpu_begin();
while ((nbytes = walk.nbytes) >= AES_BLOCK_SIZE) {
+ kernel_fpu_begin();
aesni_ctr_enc(ctx, walk.dst.virt.addr, walk.src.virt.addr,
nbytes & AES_BLOCK_MASK, walk.iv);
+ kernel_fpu_end();
nbytes &= AES_BLOCK_SIZE - 1;
err = blkcipher_walk_done(desc, &walk, nbytes);
}
if (walk.nbytes) {
+ kernel_fpu_begin();
ctr_crypt_final(ctx, &walk);
+ kernel_fpu_end();
err = blkcipher_walk_done(desc, &walk, 0);
}
- kernel_fpu_end();
return err;
}
Index: linux-3.2/drivers/md/dm.c
===================================================================
--- linux-3.2.orig/drivers/md/dm.c
+++ linux-3.2/drivers/md/dm.c
@@ -1648,14 +1648,14 @@ static void dm_request_fn(struct request
if (map_request(ti, clone, md))
goto requeued;
- BUG_ON(!irqs_disabled());
+ BUG_ON_NORT(!irqs_disabled());
spin_lock(q->queue_lock);
}
goto out;
requeued:
- BUG_ON(!irqs_disabled());
+ BUG_ON_NORT(!irqs_disabled());
spin_lock(q->queue_lock);
delay_and_out:
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [ANNOUNCE] 3.2-rc1-rt2
2011-11-15 1:15 [ANNOUNCE] 3.2-rc1-rt2 Thomas Gleixner
@ 2011-11-15 8:40 ` Yong Zhang
2011-11-15 13:52 ` Luis Henriques
2011-11-16 9:16 ` [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events() Yong Zhang
0 siblings, 2 replies; 15+ messages in thread
From: Yong Zhang @ 2011-11-15 8:40 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: LKML, linux-rt-users
On Tue, Nov 15, 2011 at 02:15:52AM +0100, Thomas Gleixner wrote:
> 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)
Got below warning:
[ 8.459702] BUG: sleeping function called from invalid context at linux/kernel/rtmutex.c:645
[ 8.459705] in_atomic(): 1, irqs_disabled(): 0, pid: 1, name: swapper/0
[ 8.459707] 1 lock held by swapper/0/1:
[ 8.459708] #0: (&sig->cred_guard_mutex){+.+.+.}, at: [<ffffffff81154c45>] prepare_bprm_creds+0x35/0x80
[ 8.459717] Pid: 1, comm: swapper/0 Not tainted 3.2.0-rc1-rt2-11311-g3c4c0e7-dirty #10
[ 8.459718] Call Trace:
[ 8.459724] [<ffffffff81040c4e>] __might_sleep+0x12e/0x1e0
[ 8.459730] [<ffffffff815f4d74>] rt_spin_lock+0x24/0x60
[ 8.459735] [<ffffffff8114506e>] memcg_check_events+0x11e/0x230
[ 8.459739] [<ffffffff81146c5a>] T.1144+0x8a/0xf0
[ 8.459743] [<ffffffff81146d16>] __mem_cgroup_commit_charge_lrucare+0x56/0x180
[ 8.459747] [<ffffffff810430c9>] ? sub_preempt_count+0xa9/0xe0
[ 8.459751] [<ffffffff81147d08>] mem_cgroup_cache_charge+0xd8/0xe0
[ 8.459756] [<ffffffff81103a49>] add_to_page_cache_locked+0x49/0x100
[ 8.459760] [<ffffffff8110344f>] ? find_get_page+0xdf/0x1a0
[ 8.459764] [<ffffffff81103b22>] add_to_page_cache_lru+0x22/0x50
[ 8.459767] [<ffffffff81103ca5>] do_read_cache_page+0x75/0x1a0
[ 8.459774] [<ffffffff812695f0>] ? nfs_follow_link+0xc0/0xc0
[ 8.459777] [<ffffffff81103e1c>] read_cache_page_async+0x1c/0x20
[ 8.459781] [<ffffffff81103e2e>] read_cache_page+0xe/0x20
[ 8.459785] [<ffffffff81269589>] nfs_follow_link+0x59/0xc0
[ 8.459789] [<ffffffff8115de07>] path_openat+0x2a7/0x470
[ 8.459793] [<ffffffff8115e0e9>] do_filp_open+0x49/0xa0
[ 8.459798] [<ffffffff81155b22>] open_exec+0x32/0xf0
[ 8.459803] [<ffffffff811a3e0b>] load_elf_binary+0x85b/0x1d30
[ 8.459810] [<ffffffff81096d55>] ? __lock_acquire+0x4f5/0xbf0
[ 8.459816] [<ffffffff8100a819>] ? native_sched_clock+0x29/0x80
[ 8.459821] [<ffffffff8108429f>] ? local_clock+0x4f/0x60
[ 8.459824] [<ffffffff815f4658>] ? rt_spin_lock_slowunlock+0x78/0x80
[ 8.459828] [<ffffffff81091a39>] ? trace_hardirqs_off_caller+0x29/0x120
[ 8.459832] [<ffffffff8109119e>] ? put_lock_stats+0xe/0x40
[ 8.459836] [<ffffffff815f4658>] ? rt_spin_lock_slowunlock+0x78/0x80
[ 8.459840] [<ffffffff811a35b0>] ? elf_map+0x1d0/0x1d0
[ 8.459843] [<ffffffff810430c9>] ? sub_preempt_count+0xa9/0xe0
[ 8.459847] [<ffffffff811a35b0>] ? elf_map+0x1d0/0x1d0
[ 8.459850] [<ffffffff81154748>] search_binary_handler+0x1c8/0x4b0
[ 8.459854] [<ffffffff811545d7>] ? search_binary_handler+0x57/0x4b0
[ 8.459858] [<ffffffff81156876>] do_execve_common+0x276/0x330
[ 8.459862] [<ffffffff811569ba>] do_execve+0x3a/0x40
[ 8.459866] [<ffffffff8100bdca>] sys_execve+0x4a/0x80
[ 8.459871] [<ffffffff815f85e8>] kernel_execve+0x68/0xd0
[ 8.459877] [<ffffffff81000333>] ? run_init_process+0x23/0x30
[ 8.459881] [<ffffffff81000398>] init_post+0x58/0xd0
[ 8.459888] [<ffffffff81b5e6bd>] kernel_init+0x156/0x160
[ 8.459892] [<ffffffff815f8574>] kernel_thread_helper+0x4/0x10
[ 8.459896] [<ffffffff810423bc>] ? finish_task_switch+0x8c/0x110
[ 8.459900] [<ffffffff815f5bab>] ? _raw_spin_unlock_irq+0x3b/0x70
[ 8.459903] [<ffffffff815f5f61>] ? retint_restore_args+0xe/0xe
[ 8.459907] [<ffffffff81b5e567>] ? parse_early_options+0x20/0x20
[ 8.459911] [<ffffffff815f8570>] ? gs_change+0xb/0xb
Thanks,
Yong
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [ANNOUNCE] 3.2-rc1-rt2
2011-11-15 8:40 ` Yong Zhang
@ 2011-11-15 13:52 ` Luis Henriques
2011-11-16 9:16 ` [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events() Yong Zhang
1 sibling, 0 replies; 15+ messages in thread
From: Luis Henriques @ 2011-11-15 13:52 UTC (permalink / raw)
To: Yong Zhang; +Cc: Thomas Gleixner, LKML, linux-rt-users
On Tue, Nov 15, 2011 at 04:40:59PM +0800, Yong Zhang wrote:
> Got below warning:
>
> [ 8.459702] BUG: sleeping function called from invalid context at linux/kernel/rtmutex.c:645
> [ 8.459705] in_atomic(): 1, irqs_disabled(): 0, pid: 1, name: swapper/0
> [ 8.459707] 1 lock held by swapper/0/1:
> [ 8.459708] #0: (&sig->cred_guard_mutex){+.+.+.}, at: [<ffffffff81154c45>] prepare_bprm_creds+0x35/0x80
> [ 8.459717] Pid: 1, comm: swapper/0 Not tainted 3.2.0-rc1-rt2-11311-g3c4c0e7-dirty #10
> [ 8.459718] Call Trace:
I'm hitting the same issue. Here's the trace:
[ 0.915638] BUG: sleeping function called from invalid context at /home/luis.henriques/git-trees/linux-2.6/kernel/rtmutex.c:645
[ 0.915641] in_atomic(): 1, irqs_disabled(): 0, pid: 33, name: busybox
[ 0.915642] 1 lock held by busybox/33:
[ 0.915643] #0: (&mm->page_table_lock){+.+...}, at: [<ffffffff81119ec3>] unmap_vmas+0x333/0x6d0
[ 0.915664] Pid: 33, comm: busybox Not tainted 3.2.0-rc1-rt2+ #1
[ 0.915665] Call Trace:
[ 0.915674] [<ffffffff810497ba>] __might_sleep+0x12a/0x1e0
[ 0.915687] [<ffffffff81300f74>] rt_spin_lock+0x24/0x60
[ 0.915692] [<ffffffff8113efcd>] memcg_check_events.clone.33+0x14d/0x250
[ 0.915696] [<ffffffff8113f1bf>] __mem_cgroup_uncharge_common+0xef/0x270
[ 0.915698] [<ffffffff8114311f>] mem_cgroup_uncharge_page+0x2f/0x40
[ 0.915701] [<ffffffff81125410>] page_remove_rmap+0x70/0x90
[ 0.915703] [<ffffffff81119fbf>] unmap_vmas+0x42f/0x6d0
[ 0.915705] [<ffffffff811229d9>] exit_mmap+0xa9/0x120
[ 0.915710] [<ffffffff8104b9e0>] mmput+0x50/0x100
[ 0.915713] [<ffffffff81052c5b>] exit_mm+0x11b/0x140
[ 0.915718] [<ffffffff8109cb6d>] ? acct_collect+0x17d/0x1c0
[ 0.915720] [<ffffffff81053518>] do_exit+0x678/0x930
[ 0.915722] [<ffffffff8130224e>] ? retint_swapgs+0xe/0x13
[ 0.915724] [<ffffffff81053881>] do_group_exit+0x61/0xe0
[ 0.915726] [<ffffffff81053917>] sys_exit_group+0x17/0x20
[ 0.915728] [<ffffffff81302aeb>] system_call_fastpath+0x16/0x1b
Cheers,
--
Luis Henriques
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events()
2011-11-15 8:40 ` Yong Zhang
2011-11-15 13:52 ` Luis Henriques
@ 2011-11-16 9:16 ` Yong Zhang
2011-11-16 14:12 ` Steven Rostedt
1 sibling, 1 reply; 15+ messages in thread
From: Yong Zhang @ 2011-11-16 9:16 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: LKML, linux-rt-users, Steven Rostedt, Peter Zijlstra
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.
BUG: sleeping function called from invalid context at linux/kernel/rtmutex.c:645
in_atomic(): 1, irqs_disabled(): 0, pid: 1, name: swapper/0
1 lock held by swapper/0/1:
#0: (&sig->cred_guard_mutex){+.+.+.}, at: [<ffffffff81154c45>] prepare_bprm_creds+0x35/0x80
Pid: 1, comm: swapper/0 Not tainted 3.2.0-rc1-rt2-11311-g3c4c0e7-dirty #10
Call Trace:
[<ffffffff81040c4e>] __might_sleep+0x12e/0x1e0
[<ffffffff815f4d74>] rt_spin_lock+0x24/0x60
[<ffffffff8114506e>] memcg_check_events+0x11e/0x230
[<ffffffff81146c5a>] T.1144+0x8a/0xf0
[<ffffffff81146d16>] __mem_cgroup_commit_charge_lrucare+0x56/0x180
[<ffffffff810430c9>] ? sub_preempt_count+0xa9/0xe0
[<ffffffff81147d08>] mem_cgroup_cache_charge+0xd8/0xe0
[<ffffffff81103a49>] add_to_page_cache_locked+0x49/0x100
[<ffffffff8110344f>] ? find_get_page+0xdf/0x1a0
[<ffffffff81103b22>] add_to_page_cache_lru+0x22/0x50
[<ffffffff81103ca5>] do_read_cache_page+0x75/0x1a0
[<ffffffff812695f0>] ? nfs_follow_link+0xc0/0xc0
[<ffffffff81103e1c>] read_cache_page_async+0x1c/0x20
[<ffffffff81103e2e>] read_cache_page+0xe/0x20
[<ffffffff81269589>] nfs_follow_link+0x59/0xc0
[<ffffffff8115de07>] path_openat+0x2a7/0x470
[<ffffffff8115e0e9>] do_filp_open+0x49/0xa0
[<ffffffff81155b22>] open_exec+0x32/0xf0
[<ffffffff811a3e0b>] load_elf_binary+0x85b/0x1d30
[<ffffffff81096d55>] ? __lock_acquire+0x4f5/0xbf0
[<ffffffff8100a819>] ? native_sched_clock+0x29/0x80
[<ffffffff8108429f>] ? local_clock+0x4f/0x60
[<ffffffff815f4658>] ? rt_spin_lock_slowunlock+0x78/0x80
[<ffffffff81091a39>] ? trace_hardirqs_off_caller+0x29/0x120
[<ffffffff8109119e>] ? put_lock_stats+0xe/0x40
[<ffffffff815f4658>] ? rt_spin_lock_slowunlock+0x78/0x80
[<ffffffff811a35b0>] ? elf_map+0x1d0/0x1d0
[<ffffffff810430c9>] ? sub_preempt_count+0xa9/0xe0
[<ffffffff811a35b0>] ? elf_map+0x1d0/0x1d0
[<ffffffff81154748>] search_binary_handler+0x1c8/0x4b0
[<ffffffff811545d7>] ? search_binary_handler+0x57/0x4b0
[<ffffffff81156876>] do_execve_common+0x276/0x330
[<ffffffff811569ba>] do_execve+0x3a/0x40
[<ffffffff8100bdca>] sys_execve+0x4a/0x80
[<ffffffff815f85e8>] kernel_execve+0x68/0xd0
[<ffffffff81000333>] ? run_init_process+0x23/0x30
[<ffffffff81000398>] init_post+0x58/0xd0
[<ffffffff81b5e6bd>] kernel_init+0x156/0x160
[<ffffffff815f8574>] kernel_thread_helper+0x4/0x10
[<ffffffff810423bc>] ? finish_task_switch+0x8c/0x110
[<ffffffff815f5bab>] ? _raw_spin_unlock_irq+0x3b/0x70
[<ffffffff815f5f61>] ? retint_restore_args+0xe/0xe
[<ffffffff81b5e567>] ? parse_early_options+0x20/0x20
[<ffffffff815f8570>] ? gs_change+0xb/0xb
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
---
mm/memcontrol.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6aff93c..afa1954 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -722,7 +722,7 @@ static void __mem_cgroup_target_update(struct mem_cgroup *memcg, int target)
*/
static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
{
- preempt_disable();
+ migrate_disable();
/* threshold event is triggered in finer grain than soft limit */
if (unlikely(__memcg_event_check(memcg, MEM_CGROUP_TARGET_THRESH))) {
mem_cgroup_threshold(memcg);
@@ -742,7 +742,7 @@ static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
}
#endif
}
- preempt_enable();
+ migrate_enable();
}
static struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
--
1.7.5.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events()
2011-11-16 9:16 ` [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events() Yong Zhang
@ 2011-11-16 14:12 ` Steven Rostedt
2011-11-16 17:02 ` Thomas Gleixner
2011-11-17 2:15 ` Yong Zhang
0 siblings, 2 replies; 15+ messages in thread
From: Steven Rostedt @ 2011-11-16 14:12 UTC (permalink / raw)
To: Yong Zhang; +Cc: Thomas Gleixner, LKML, linux-rt-users, Peter Zijlstra
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.
>
> mm/memcontrol.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6aff93c..afa1954 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -722,7 +722,7 @@ static void __mem_cgroup_target_update(struct mem_cgroup *memcg, int target)
> */
> static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
> {
> - preempt_disable();
> + migrate_disable();
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
> /* threshold event is triggered in finer grain than soft limit */
> if (unlikely(__memcg_event_check(memcg, MEM_CGROUP_TARGET_THRESH))) {
> mem_cgroup_threshold(memcg);
> @@ -742,7 +742,7 @@ static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
> }
> #endif
> }
> - preempt_enable();
> + migrate_enable();
> }
>
> static struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events()
2011-11-16 14:12 ` Steven Rostedt
@ 2011-11-16 17:02 ` Thomas Gleixner
2011-11-16 17:18 ` Luis Henriques
2011-11-17 3:03 ` Yong Zhang
2011-11-17 2:15 ` Yong Zhang
1 sibling, 2 replies; 15+ messages in thread
From: Thomas Gleixner @ 2011-11-16 17:02 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Yong Zhang, LKML, linux-rt-users, Peter Zijlstra
On Wed, 16 Nov 2011, Steven Rostedt wrote:
> 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.
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
===================================================================
--- linux-3.2.orig/mm/memcontrol.c
+++ linux-3.2/mm/memcontrol.c
@@ -49,6 +49,8 @@
#include <linux/page_cgroup.h>
#include <linux/cpu.h>
#include <linux/oom.h>
+#include <linux/locallock.h>
+
#include "internal.h"
#include <asm/uaccess.h>
@@ -363,6 +365,8 @@ enum charge_type {
#define MEM_CGROUP_RECLAIM_SOFT_BIT 0x2
#define MEM_CGROUP_RECLAIM_SOFT (1 << MEM_CGROUP_RECLAIM_SOFT_BIT)
+static DEFINE_LOCAL_IRQ_LOCK(stats_lock);
+
static void mem_cgroup_get(struct mem_cgroup *memcg);
static void mem_cgroup_put(struct mem_cgroup *memcg);
static struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *memcg);
@@ -619,7 +623,7 @@ static unsigned long mem_cgroup_read_eve
static void mem_cgroup_charge_statistics(struct mem_cgroup *memcg,
bool file, int nr_pages)
{
- preempt_disable();
+ local_lock(stats_lock);
if (file)
__this_cpu_add(memcg->stat->count[MEM_CGROUP_STAT_CACHE],
@@ -638,7 +642,7 @@ static void mem_cgroup_charge_statistics
__this_cpu_add(memcg->stat->events[MEM_CGROUP_EVENTS_COUNT], nr_pages);
- preempt_enable();
+ local_unlock(stats_lock);
}
unsigned long
@@ -722,7 +726,7 @@ static void __mem_cgroup_target_update(s
*/
static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
{
- preempt_disable();
+ local_lock(stats_lock);
/* threshold event is triggered in finer grain than soft limit */
if (unlikely(__memcg_event_check(memcg, MEM_CGROUP_TARGET_THRESH))) {
mem_cgroup_threshold(memcg);
@@ -742,7 +746,7 @@ static void memcg_check_events(struct me
}
#endif
}
- preempt_enable();
+ local_unlock(stats_lock);
}
static struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
@@ -2608,10 +2612,10 @@ static int mem_cgroup_move_account(struc
if (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)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events()
2011-11-16 17:02 ` Thomas Gleixner
@ 2011-11-16 17:18 ` Luis Henriques
2011-11-16 23:48 ` KAMEZAWA Hiroyuki
2011-11-17 3:03 ` Yong Zhang
1 sibling, 1 reply; 15+ messages in thread
From: Luis Henriques @ 2011-11-16 17:18 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Steven Rostedt, Yong Zhang, LKML, linux-rt-users, Peter Zijlstra
On Wed, Nov 16, 2011 at 06:02:42PM +0100, Thomas Gleixner wrote:
> On Wed, 16 Nov 2011, Steven Rostedt wrote:
> > 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.
>
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events()
2011-11-16 17:18 ` Luis Henriques
@ 2011-11-16 23:48 ` KAMEZAWA Hiroyuki
2011-11-17 0:23 ` Steven Rostedt
0 siblings, 1 reply; 15+ messages in thread
From: KAMEZAWA Hiroyuki @ 2011-11-16 23:48 UTC (permalink / raw)
To: Luis Henriques
Cc: Thomas Gleixner, Steven Rostedt, Yong Zhang, LKML, linux-rt-users,
Peter Zijlstra
On Wed, 16 Nov 2011 17:18:09 +0000
Luis Henriques <henrix@camandro.org> wrote:
> On Wed, Nov 16, 2011 at 06:02:42PM +0100, Thomas Gleixner wrote:
> > On Wed, 16 Nov 2011, Steven Rostedt wrote:
> > > 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.
> >
> > 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events()
2011-11-16 23:48 ` KAMEZAWA Hiroyuki
@ 2011-11-17 0:23 ` Steven Rostedt
0 siblings, 0 replies; 15+ messages in thread
From: Steven Rostedt @ 2011-11-17 0:23 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: Luis Henriques, Thomas Gleixner, Yong Zhang, LKML, linux-rt-users,
Peter Zijlstra
On Thu, 2011-11-17 at 08:48 +0900, KAMEZAWA Hiroyuki wrote:
> > 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events()
2011-11-16 14:12 ` Steven Rostedt
2011-11-16 17:02 ` Thomas Gleixner
@ 2011-11-17 2:15 ` Yong Zhang
1 sibling, 0 replies; 15+ messages in thread
From: Yong Zhang @ 2011-11-17 2:15 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Thomas Gleixner, LKML, linux-rt-users, Peter Zijlstra
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:
> > 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.
> >
>
> > mm/memcontrol.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 6aff93c..afa1954 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -722,7 +722,7 @@ static void __mem_cgroup_target_update(struct mem_cgroup *memcg, int target)
> > */
> > static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
> > {
> > - preempt_disable();
> > + migrate_disable();
>
> 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.
>
> Perhaps a local_lock would work.
Yeah, will try tglx's patch later.
Thanks,
Yong
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events()
2011-11-16 17:02 ` Thomas Gleixner
2011-11-16 17:18 ` Luis Henriques
@ 2011-11-17 3:03 ` Yong Zhang
2011-11-17 10:25 ` Thomas Gleixner
1 sibling, 1 reply; 15+ messages in thread
From: Yong Zhang @ 2011-11-17 3:03 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Steven Rostedt, LKML, linux-rt-users, Peter Zijlstra
On Wed, Nov 16, 2011 at 06:02:42PM +0100, Thomas Gleixner wrote:
> On Wed, 16 Nov 2011, Steven Rostedt wrote:
> > 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.
>
> Yes, that's the only sensible option for now. Untested patch below.
Works for me.
Thanks,
Yong
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events()
2011-11-17 3:03 ` Yong Zhang
@ 2011-11-17 10:25 ` Thomas Gleixner
2011-11-17 11:44 ` Luis Henriques
2011-11-18 7:08 ` Yong Zhang
0 siblings, 2 replies; 15+ messages in thread
From: Thomas Gleixner @ 2011-11-17 10:25 UTC (permalink / raw)
To: Yong Zhang; +Cc: Steven Rostedt, LKML, linux-rt-users, Peter Zijlstra
On Thu, 17 Nov 2011, Yong Zhang wrote:
> On Wed, Nov 16, 2011 at 06:02:42PM +0100, Thomas Gleixner wrote:
> > On Wed, 16 Nov 2011, Steven Rostedt wrote:
> > > 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.
> >
> > 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 <jweiner@redhat.com>
---
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.
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6aff93c..8e62d3e 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -683,37 +683,32 @@ static unsigned long mem_cgroup_nr_lru_pages(struct mem_cgroup *memcg,
return total;
}
-static bool __memcg_event_check(struct mem_cgroup *memcg, int target)
+static bool mem_cgroup_event_ratelimit(struct mem_cgroup *memcg,
+ enum mem_cgroup_events_target target)
{
unsigned long val, 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);
-}
-
-static void __mem_cgroup_target_update(struct mem_cgroup *memcg, int target)
-{
- unsigned long val, next;
-
- val = __this_cpu_read(memcg->stat->events[MEM_CGROUP_EVENTS_COUNT]);
-
- switch (target) {
- case MEM_CGROUP_TARGET_THRESH:
- next = val + THRESHOLDS_EVENTS_TARGET;
- break;
- case MEM_CGROUP_TARGET_SOFTLIMIT:
- next = val + SOFTLIMIT_EVENTS_TARGET;
- break;
- case MEM_CGROUP_TARGET_NUMAINFO:
- next = val + NUMAINFO_EVENTS_TARGET;
- break;
- default:
- return;
+ if ((long)next - (long)val < 0) {
+ switch (target) {
+ case MEM_CGROUP_TARGET_THRESH:
+ next = val + THRESHOLDS_EVENTS_TARGET;
+ break;
+ case MEM_CGROUP_TARGET_SOFTLIMIT:
+ next = val + SOFTLIMIT_EVENTS_TARGET;
+ break;
+ case MEM_CGROUP_TARGET_NUMAINFO:
+ next = val + NUMAINFO_EVENTS_TARGET;
+ break;
+ default:
+ break;
+ }
+ __this_cpu_write(memcg->stat->targets[target], next);
+ return true;
}
-
- __this_cpu_write(memcg->stat->targets[target], next);
+ return false;
}
/*
@@ -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))) {
+ bool do_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();
}
static struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events()
2011-11-17 10:25 ` Thomas Gleixner
@ 2011-11-17 11:44 ` Luis Henriques
2011-11-17 11:59 ` Thomas Gleixner
2011-11-18 7:08 ` Yong Zhang
1 sibling, 1 reply; 15+ messages in thread
From: Luis Henriques @ 2011-11-17 11:44 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Yong Zhang, Steven Rostedt, LKML, linux-rt-users, Peter Zijlstra
On Thu, Nov 17, 2011 at 11:25:56AM +0100, Thomas Gleixner wrote:
> Johannes came up with a different solution. Could you please give it a try?
Again, I've run a quick test and it looks like this solves the issue.
Cheers,
--
Luis Henriques
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events()
2011-11-17 11:44 ` Luis Henriques
@ 2011-11-17 11:59 ` Thomas Gleixner
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Gleixner @ 2011-11-17 11:59 UTC (permalink / raw)
To: Luis Henriques
Cc: Yong Zhang, Steven Rostedt, LKML, linux-rt-users, Peter Zijlstra,
Johannes Weiner
Luis,
On Thu, 17 Nov 2011, Luis Henriques wrote:
> On Thu, Nov 17, 2011 at 11:25:56AM +0100, Thomas Gleixner wrote:
> > Johannes came up with a different solution. Could you please give it a try?
>
> Again, I've run a quick test and it looks like this solves the issue.
thanks for testing. That solution is way better than the local_lock
based one.
Thanks,
tglx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events()
2011-11-17 10:25 ` Thomas Gleixner
2011-11-17 11:44 ` Luis Henriques
@ 2011-11-18 7:08 ` Yong Zhang
1 sibling, 0 replies; 15+ messages in thread
From: Yong Zhang @ 2011-11-18 7:08 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Steven Rostedt, LKML, linux-rt-users, Peter Zijlstra
On Thu, Nov 17, 2011 at 11:25:56AM +0100, Thomas Gleixner wrote:
> On Thu, 17 Nov 2011, Yong Zhang wrote:
>
> > On Wed, Nov 16, 2011 at 06:02:42PM +0100, Thomas Gleixner wrote:
> > > On Wed, 16 Nov 2011, Steven Rostedt wrote:
> > > > 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.
> > >
> > > 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
>
> 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 <jweiner@redhat.com>
> ---
> 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.
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6aff93c..8e62d3e 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -683,37 +683,32 @@ static unsigned long mem_cgroup_nr_lru_pages(struct mem_cgroup *memcg,
> return total;
> }
>
> -static bool __memcg_event_check(struct mem_cgroup *memcg, int target)
> +static bool mem_cgroup_event_ratelimit(struct mem_cgroup *memcg,
> + enum mem_cgroup_events_target target)
> {
> unsigned long val, 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);
> -}
> -
> -static void __mem_cgroup_target_update(struct mem_cgroup *memcg, int target)
> -{
> - unsigned long val, next;
> -
> - val = __this_cpu_read(memcg->stat->events[MEM_CGROUP_EVENTS_COUNT]);
> -
> - switch (target) {
> - case MEM_CGROUP_TARGET_THRESH:
> - next = val + THRESHOLDS_EVENTS_TARGET;
> - break;
> - case MEM_CGROUP_TARGET_SOFTLIMIT:
> - next = val + SOFTLIMIT_EVENTS_TARGET;
> - break;
> - case MEM_CGROUP_TARGET_NUMAINFO:
> - next = val + NUMAINFO_EVENTS_TARGET;
> - break;
> - default:
> - return;
> + if ((long)next - (long)val < 0) {
> + switch (target) {
> + case MEM_CGROUP_TARGET_THRESH:
> + next = val + THRESHOLDS_EVENTS_TARGET;
> + break;
> + case MEM_CGROUP_TARGET_SOFTLIMIT:
> + next = val + SOFTLIMIT_EVENTS_TARGET;
> + break;
> + case MEM_CGROUP_TARGET_NUMAINFO:
> + next = val + NUMAINFO_EVENTS_TARGET;
> + break;
> + default:
> + break;
> + }
> + __this_cpu_write(memcg->stat->targets[target], next);
> + return true;
> }
> -
> - __this_cpu_write(memcg->stat->targets[target], next);
> + return false;
> }
>
> /*
> @@ -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))) {
> + bool do_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();
> }
>
> static struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *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/
--
Only stand for myself
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-11-18 7:08 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-15 1:15 [ANNOUNCE] 3.2-rc1-rt2 Thomas Gleixner
2011-11-15 8:40 ` Yong Zhang
2011-11-15 13:52 ` Luis Henriques
2011-11-16 9:16 ` [PATCH -rt] memcg: use migrate_disable()/migrate_enable( ) in memcg_check_events() Yong Zhang
2011-11-16 14:12 ` Steven Rostedt
2011-11-16 17:02 ` Thomas Gleixner
2011-11-16 17:18 ` Luis Henriques
2011-11-16 23:48 ` KAMEZAWA Hiroyuki
2011-11-17 0:23 ` Steven Rostedt
2011-11-17 3:03 ` Yong Zhang
2011-11-17 10:25 ` Thomas Gleixner
2011-11-17 11:44 ` Luis Henriques
2011-11-17 11:59 ` Thomas Gleixner
2011-11-18 7:08 ` Yong Zhang
2011-11-17 2:15 ` Yong Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).