On Thursday, September 08, 2016 11:28:48 AM Srinivas Pandruvada wrote:

On Thu, 2016-09-08 at 20:22 +0200, Peter Zijlstra wrote:

quoted

On Thu, Sep 08, 2016 at 11:09:28AM -0700, Tim Chen wrote:

quoted

On Thu, Sep 08, 2016 at 09:59:55AM +0200, Peter Zijlstra wrote:

quoted

I think there's a race here, if two tasks were to write to the
sysctl
they'd both change the value before getting stuck on the mutex in
enable_sched_itmt().

One way around that is doing something like:


	struct ctl_table t;
	int val = sysctl_sched_itmt_enabled;

	t = *table;
	t.data = &val;

	proc_dointvec_minmax(&t, ...);

	/* and update the sysctl_sched_itmt_enabled value inside the
mutex */
	enable_sched_itmi(val);

Peter,

Since enable_sched_itmt is only used by sched_itmt_update_handler,
I've moved the mutex locking to sched_itmt_update_handler to
eliminate
the race condition in the code path you mentioned.

That is indeed simpler. Thanks!

Do we need to send v3 to include these changes?

Yes, please.

Thanks,
Rafael