Re: sched: horrible way to detect whether a task has been preempted

[Josh Poimboeuf]

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