From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1756186AbdDMJCc (ORCPT <rfc822;w@1wt.eu>);
        Thu, 13 Apr 2017 05:02:32 -0400
Received: from merlin.infradead.org ([205.233.59.134]:49032 "EHLO
        merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1750952AbdDMJCa (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 13 Apr 2017 05:02:30 -0400
Date: Thu, 13 Apr 2017 11:02:11 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@kernel.org>,
        Sebastian Siewior <bigeasy@linutronix.de>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        "David S. Miller" <davem@davemloft.net>,
        Fenghua Yu <fenghua.yu@intel.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Lai Jiangshan <jiangshanlai@gmail.com>, Len Brown <lenb@kernel.org>,
        Michael Ellerman <mpe@ellerman.id.au>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>, Tejun Heo <tj@kernel.org>,
        Tony Luck <tony.luck@intel.com>,
        Viresh Kumar <viresh.kumar@linaro.org>
Subject: Re: [patch 00/13] sched/treewide: Clean up various racy task
 affinity issues
Message-ID: <20170413090211.6h5cx2q3odmz5wcj@hirez.programming.kicks-ass.net>
References: <20170412200726.941336635@linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170412200726.941336635@linutronix.de>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Apr 12, 2017 at 10:07:26PM +0200, Thomas Gleixner wrote:
> While dealing with the fallout of the scheduler cleanups on RT, we found
> several racy usage sites of the following scheme:
> 
> 	cpumask_copy(&save_cpus_allowed, &current->cpus_allowed);
> 	set_cpus_allowed_ptr(current, cpumask_of(cpu));
> 	do_stuff();
> 	set_cpus_allowed_ptr(current, &save_cpus_allowed);
> 
> That's racy in two aspects:
> 
> 1) Nothing prevents the CPU from being unplugged after the temporary
>    affinity setting is in place. This results on code being executed on the
>    wrong CPU(s).
> 
> 2) Nothing prevents a concurrent affinity setting from user space. That
>    also results in code being executed on the wrong CPU(s) and the restore
>    of the previous affinity setting overwrites the new one.
> 
> Various variants of cleanups:
> 
>  - Removal, because the calling thread is already guaranteed to run on the
>    correct CPU.
> 
>  - Conversion to smp function calls (simple register read/write)
> 
>  - Conversion to work_on_cpu(). There were even files containing comments
>    to that effect.
> 
>  - The rest needs seperate hotplug protection for work_on_cpu(). To avoid open
>    coding the
> 
> 	get_online_cpus();
> 	if (cpu_online(cpu))
> 		ret = do_stuff();
> 	else
> 		ret = -ENODEV;
> 	put_online_cpus();
> 
>    scheme this series provides a new helper function work_on_cpu_safe()
>    which implements the above.
> 
> Aside of fixing these races this allows to restrict the access to
> current->cpus_allowed with a follow up series.

Looks good, thanks for tackling these!