On Thu, 2021-08-26 at 17:18 +0200, Sebastian Andrzej Siewior wrote:
> On 2021-08-22 19:29:00 [+0200], Vlastimil Babka wrote:
> > On 8/22/21 6:35 PM, Mike Galbraith wrote:
> > > On second thought, burn both patches. Making K[AC]SAN work with RT
> > > ain't worth the warts.
> >
> > The stackdepot part of kasan patch will be useful at some point.
> > slub_debug should eventually switch to stackdepot - it almost did in
> > mainline already. I'll try to remember and dig that part up when
> > needed.
> > Thanks.
>
> If you want them dropped, sure.
If you think they're worth having, feel free. The kcsan patch I found
especially icky, and changed it like so, avoiding icky config dependent
branches while giving myself an ever so tiny excuse for looping ;-)
kcsan: Make it RT aware
Converting report_filterlist_lock to raw_spinlock_t lets RT report
problem free, but makes allocations in insert_report_filterlist()
problematic. Solve that by taking the lock after allocation. That
means all configs must check that the situation didn't change while
acquiring if we're to avoid ugly config specific branches, but it
also removes the need for GFP_ATOMIC allocations.
Signed-off-by: Mike Galbraith <efault@xxxxxx>
---
kernel/kcsan/debugfs.c | 74 +++++++++++++++++++++++++++++--------------------
1 file changed, 45 insertions(+), 29 deletions(-)
--- a/kernel/kcsan/debugfs.c
+++ b/kernel/kcsan/debugfs.c
@@ -53,7 +53,7 @@ static struct {
.sorted = false,
.whitelist = false, /* default is blacklist */
};
-static DEFINE_SPINLOCK(report_filterlist_lock);
+static DEFINE_RAW_SPINLOCK(report_filterlist_lock);
/*
* The microbenchmark allows benchmarking KCSAN core runtime only. To run
@@ -110,7 +110,7 @@ bool kcsan_skip_report_debugfs(unsigned
return false;
func_addr -= offset; /* Get function start */
- spin_lock_irqsave(&report_filterlist_lock, flags);
+ raw_spin_lock_irqsave(&report_filterlist_lock, flags);
if (report_filterlist.used == 0)
goto out;
@@ -127,7 +127,7 @@ bool kcsan_skip_report_debugfs(unsigned
ret = !ret;
out:
- spin_unlock_irqrestore(&report_filterlist_lock, flags);
+ raw_spin_unlock_irqrestore(&report_filterlist_lock, flags);
return ret;
}
@@ -135,9 +135,9 @@ static void set_report_filterlist_whitel
{
unsigned long flags;
- spin_lock_irqsave(&report_filterlist_lock, flags);
+ raw_spin_lock_irqsave(&report_filterlist_lock, flags);
report_filterlist.whitelist = whitelist;
- spin_unlock_irqrestore(&report_filterlist_lock, flags);
+ raw_spin_unlock_irqrestore(&report_filterlist_lock, flags);
}
/* Returns 0 on success, error-code otherwise. */
@@ -145,39 +145,56 @@ static ssize_t insert_report_filterlist(
{
unsigned long flags;
unsigned long addr = kallsyms_lookup_name(func);
- ssize_t ret = 0;
if (!addr) {
pr_err("could not find function: '%s'\n", func);
return -ENOENT;
}
- spin_lock_irqsave(&report_filterlist_lock, flags);
-
+repeat:
if (report_filterlist.addrs == NULL) {
/* initial allocation */
- report_filterlist.addrs =
- kmalloc_array(report_filterlist.size,
- sizeof(unsigned long), GFP_ATOMIC);
- if (report_filterlist.addrs == NULL) {
- ret = -ENOMEM;
- goto out;
+ unsigned long *array = kmalloc_array(report_filterlist.size,
+ sizeof(unsigned long),
+ GFP_KERNEL);
+ if (array == NULL)
+ return -ENOMEM;
+ raw_spin_lock_irqsave(&report_filterlist_lock, flags);
+ if (report_filterlist.addrs != NULL) {
+ /* Someone beat us to it, move along to resize check. */
+ raw_spin_unlock_irqrestore(&report_filterlist_lock, flags);
+ kfree(array);
+ goto repeat;
}
+ report_filterlist.addrs = array;
} else if (report_filterlist.used == report_filterlist.size) {
/* resize filterlist */
- size_t new_size = report_filterlist.size * 2;
- unsigned long *new_addrs =
- krealloc(report_filterlist.addrs,
- new_size * sizeof(unsigned long), GFP_ATOMIC);
-
- if (new_addrs == NULL) {
- /* leave filterlist itself untouched */
- ret = -ENOMEM;
- goto out;
+ size_t old_size = report_filterlist.size;
+ size_t new_size = old_size * 2;
+ unsigned long *new_addrs = krealloc(report_filterlist.addrs,
+ new_size * sizeof(unsigned long),
+ GFP_KERNEL);
+ if (new_addrs == NULL)
+ return -ENOMEM;
+ raw_spin_lock_irqsave(&report_filterlist_lock, flags);
+ if (report_filterlist.size != old_size) {
+ /* Someone else resized, recheck space availability. */
+ raw_spin_unlock_irqrestore(&report_filterlist_lock, flags);
+ kfree(new_addrs);
+ goto repeat;
}
-
report_filterlist.size = new_size;
report_filterlist.addrs = new_addrs;
+ } else {
+ /*
+ * spekulatively, all looked fine, but we now must both take the
+ * lock and verify space availability before proceeding.
+ */
+ raw_spin_lock_irqsave(&report_filterlist_lock, flags);
+ if (report_filterlist.used == report_filterlist.size) {
+ raw_spin_unlock_irqrestore(&report_filterlist_lock, flags);
+ goto repeat;
+ }
}
/* Note: deduplicating should be done in userspace. */
@@ -185,10 +202,9 @@ static ssize_t insert_report_filterlist(
kallsyms_lookup_name(func);
report_filterlist.sorted = false;
-out:
- spin_unlock_irqrestore(&report_filterlist_lock, flags);
+ raw_spin_unlock_irqrestore(&report_filterlist_lock, flags);
- return ret;
+ return 0;
}
static int show_info(struct seq_file *file, void *v)
@@ -204,13 +220,13 @@ static int show_info(struct seq_file *fi
}
/* show filter functions, and filter type */
- spin_lock_irqsave(&report_filterlist_lock, flags);
+ raw_spin_lock_irqsave(&report_filterlist_lock, flags);
seq_printf(file, "\n%s functions: %s\n",
report_filterlist.whitelist ? "whitelisted" : "blacklisted",
report_filterlist.used == 0 ? "none" : "");
for (i = 0; i < report_filterlist.used; ++i)
seq_printf(file, " %ps\n", (void *)report_filterlist.addrs[i]);
- spin_unlock_irqrestore(&report_filterlist_lock, flags);
+ raw_spin_unlock_irqrestore(&report_filterlist_lock, flags);
return 0;
}
