Re: [PATCH v5 2/6] cpu_isolated: add initial support

[Frederic Weisbecker]

On Wed, Aug 12, 2015 at 02:22:09PM -0400, Chris Metcalf wrote:
> On 08/12/2015 12:00 PM, Frederic Weisbecker wrote:
> >>+#ifdef CONFIG_CPU_ISOLATED
> >>+void cpu_isolated_wait(void)
> >>+{
> >>+	set_current_state(TASK_INTERRUPTIBLE);
> >>+	_cpu_idle();
> >>+	set_current_state(TASK_RUNNING);
> >>+}
> >I'm still uncomfortable with that. A wake up model could work?
> 
> I don't know exactly what you have in mind.  The theory is that
> at this point we're ready to return to user space and we're just
> waiting for a timer tick that is guaranteed to arrive, since there
> is something pending for the timer.

Hmm, ok I'm going to discuss that in the new version. One worry is that
it gets racy and we sleep there for ever.

> 
> And, this is an arch-specific method anyway; the generic method
> is actually checking to see if a signal has been delivered,
> scheduling is needed, etc., each time around the loop, so if
> you're not sure your architecture will do the right thing, just
> don't provide a method that idles while waiting.  For tilegx I'm
> sure it works correctly, so I'm OK providing that method.

Yes but we do busy waiting on all other archs then. And since we can wait
for a while there, it doesn't look sane.

> >>diff --git a/include/linux/sched.h b/include/linux/sched.h
> >>index 04b5ada460b4..0bb248385d88 100644
> >>--- a/include/linux/sched.h
> >>+++ b/include/linux/sched.h
> >>@@ -1776,6 +1776,9 @@ struct task_struct {
> >>  	unsigned long	task_state_change;
> >>  #endif
> >>  	int pagefault_disabled;
> >>+#ifdef CONFIG_CPU_ISOLATED
> >>+	unsigned int	cpu_isolated_flags;
> >>+#endif
> >Can't we add a new flag to tsk->flags? There seem to be some values remaining.
> 
> Yeah, I thought of that, but it seems like a pretty scarce resource,
> and I wasn't sure it was the right thing to do.  Also, I'm not actually
> sure why the lowest two bits aren't apparently being used

Probably they were used but got removed.

> looks
> like PF_EXITING (0x4) is the first bit used.  And there are only three
> more bits higher up in the word that are not assigned.

Which makes room for 5 :)

> 
> Also, right now we are allowing users to customize the signal delivered
> for STRICT violation, and that signal value is stored in the
> cpu_isolated_flags word as well, so we really don't have room in
> tsk->flags for all of that anyway.

Yeah indeed, ok lets keep it that way for now.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/