Re: [patch 6/7] cpusets: per cpuset dirty ratios

On Thu, 30 Oct 2008, Peter Zijlstra wrote:

+/*
+ * Determine the dirty ratios for the currently active cpuset
+ */
+void cpuset_get_current_dirty_ratios(int *background, int *throttle)
+{
+ mutex_lock(&callback_mutex);
+ task_lock(current);
+ *background = task_cs(current)->dirty_background_ratio;
+ *throttle = task_cs(current)->cpuset_dirty_ratio;
+ task_unlock(current);
+ mutex_unlock(&callback_mutex);
+
+ if (*background == -1)
+ *background = dirty_background_ratio;
+ if (*throttle == -1)
+ *throttle = vm_dirty_ratio;
+}

That's rather an awful lot of locking to read just two integers.


As far as I know, task_lock(current) is required to dereference
task_cs(current) and callback_mutex is required to ensure its the same
cpuset.

Since we read these things for every evaluation, getting it wrong isn't
too harmful.

So I would suggest just enough locking to ensure we don't reference any
NULL pointers and such.

IIRC the cpuset stuff is RCU freed, so some racy read should be
possible, no?


Ah, that sounds reasonable. We'll no longer require callback_mutex if we
accept races when current attaches to another cpuset here. We'll need
rcu_read_lock() to safely dereference task_cs(current) unless it's
top_cpuset, but that's much better than callback_mutex and spinning on
task_lock(current).

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/

Relevant Pages