linux-kernel - Re: [RFC PATCH for 4.21 06/16] cpu_opv: Provide cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1102241735.877.1539792591430.JavaMail.zimbra@efficios.com>
Date:   Wed, 17 Oct 2018 12:09:51 -0400 (EDT)
From:   Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:     Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        Boqun Feng <boqun.feng@...il.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-api <linux-api@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andy Lutomirski <luto@...capital.net>,
        Dave Watson <davejwatson@...com>, Paul Turner <pjt@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Russell King <linux@....linux.org.uk>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, Andi Kleen <andi@...stfloor.org>,
        Chris Lameter <cl@...ux.com>, Ben Maurer <bmaurer@...com>,
        rostedt <rostedt@...dmis.org>,
        Josh Triplett <josh@...htriplett.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Joel Fernandes <joelaf@...gle.com>
Subject: Re: [RFC PATCH for 4.21 06/16] cpu_opv: Provide cpu_opv system call
 (v8)

----- On Oct 17, 2018, at 11:11 AM, Mathieu Desnoyers mathieu.desnoyers@...icios.com wrote:

> ----- On Oct 17, 2018, at 3:19 AM, Srikar Dronamraju srikar@...ux.vnet.ibm.com
> wrote:
> 
>> Hi Mathieu,
>> 
>>> +static int do_cpu_opv(struct cpu_op *cpuop, int cpuopcnt,
>>> +		      struct cpu_opv_vaddr *vaddr_ptrs, int cpu)
>>> +{
>>> +	struct mm_struct *mm = current->mm;
>>> +	int ret;
>>> +
>>> +retry:
>>> +	if (cpu != raw_smp_processor_id()) {
>>> +		ret = push_task_to_cpu(current, cpu);
>>> +		if (ret)
>>> +			goto check_online;
>>> +	}
>>> +	down_read(&mm->mmap_sem);
>>> +	ret = vaddr_ptrs_check(vaddr_ptrs);
>>> +	if (ret)
>>> +		goto end;
>>> +	preempt_disable();
>>> +	if (cpu != smp_processor_id()) {
>>> +		preempt_enable();
>>> +		up_read(&mm->mmap_sem);
>>> +		goto retry;
>>> +	}
>> 
>> If we have a higher priority task/s either pinned to the cpu, dont we end up
>> in busy-looping till the task exits/sleeps?
> 
> You're right!
> 
> How about we ditch the thread migration altogether, and simply perform
> the cpu_opv operations in a IPI handler ?
> 
> This is possible now that cpu_opv uses a temporary vmap() rather than
> try to touch the user-space page through the current thread's page table.
> 
> Thoughts ?

Here is the associated implementation on top of this patchset:

commit 759c5a8860d867091e168900329f0955e5101989
Author: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Date:   Wed Oct 17 11:32:02 2018 -0400

    cpu opv: use ipi

diff --git a/kernel/cpu_opv.c b/kernel/cpu_opv.c
index db144b71d51a..30405e0cc049 100644
--- a/kernel/cpu_opv.c
+++ b/kernel/cpu_opv.c
@@ -31,6 +31,7 @@
 #include <linux/mm.h>
 #include <linux/vmalloc.h>
 #include <linux/atomic.h>
+#include <linux/smp.h>
 #include <asm/ptrace.h>
 #include <asm/byteorder.h>
 #include <asm/cacheflush.h>
@@ -1039,41 +1040,48 @@ static int vaddr_ptrs_check(struct cpu_opv_vaddr *vaddr_ptrs)
        return 0;
 }

+struct opv_ipi_args {
+       struct cpu_op *cpuop;
+       int cpuopcnt;
+       int ret;
+};
+
+static void cpu_opv_ipi(void *info)
+{
+       struct opv_ipi_args *args = info;
+
+       rseq_preempt(current);
+       args->ret = __do_cpu_opv(args->cpuop, args->cpuopcnt);
+}
+
 static int do_cpu_opv(struct cpu_op *cpuop, int cpuopcnt,
                      struct cpu_opv_vaddr *vaddr_ptrs, int cpu)
 {
        struct mm_struct *mm = current->mm;
+       struct opv_ipi_args args = {
+               .cpuop = cpuop,
+               .cpuopcnt = cpuopcnt,
+       };
        int ret;

 retry:
-       if (cpu != raw_smp_processor_id()) {
-               ret = push_task_to_cpu(current, cpu);
-               if (ret)
-                       goto check_online;
-       }
+       if (!cpumask_test_cpu(cpu, &current->cpus_allowed))
+               return -EINVAL;
        down_read(&mm->mmap_sem);
        ret = vaddr_ptrs_check(vaddr_ptrs);
        if (ret)
                goto end;
-       preempt_disable();
-       if (cpu != smp_processor_id()) {
-               preempt_enable();
+       ret = smp_call_function_single(cpu, cpu_opv_ipi, &args, 1);
+       if (ret) {
                up_read(&mm->mmap_sem);
-               goto retry;
+               goto check_online;
        }
-       ret = __do_cpu_opv(cpuop, cpuopcnt);
-       preempt_enable();
+       ret = args.ret;
 end:
        up_read(&mm->mmap_sem);
        return ret;

 check_online:
-       /*
-        * push_task_to_cpu() returns -EINVAL if the requested cpu is not part
-        * of the current thread's cpus_allowed mask.
-        */
-       if (ret == -EINVAL)
-               return ret;
        get_online_cpus();
        if (cpu_online(cpu)) {
                put_online_cpus();




-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com