Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758144AbcDHObf (ORCPT ); Fri, 8 Apr 2016 10:31:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40977 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752995AbcDHObd (ORCPT ); Fri, 8 Apr 2016 10:31:33 -0400 Date: Fri, 8 Apr 2016 09:31:31 -0500 From: Josh Poimboeuf To: Petr Mladek Cc: Jiri Kosina , Jessica Yu , Miroslav Benes , linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, Vojtech Pavlik Subject: Re: sched: horrible way to detect whether a task has been preempted Message-ID: <20160408143131.46ysnwy6c7i6uxmb@treble.redhat.com> References: <24db5a6ae5b63dfcd2096a12d18e1399a351348e.1458933243.git.jpoimboe@redhat.com> <20160407211525.GB25804@packer-debian-8-amd64.digitalocean.com> <20160407231512.GA27994@packer-debian-8-amd64.digitalocean.com> <20160408080304.GE5218@pathway.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160408080304.GE5218@pathway.suse.cz> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2225 Lines: 51 On Fri, Apr 08, 2016 at 10:03:04AM +0200, Petr Mladek wrote: > On Fri 2016-04-08 09:05:28, Jiri Kosina wrote: > > On Thu, 7 Apr 2016, Jessica Yu wrote: > > > > > > Alternatively, without eating up a TIF_ space, it'd be possible to push a > > > > magic contents on top of the stack in preempt_schedule_irq() (and pop it > > > > once we are returning from there), and if such magic value is detected, we > > > > just don't bother and claim unreliability. > > > > > > Ah, but wouldn't we still have to walk through the frames (i.e. enter > > > the loop in patch 7/14) to look for the magic value in this approach? > > > > The idea was that it'd be located at a place to which saved stack pointer > > of the sleeping task is pointing to (or at a fixed offset from it). > > It is an interesting idea but it looks even more hacky than checking > the frame pointers and return values. > > Checking the stack might be an overkill but we already do this for all > the other patched functions. > > The big advantage about checking the stack is that it does not add > any overhead to the scheduler code, does not eat any TIF flag or > memory. The overhead is only when we are migrating a task and it is > charged to a separate process. My biggest concern about checking the stack for preempt_schedule_irq() is that it's kind of brittle: - What if the preemption code changes such that it's no longer a reliable indicator? For example, what if preempt_schedule_irq() is only called in some places, and a new __preempt_schedule_irq() is called elsewhere? - Or due to some obscure gcc optimization like partial inlining or sibling tail calls, preempt_schedule_irq() doesn't show up on the stack? - Or the code could silently break if there were another static preempt_schedule_irq symbol somewhere (though we could prevent this by searching all symbols to ensure there are no duplicates). These scenarios are unlikely, but they could conceivably happen... Anyway, we really wouldn't have to eat a TIF flag. We could instead add something to task_struct. I can at least propose it. If anybody doesn't like it, maybe they'll suggest something else, or maybe then we can go with checking the stack. -- Josh