* [PATCH] locking/rtmutex: Provide proper spin_is_contended
@ 2022-06-07 9:15 Zhipeng Shi
2022-06-07 9:24 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 4+ messages in thread
From: Zhipeng Shi @ 2022-06-07 9:15 UTC (permalink / raw)
To: peterz, mingo, will, longman, boqun.feng
Cc: tglx, bigeasy, schspa, shengjian.xu, linux-kernel, Zhipeng Shi
Commit d89c70356acf ("locking/core: Remove break_lock field when
CONFIG_GENERIC_LOCKBREAK=y") removed GENERIC_LOCKBREAK, which caused
spin_is_contended depend on the implementation of arch_spin_is_contended.
But now in rt-spinlock, spin_is_contended returns 0 directly.
This causes cond_resched_lock to fail to correctly detect lock contention
in RT-linux. In some scenarios (such as __purge_vmap_area_lazy in vmalloc),
this will cause a large latency.
This patch provides the implementation of spin_is_contended for
rt-spinlock.
Signed-off-by: Zhipeng Shi <zhipeng.shi0@gmail.com>
---
include/linux/rtmutex.h | 2 ++
include/linux/spinlock_rt.h | 13 ++++++++++++-
kernel/locking/rtmutex_common.h | 2 --
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/include/linux/rtmutex.h b/include/linux/rtmutex.h
index 7d049883a08a..cd7ac1785c6a 100644
--- a/include/linux/rtmutex.h
+++ b/include/linux/rtmutex.h
@@ -70,6 +70,8 @@ extern void rt_mutex_debug_task_free(struct task_struct *tsk);
static inline void rt_mutex_debug_task_free(struct task_struct *tsk) { }
#endif
+#define RT_MUTEX_HAS_WAITERS 1UL
+
#define rt_mutex_init(mutex) \
do { \
static struct lock_class_key __key; \
diff --git a/include/linux/spinlock_rt.h b/include/linux/spinlock_rt.h
index 835aedaf68ac..54abf2b50494 100644
--- a/include/linux/spinlock_rt.h
+++ b/include/linux/spinlock_rt.h
@@ -145,7 +145,18 @@ static __always_inline void spin_unlock_irqrestore(spinlock_t *lock,
#define spin_trylock_irqsave(lock, flags) \
__cond_lock(lock, __spin_trylock_irqsave(lock, flags))
-#define spin_is_contended(lock) (((void)(lock), 0))
+/**
+ * spin_is_contended - check if the lock is contended
+ * @lock : Pointer to spinlock structure
+ *
+ * Return: 1 if lock is contended, 0 otherwise
+ */
+static inline int spin_is_contended(spinlock_t *lock)
+{
+ unsigned long *p = (unsigned long *) &lock->lock.owner;
+
+ return (READ_ONCE(*p) & RT_MUTEX_HAS_WAITERS);
+}
static inline int spin_is_locked(spinlock_t *lock)
{
diff --git a/kernel/locking/rtmutex_common.h b/kernel/locking/rtmutex_common.h
index c47e8361bfb5..70c765a26163 100644
--- a/kernel/locking/rtmutex_common.h
+++ b/kernel/locking/rtmutex_common.h
@@ -131,8 +131,6 @@ static inline struct rt_mutex_waiter *task_top_pi_waiter(struct task_struct *p)
pi_tree_entry);
}
-#define RT_MUTEX_HAS_WAITERS 1UL
-
static inline struct task_struct *rt_mutex_owner(struct rt_mutex_base *lock)
{
unsigned long owner = (unsigned long) READ_ONCE(lock->owner);
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] locking/rtmutex: Provide proper spin_is_contended
2022-06-07 9:15 [PATCH] locking/rtmutex: Provide proper spin_is_contended Zhipeng Shi
@ 2022-06-07 9:24 ` Sebastian Andrzej Siewior
2022-06-08 14:24 ` Zhipeng Shi
0 siblings, 1 reply; 4+ messages in thread
From: Sebastian Andrzej Siewior @ 2022-06-07 9:24 UTC (permalink / raw)
To: Zhipeng Shi
Cc: peterz, mingo, will, longman, boqun.feng, tglx, schspa,
shengjian.xu, linux-kernel
On 2022-06-07 17:15:14 [+0800], Zhipeng Shi wrote:
> Commit d89c70356acf ("locking/core: Remove break_lock field when
> CONFIG_GENERIC_LOCKBREAK=y") removed GENERIC_LOCKBREAK, which caused
> spin_is_contended depend on the implementation of arch_spin_is_contended.
> But now in rt-spinlock, spin_is_contended returns 0 directly.
>
> This causes cond_resched_lock to fail to correctly detect lock contention
> in RT-linux. In some scenarios (such as __purge_vmap_area_lazy in vmalloc),
> this will cause a large latency.
>
> This patch provides the implementation of spin_is_contended for
> rt-spinlock.
Do you have a test-case or an example?
The thing if _this_ task has higher priority than other tasks waiting on
this lock then a simple unlock+lock won't do a thing. That is why I
ripped it all out while it was half way done.
> Signed-off-by: Zhipeng Shi <zhipeng.shi0@gmail.com>
Sebastian
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] locking/rtmutex: Provide proper spin_is_contended
2022-06-07 9:24 ` Sebastian Andrzej Siewior
@ 2022-06-08 14:24 ` Zhipeng Shi
0 siblings, 0 replies; 4+ messages in thread
From: Zhipeng Shi @ 2022-06-08 14:24 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: peterz, mingo, will, longman, boqun.feng, tglx, schspa,
shengjian.xu, linux-kernel
On Tue, Jun 07, 2022 at 11:24:56AM +0200, Sebastian Andrzej Siewior wrote:
> On 2022-06-07 17:15:14 [+0800], Zhipeng Shi wrote:
> > Commit d89c70356acf ("locking/core: Remove break_lock field when
> > CONFIG_GENERIC_LOCKBREAK=y") removed GENERIC_LOCKBREAK, which caused
> > spin_is_contended depend on the implementation of arch_spin_is_contended.
> > But now in rt-spinlock, spin_is_contended returns 0 directly.
> >
> > This causes cond_resched_lock to fail to correctly detect lock contention
> > in RT-linux. In some scenarios (such as __purge_vmap_area_lazy in vmalloc),
> > this will cause a large latency.
> >
> > This patch provides the implementation of spin_is_contended for
> > rt-spinlock.
>
> Do you have a test-case or an example?
Sorry for late reply, here is my test-case.
After apply this patch, max-latency of vmalloc reduce from 10+ msec to
200+ usec, this because spin_lock is released halfway through
__purge_vmap_area_lazy.
But today I find if I 'chrt -f 10' to my test script, There will still be
a large latency in the vmalloc process. After trace, it seems update_curr_rt
leads to resched_curr. I haven't figured out the reason behind it, and I'm
still trying to figure out why. I would really appreciate your thoughts
on this issue.
Below is the test module
---------------------------------
#include <linux/module.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/poll.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
#include <trace/events/kmem.h>
#define TEST_MALLOC_SIZE 64
#define MAX_OFF 20000
void* buf[MAX_OFF];
void* fub[MAX_OFF];
int off = 0;
int ffo = 0;
u64 total_time1 = 0;
u64 total_time2 = 0;
u64 total_cnt1 = 0;
u64 total_cnt2 = 0;
u64 alloc_time1[16];
u64 alloc_time2[16];
#define DECIMAL_BASE 10
#define MODULE_NAME "vmalloc_test"
unsigned long long duration_max = 0;
ssize_t vmalloc_test_proc_write(struct file *file,
const char __user *buffer, size_t count, loff_t *pos)
{
char set_str[4] = {0};
uint32_t set_bits = 0;
ssize_t err = 0;
ktime_t time1, time2, delta;
unsigned long long duration;
int i = 0;
err = simple_write_to_buffer((void*)set_str, sizeof(set_str) - 1u,
pos, (const void*)buffer, count);
if (err == 0)
return err;
if (kstrtouint(set_str, DECIMAL_BASE, &set_bits) != 0)
return -EINVAL;
switch (set_bits) {
case 0:
if (off >= MAX_OFF) {
printk("buf number exceed\n");
return -1;
}
time1 = ktime_get();
buf[off] = vmalloc(TEST_MALLOC_SIZE);
if (NULL == buf[off]) {
printk("vmalloc buf failed\n");
return -1;
}
memset(buf[off], 1, TEST_MALLOC_SIZE);
off++;
time2 = ktime_get();
delta = ktime_sub(time2, time1);
duration = (unsigned long long) ktime_to_ns(delta) >> 10;
if (duration > 8000) {
printk("%s:%d, elapse %llu usec\n",
__func__, __LINE__, duration);
return -1;
}
total_cnt1++;
total_time1 += duration;
alloc_time1[fls(duration) - 1]++;
if (duration > duration_max)
duration_max = duration;
break;
case 1:
for (i = 0; i < MAX_OFF; i++) {
if (buf[i] == NULL)
break;
vfree(buf[i]);
buf[i] = NULL;
}
off = 0;
break;
case 2:
if (ffo >= MAX_OFF) {
printk("buf number exceed\n");
return -1;
}
time1 = ktime_get();
fub[ffo] = vmalloc(TEST_MALLOC_SIZE);
if (NULL == fub[ffo]) {
printk("vmalloc buf failed\n");
return -1;
}
memset(fub[ffo], 1, TEST_MALLOC_SIZE);
ffo++;
time2 = ktime_get();
delta = ktime_sub(time2, time1);
duration = (unsigned long long) ktime_to_ns(delta) >> 10;
if (duration > 8000) {
printk("%s:%d, elapse %llu usec\n",
__func__, __LINE__, duration);
//return -1;
}
total_cnt2++;
total_time2 += duration;
alloc_time1[fls(duration)- 1]++;
if (duration > duration_max)
duration_max = duration;
break;
case 3:
for (i = 0; i < MAX_OFF; i++) {
if (fub[i] == NULL)
break;
vfree(fub[i]);
fub[i] = NULL;
}
ffo = 0;
break;
default:
printk("input error\n");
return -EINVAL;
}
return (ssize_t)count;
}
static int32_t vmalloc_test_proc_show(struct seq_file *m, void *v)
{
int i = 0;
seq_printf(m, "max: %llu us\n", duration_max);
seq_printf(m, "avg1: %llu us\n", total_time1/total_cnt1);
seq_printf(m, "avg2: %llu us\n", total_time2/total_cnt2);
for (i = 0; i < 16; i++) {
seq_printf(m, "[%u ~ %u): %llu\n",
1 << i, 1 << (i + 1), alloc_time1[i]);
}
return 0;
}
static int32_t vmalloc_test_fault_injection_proc_open(struct inode *inode,
struct file *file)
{
return single_open(file, vmalloc_test_proc_show, PDE_DATA(inode));
}
static const struct proc_ops vmalloc_test_fault_inject_proc_fops = {
.proc_open = vmalloc_test_fault_injection_proc_open,
.proc_read = seq_read,
.proc_lseek = seq_lseek,
.proc_release = single_release,
.proc_write = vmalloc_test_proc_write,
};
static struct proc_dir_entry *root_irq_dir = NULL;
static struct proc_dir_entry *entry;
#define vmalloc_test_FAULT_INJECTION_MODE 444
static int32_t vmalloc_test_register_procfs(void)
{
if (NULL == root_irq_dir) {
root_irq_dir = proc_mkdir("driver/vmalloc_test", NULL);
if (NULL == root_irq_dir) {
return -EINVAL;
}
}
entry = proc_create_data("test",
vmalloc_test_FAULT_INJECTION_MODE,
root_irq_dir,
&vmalloc_test_fault_inject_proc_fops, NULL);
if (entry == NULL) {
return -ENOMEM;
}
return 0;
}
static int32_t vmalloc_test_unregister_procfs(void)
{
proc_remove(root_irq_dir);
return 0;
}
static s32 __init vmalloc_test_init(void)
{
int ret;
ret = vmalloc_test_register_procfs();
if (ret) {
printk("register proc failed\n");
return -1;
}
return 0;
}
module_init(vmalloc_test_init);/*PRQA S 0605*/
static void __exit vmalloc_test_exit(void)
{
vmalloc_test_unregister_procfs();
return;
}
module_exit(vmalloc_test_exit);/*PRQA S 0605*/
MODULE_AUTHOR("Sun Kaikai<kaikai.sun@horizon.com>");
MODULE_DESCRIPTION("Hobot GDC driver");
MODULE_LICENSE("GPL v2");
-------------------------------
Below is the test script no.1
-------------------------------------
#!/bin/bash
x=0
while true
do
for i in {1..10000}
do
echo 0 > /proc/driver/vmalloc_test/test
if [ $? != 0 ];then
echo 0 > /sys/kernel/debug/tracing/tracing_on
echo 1 > /proc/driver/vmalloc_test/test
exit
fi
done
echo 1 > /proc/driver/vmalloc_test/test
echo "loop $x done"
x=$((x+1))
done
---------------------------------
Below is the test script no.2
------------------------------
#!/bin/bash
x=0
while true
do
for i in {1..10000}
do
echo 2 > /proc/driver/vmalloc_test/test
if [ $? != 0 ];then
echo 0 > /sys/kernel/debug/tracing/tracing_on
echo 3 > /proc/driver/vmalloc_test/test
exit
fi
done
echo 3 > /proc/driver/vmalloc_test/test
echo "loop1 $x done"
x=$((x+1))
done
------------------------------
>
> The thing if _this_ task has higher priority than other tasks waiting on
> this lock then a simple unlock+lock won't do a thing. That is why I
> ripped it all out while it was half way done.
>
> > Signed-off-by: Zhipeng Shi <zhipeng.shi0@gmail.com>
>
> Sebastian
Zhipeng
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] locking/rtmutex: Provide proper spin_is_contended
@ 2022-06-07 9:24 Zhipeng Shi
0 siblings, 0 replies; 4+ messages in thread
From: Zhipeng Shi @ 2022-06-07 9:24 UTC (permalink / raw)
To: peterz, mingo, will, longman, boqun.feng
Cc: tglx, bigeasy, schspa, shengjian.xu, linux-kernel, Zhipeng Shi
Commit d89c70356acf ("locking/core: Remove break_lock field when
CONFIG_GENERIC_LOCKBREAK=y") removed GENERIC_LOCKBREAK, which caused
spin_is_contended depend on the implementation of arch_spin_is_contended.
But now in rt-spinlock, spin_is_contended returns 0 directly.
This causes cond_resched_lock to fail to correctly detect lock contention
in RT-linux. In some scenarios (such as __purge_vmap_area_lazy in vmalloc),
this will cause a large latency.
This patch provides the implementation of spin_is_contended for
rt-spinlock.
Signed-off-by: Zhipeng Shi <zhipeng.shi0@gmail.com>
---
include/linux/rtmutex.h | 2 ++
include/linux/spinlock_rt.h | 13 ++++++++++++-
kernel/locking/rtmutex_common.h | 2 --
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/include/linux/rtmutex.h b/include/linux/rtmutex.h
index 7d049883a08a..cd7ac1785c6a 100644
--- a/include/linux/rtmutex.h
+++ b/include/linux/rtmutex.h
@@ -70,6 +70,8 @@ extern void rt_mutex_debug_task_free(struct task_struct *tsk);
static inline void rt_mutex_debug_task_free(struct task_struct *tsk) { }
#endif
+#define RT_MUTEX_HAS_WAITERS 1UL
+
#define rt_mutex_init(mutex) \
do { \
static struct lock_class_key __key; \
diff --git a/include/linux/spinlock_rt.h b/include/linux/spinlock_rt.h
index 835aedaf68ac..54abf2b50494 100644
--- a/include/linux/spinlock_rt.h
+++ b/include/linux/spinlock_rt.h
@@ -145,7 +145,18 @@ static __always_inline void spin_unlock_irqrestore(spinlock_t *lock,
#define spin_trylock_irqsave(lock, flags) \
__cond_lock(lock, __spin_trylock_irqsave(lock, flags))
-#define spin_is_contended(lock) (((void)(lock), 0))
+/**
+ * spin_is_contended - check if the lock is contended
+ * @lock : Pointer to spinlock structure
+ *
+ * Return: 1 if lock is contended, 0 otherwise
+ */
+static inline int spin_is_contended(spinlock_t *lock)
+{
+ unsigned long *p = (unsigned long *) &lock->lock.owner;
+
+ return (READ_ONCE(*p) & RT_MUTEX_HAS_WAITERS);
+}
static inline int spin_is_locked(spinlock_t *lock)
{
diff --git a/kernel/locking/rtmutex_common.h b/kernel/locking/rtmutex_common.h
index c47e8361bfb5..70c765a26163 100644
--- a/kernel/locking/rtmutex_common.h
+++ b/kernel/locking/rtmutex_common.h
@@ -131,8 +131,6 @@ static inline struct rt_mutex_waiter *task_top_pi_waiter(struct task_struct *p)
pi_tree_entry);
}
-#define RT_MUTEX_HAS_WAITERS 1UL
-
static inline struct task_struct *rt_mutex_owner(struct rt_mutex_base *lock)
{
unsigned long owner = (unsigned long) READ_ONCE(lock->owner);
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-08 14:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-07 9:15 [PATCH] locking/rtmutex: Provide proper spin_is_contended Zhipeng Shi
2022-06-07 9:24 ` Sebastian Andrzej Siewior
2022-06-08 14:24 ` Zhipeng Shi
2022-06-07 9:24 Zhipeng Shi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.