From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755726AbcILU7o (ORCPT ); Mon, 12 Sep 2016 16:59:44 -0400 Received: from mail-db5eur01on0071.outbound.protection.outlook.com ([104.47.2.71]:57696 "EHLO EUR01-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751441AbcILU7i (ORCPT ); Mon, 12 Sep 2016 16:59:38 -0400 Authentication-Results: infradead.org; dkim=none (message not signed) header.d=none;infradead.org; dmarc=none action=none header.from=mellanox.com; Subject: Re: [PATCH v15 04/13] task_isolation: add initial support To: Andy Lutomirski References: <1471382376-5443-1-git-send-email-cmetcalf@mellanox.com> <1471382376-5443-5-git-send-email-cmetcalf@mellanox.com> <20160829163352.GV10153@twins.programming.kicks-ass.net> <20160830075854.GZ10153@twins.programming.kicks-ass.net> <3f84f736-ed7f-adff-d5f0-4f7db664208f@mellanox.com> <440e20d1-441a-3228-6b37-6e71e9fce47c@mellanox.com> CC: "linux-doc@vger.kernel.org" , Thomas Gleixner , Christoph Lameter , Michal Hocko , Gilad Ben Yossef , Andrew Morton , Linux API , "Viresh Kumar" , Ingo Molnar , "Steven Rostedt" , Tejun Heo , Will Deacon , Rik van Riel , Frederic Weisbecker , "Paul E. McKenney" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , Catalin Marinas , Peter Zijlstra From: Chris Metcalf Message-ID: <84b47c50-6f1b-dcdb-c90e-471d1b4adbac@mellanox.com> Date: Mon, 12 Sep 2016 15:25:04 -0400 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [12.216.194.146] X-ClientProxiedBy: BY2PR07CA0096.namprd07.prod.outlook.com (10.166.107.49) To DB6PR0501MB2760.eurprd05.prod.outlook.com (10.172.226.12) X-MS-Office365-Filtering-Correlation-Id: c712a3fa-e642-4f8d-d317-08d3db428a7f X-Microsoft-Exchange-Diagnostics: 1;DB6PR0501MB2760;2:m9us4XjF2VswsV19GL16J5dUK5/BfjyhZm/XgzFBGV3dkdHVh1dtyYJ2FiljcGO+TmQfDvgPVTEWFX9+kRPA/2OGTMQgNHvaU8/uurNWggfDfDr7/V3ogkgfyVLYxq2fY41Fw34uHq7L0nEUsaNRU9b7AagkRJ8AEOy78VLiKHT3dszWM62TKcTcN1v4nKli;3:ahSfININKu7eMycAQWZC0kqRwbTyyeF/zPVZTS60k7scjZdhEmbhqLdC9T4rOjaHrpJdqI1xw3MtjzKCqVSfNqbZBw+fPNgslrVkBHCTbeHCeypAjB41cjUFKRmA+wQz;25:KTwbtzCGv4zBUxJADvmUOkcgHHdDYv3Kln34CCXQ4TN0Hqd1S7gpmnC2xzoOytXduAOxuXVv89RtXm5UxgjjleKTadgYHSN/Q6ClrWYpHMdXbUknWHabmLVSfPUc3HXQrKdoMVtiPS7+YRB7cwVwlQD7GFsf3yK67gVWblGH1gf33x6AS2yOV6iqcTONqKWpPK5/+I6FUQX1j6fgc0BLpt/RsQiyCO+O4E8bNj/cokQxIcwRg2IvxAt4jox0RampkY6v1eh2sndq35WtsrQ12c5j8bYu+lYLyGx7wm/6iyFwBeCLm7Sb7kYAvvWRnkbxSKUOMaa6RqcHY2/pvWPCMHJKH33C0F/4fSQ8ynTWWh6brV4VSUZHW9qBjvDZio5k6bcGvQGiodvbcYj+F4WnwItza80oApwIvYN3MpSEH0k= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DB6PR0501MB2760; X-Microsoft-Exchange-Diagnostics: 1;DB6PR0501MB2760;31:vhfxJz9TYjab5bkIxMEv/LjFZdqtXL074aUuhLfNQJGu3mocc4gDjSJQMJFTnlbT94ZTwjo9cKtUMtSaSDdWRFReGHfit9XM5SdnIjQg7CJLIUz/ueV9yvzYZGBUAex8DJ+kyFjAhtyD8leRACzik9M8d7TxBR+zRRfBIb05gRmv4PZDKRUbEaRCN+VHqQUGeE8NbKGpEtJwHwTenIKeopqguV+x00hs5tTa6VV2Cgs=;20:y7UxDInYtVcKdngY+t97wfUpSTwbUzYftXQgJp8iV9Q3HrVFeYwLvH6VKftvm9rVSr+FQMBLeFffhu9/Vw18X+LWGiO44CKU0Sby9z7E0rAv/xUm5JmZasd/zOfZs5nw2x/A6RdeSpoGa9r2zfP+i6wyu4Qo7C6UE4WWwSEjzRfy23lYntTHGjoA1iWTmBlqwazEDqg90++BNLJe4eFHuvEdlSD6DH774h4dlDp0x/d/eNSdW6CWAiwTueD57LUIzX1lZ7JML3m6rWpwaut3kZaPQryWpx/ESacZ3MsWw/COklSAdp1wlNtkIcobBZQV1qX22kSrkQVzwzoQb1YsHr0XBjeCw+c+GZyotWERX3gdnUKQrnFHfbA65rE1OHE5bPD/jEHjivwj4Myy53xau3aOzB6F51CjbEVBwG6d714FFG9SPHwr7mBj19EsR+t3nUdL6WFFZj2Z/8DYc03Bpmn5cwKKUnj78YzeXUQYR8c3Fr3Ncpsh2OGW//rimR8S X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(171992500451332); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026);SRVR:DB6PR0501MB2760;BCL:0;PCL:0;RULEID:;SRVR:DB6PR0501MB2760; X-Microsoft-Exchange-Diagnostics: 1;DB6PR0501MB2760;4:ZoP76KDNo8lS0U/MGkJjR/7GbGjJn/EfJa195qGWcIQneXbJJWvAjns3K63Slqeq1UQx0FNXEoC39PkZpNOmk55ZPEfrkCQAh0tzElpksu/rZmE8QNcVwCVy5UhPGdXDSmnD2kG9Q7DALHpCpgFPtNd2WBzJ1CmC/SOlfenQ205VSC3gNqjwSlIwMklWtAJHWSTLTE0rDrCdggYde91CCatHAGUt+w5k/jYr+v7NdpUSEzlBtdGTACcGGNoNQVpOMw35E4EPxAHZ68hv9E1dK5gCN8fTeYVHYFHlLBTz61kV2KNASaOOHb2LIA4iKP61uw1wCTF3zrecgbSCknx1kQtcslvm+buv1Hot/OGRKHifMcQgZGq6Op6mFPqCJedo9pUSU8gKNPMDHVKUI3daFPZEnZKlJywSI8BdS4XzQDhszgiBhGlTLLZ3wtnsuAXnDVVIJJhBPLizHNCxSmHCZQ== X-Forefront-PRVS: 006339698F X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6049001)(6009001)(7916002)(24454002)(377454003)(81166006)(305945005)(5660300001)(8676002)(31696002)(36756003)(77096005)(2950100001)(65826007)(83506001)(7416002)(19580395003)(2906002)(4326007)(5004840100003)(15975445007)(7736002)(19580405001)(7846002)(87386001)(65806001)(189998001)(66066001)(42186005)(33646002)(230700001)(23676002)(4001350100001)(4001520100001)(54356999)(92566002)(3846002)(47776003)(50986999)(76176999)(86362001)(6116002)(64126003)(586003)(50466002)(31686004)(93886004)(110136003)(65956001)(18886065003);DIR:OUT;SFP:1101;SCL:1;SRVR:DB6PR0501MB2760;H:[10.15.7.181];FPR:;SPF:None;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtEQjZQUjA1MDFNQjI3NjA7MjM6aHg3SkZhQ0pjNEJSMTJoUW1NZmx3MW91?= =?utf-8?B?M1NOQ2d6SmFtREJXZjFnRXVaTlNxdVcrTFRJTVdQeHphay9nWUV4c3pkM0Rv?= =?utf-8?B?U2E5RmVOVUMwVzFWK2V4SjRNUzQxY2tKcTBHbGo1WUdYRVRZb0FkUTRRMjc5?= =?utf-8?B?MU9TNXVLWlFSQk9xbklXZGFFRDhZY3JKVE0za3BsbDd4Tk44WWtWQ1duaDZN?= =?utf-8?B?VGw5YnA3VG9BN2tOMWhNWklvbWhrWm1qbkxSdVIxaEZ5NUp6T0ZUT2QvT1po?= =?utf-8?B?VWlpbjkrNm9sUnhrNG5OSGJVTnJRUVIvYWwxRDJTREFwd3YvRWgzT2NWbzFr?= =?utf-8?B?STI5VHNZQW9CemkzcFcvNnhoRmllMkhtei9uNXRkWFdQS2RSd1EyOVF6RWdl?= =?utf-8?B?MnhLNkMyd0t0aGx0eVF5UjE5RlF6TURKTGg3TDBEWjJHOVhzNEFKMVJ4REl5?= =?utf-8?B?RUNEMDVOcFV6UUxrVmYydllQeHJ2OTY5WjVqTFZ4NUxpTmhWM0NKengwNXZN?= =?utf-8?B?UG1Cb0NYNXVIOUM5MGtPVHNvaVR2ZjFwaDM5RDNMbVArS0JOWVgzbzNJR2Zt?= =?utf-8?B?QUY0LzFyNFZNZUFURFRvSnZBdnNnVnNJRFVMNUE1c3RuWUVSbzhPRGxCM0Uz?= =?utf-8?B?ZFFQTHNsR1g5eEZUOXBIOTd2dFo0aktLZ0JDVXdUQjFOVnZzYU9FQ1NsSjRz?= =?utf-8?B?ejh5YTFUSjl6N00yZVk0WGgwUndvN3ZabzdxR3pkNmpXWHdZTThGcWxOVndI?= =?utf-8?B?K1hmNGlwTndKVWV5Y0FpakE1ZFBSTTlweXZpWlRmcUY5RnB3b2tVMm50NGRL?= =?utf-8?B?K3k1R0V1WHJSK1AyeGFqamw3YnFLcEFzczcyb1FqNFRhQy9PVndhejFWQi9x?= =?utf-8?B?UjJpNkM0MWRaN0dFeTQ0MndYaTQ5dTd2MUtKQVcyL2t0L1VCK2czWC9jWDhH?= =?utf-8?B?eDNkWGRYeWg1Zis2WWdqekxzc08yN1c5eHVUV3pJWEtrUUN2SnJqbHdqamJT?= =?utf-8?B?ZXd5VkxQNmQvNFBZdmpQcXRQOHJHU1VWQldtbkJBSFVFRE9tNHZFYnBpUjdX?= =?utf-8?B?K0FYNnpXWk5NbC9lVjVjSitodEpNa21NRXFBYTREQTlleUVCdDVFRXZXTEM1?= =?utf-8?B?OGR0QWh3VTNKTEJVR2pScUlhWlltckUyUldheWl3K2t3V2QzZUhpK1doTWQx?= =?utf-8?B?anNmTCttT2tRdW5HSUVtbVZIN0UrRkFVaEY4SVNuQjB0QnNlMStXR0IzZGtl?= =?utf-8?B?WGFxSjBqcTd1cHUxVTlGb2tlVFF2Z0djL2ZBN0tlVHBXOVNOdnpPc281Y2Rr?= =?utf-8?B?NU5haHE0Wk0zV3N2OWxZUi9xUllQZkIxLzV3T0NpV0xRNmJwV1F5KzdpNklK?= =?utf-8?B?S0o4ZUtKWkV2elIvakgyZVJvczZCS2tjZHdRRkxDN3NoSlMyQ09BbGNoSjBk?= =?utf-8?B?c1d0dTFXL01VRFo2a3U0ejF4TkVCMFIxOGZoYjUxT01TeTBEaU1TOWEvdXA5?= =?utf-8?B?dVZDRHowemxXNlptOHY3eDQ5VHE3YjJMTEJxY0RZTUF3ZmhtdWdKVTNWUnpo?= =?utf-8?B?cEhSUkVmbjZUWGNhaFlqYjZuOFEwWm42N0NHdUE2c0NLSG94V2xBWTRRMnQ0?= =?utf-8?B?eStZaHRmMk8rSk4yWHNhV3QwY3Ria0VtYVhjTjBsY1R1UVZuWFVCQzhEL2Y2?= =?utf-8?B?QXAvZzV3VjA3NUxKdXNCZVRMY3NoOW5ackV1NkplOGlPbS9QSHM1a1JFZkZ2?= =?utf-8?Q?4E3ik8P0uYkBGiKTJy2JBLXyxvErJKu1MSNy7UE=3D?= X-Microsoft-Exchange-Diagnostics: 1;DB6PR0501MB2760;6:SprSx6ssZUhZyKmsQElwHiPFK0jDhQItcjmBhIOJYGyDWGA+01g1upEPEpnbAPxCDbdzGo1tiECPb6zFjCMxFYjEwKHewx7EUNbz7LgpI7+X/UL3azzjGAx41y57310fBDfu7jN8aLaF43786YJgvogWLusJJME21sd6KudPbJE+g7rH+brtiSXDCasNajjMmQ3ylWxgAUfIiPz2iNxCPGm1DBvgTdIkxLM77/FR/C5FBPoUOfMm6+oFyGSgyQ4b1ZO314dJDRT1qo1Lhn6U4Pp0CNen+CFaNtCWat67MtqyCcVFEa+flmc93R0QEr0+ffn0zoDktaQv7WKOMu/ZUg==;5:h+YjCzOWY81IXgyXXvXT0gz6aDvANNlWF+82R/S2lYpp5+hf21BXHVVmp08uuNUhX82lh1kvhflU0FHfrcausGCiTVuGy7lg6sBlJA5Xo5CuuvHDMg4tSQJBhppqKayz5pdwO7tkk6lfVkCReg05oQ==;24:e32OTK3zjz8b6uWHCMR6adegyWJSCtosHNkFlVk8pGlCov3hzSksTbkP9igT+FFp4p8snCvKi3Tyh+3wT4P0c4ADpme9uYi9xY+ZGaq8noQ=;7:F65CCTmSSiVOFWXxiwspky9q/2Jj70rCqkNaAXqdsAnHFPlaRr16agD8UlWL7MkvyZpaE0iU97wLigLIYU0rW+td5acoRzBkiIv8BkFissrt3IKSiMkF+8KpjU6+eVZju1fIXSmWf+WKLc2sRBjEPcMV1OOdt1WS59cyMRBUfxxHtRjuGxDEO6K3Hjw9nJMlbeU2ZewNvAO/Lmq9UMzaGKR9x/YXLdDRenXrl3GQo01bvX2o/uHQ+ByK9FMwdTjv SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Sep 2016 19:25:17.4435 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0501MB2760 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 9/12/2016 1:41 PM, Andy Lutomirski wrote: > On Sep 9, 2016 1:40 PM, "Chris Metcalf" wrote: >> On 9/2/2016 1:28 PM, Andy Lutomirski wrote: >>> On Sep 2, 2016 7:04 AM, "Chris Metcalf" wrote: >>>> On 8/30/2016 3:50 PM, Andy Lutomirski wrote: >>>>> On Tue, Aug 30, 2016 at 12:37 PM, Chris Metcalf wrote: >>>>>> So to pop up a level, what is your actual concern about the existing >>>>>> "do it in a loop" model? The macrology currently in use means there >>>>>> is zero cost if you don't configure TASK_ISOLATION, and the software >>>>>> maintenance cost seems low since the idioms used for task isolation >>>>>> in the loop are generally familiar to people reading that code. >>>>> My concern is that it's not obvious to readers of the code that the >>>>> loop ever terminates. It really ought to, but it's doing something >>>>> very odd. Normally we can loop because we get scheduled out, but >>>>> actually blocking in the return-to-userspace path, especially blocking >>>>> on a condition that doesn't have a wakeup associated with it, is odd. >>>> >>>> True, although, comments :-) >>>> >>>> Regardless, though, this doesn't seem at all weird to me in the >>>> context of the vmstat and lru stuff, though. It's exactly parallel to >>>> the fact that we loop around on checking need_resched and signal, and >>>> in some cases you could imagine multiple loops around when we schedule >>>> out and get a signal, so loop around again, and then another >>>> reschedule event happens during signal processing so we go around >>>> again, etc. Eventually it settles down. It's the same with the >>>> vmstat/lru stuff. >>> Only kind of. >>> >>> When we say, effectively, while (need_resched()) schedule();, we're >>> not waiting for an event or condition per se. We're runnable (in the >>> sense that userspace wants to run and we're not blocked on anything) >>> the entire time -- we're simply yielding to some other thread that is >>> also runnable. So if that loop runs forever, it either means that >>> we're at low priority and we genuinely shouldn't be running or that >>> there's a scheduler bug. >>> >>> If, on the other hand, we say while (not quiesced) schedule(); (or >>> equivalent), we're saying that we're *not* really ready to run and >>> that we're waiting for some condition to change. The condition in >>> question is fairly complicated and won't wake us when we are ready. I >>> can also imagine the scheduler getting rather confused, since, as far >>> as the scheduler knows, we are runnable and we are supposed to be >>> running. >> >> So, how about a code structure like this? >> >> In the main return-to-userspace loop where we check TIF flags, >> we keep the notion of our TIF_TASK_ISOLATION flag that causes >> us to invoke a task_isolation_prepare() routine. This routine >> does the following things: >> >> 1. As you suggested, set a new TIF bit (or equivalent) that says the >> system should no longer create deferred work on this core, and then >> flush any necessary already-deferred work (currently, the LRU cache >> and the vmstat stuff). We never have to go flush the deferred work >> again during this task's return to userspace. Note that this bit can >> only be set on a core marked for task isolation, so it can't be used >> for denial of service type attacks on normal cores that are trying to >> multitask normal Linux processes. > I think it can't be a TIF flag unless you can do the whole mess with > preemption off because, if you get preempted, other tasks on the CPU > won't see the flag. You could do it with a percpu flag, I think. Yes, a percpu flag - you're right. I think it will make sense for this to be a flag declared in linux/isolation.h which can be read by vmstat, LRU, etc. >> 2. Check if the dyntick is stopped, and if not, wait on a completion >> that will be set when it does stop. This means we may schedule out at >> this point, but when we return, the deferred work stuff is still safe >> since your bit is still set, and in principle the dyn tick is >> stopped. >> >> Then, after we disable interrupts and re-read the thread-info flags, >> we check to see if the TIF_TASK_ISOLATION flag is the ONLY flag still >> set that would keep us in the loop. This will always end up happening >> on each return to userspace, since the only thing that actually clears >> the bit is a prctl() call. When that happens we know we are about to >> return to userspace, so we call task_isolation_ready(), which now has >> two things to do: > Would it perhaps be more straightforward to do the stuff before the > loop and not check TIF_TASK_ISOLATION in the loop? We can certainly play around with just not looping in this case, but in particular I can imagine an isolated task entering the kernel and then doing something that requires scheduling a kernel task. We'd clearly like that other task to run before the isolated task returns to userspace. But then, that other task might do something to re-enable the dyntick. That's why we'd like to recheck that dyntick is off in the loop after each potential call to schedule(). >> 1. We check that the dyntick is in fact stopped, since it's possible >> that a race condition led to it being somehow restarted by an interrupt. >> If it is not stopped, we go around the loop again so we can go back in >> to the completion discussed above and wait some more. This may merit >> a WARN_ON or other notice since it seems like people aren't convinced >> there are things that could restart it, but honestly the dyntick stuff >> is complex enough that I think a belt-and-suspenders kind of test here >> at the last minute is just defensive programming. > Seems reasonable. But maybe this could go after the loop and, if the > dyntick is back, it could be treated like any other kernel bug that > interrupts an isolated task? That would preserve more of the existing > code structure. Well, we can certainly try it that way. If I move it out and my testing doesn't trigger the bug, that's at least an initial sign that it might be OK. But I worry/suspect that it will trip up at some point in some use case and we'll have to fix it at that point. > If that works, it could go in user_enter(). Presumably with trace_user_enter() and vtime_user_enter() in __context_tracking_enter()? -- Chris Metcalf, Mellanox Technologies http://www.mellanox.com