On Mon, 2008-08-25 at 09:34 -0700, mark gross wrote: > On Fri, Aug 15, 2008 at 12:51:11AM +0200, John Kacur wrote: > > On Thu, Aug 14, 2008 at 7:48 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > > > On Thu, 2008-08-14 at 08:52 -0700, mark gross wrote: > > > > > >> Keeping a lock around the different "target_value"s may not be so > > >> important. Its just a 32bit scaler value, and perhaps we can make it an > > >> atomic type? That way we loose the raw_spinlock. > > > > > > My suggestion was to keep the locking for the write side - so as to > > > avoid stuff stomping on one another, but drop the read side as: > > > > > > spin_lock > > > foo = var; > > > spin_unlock > > > return foo; > > > > > > is kinda useless, it doesn't actually serialize against the usage of > > > foo, that is, once it gets used, var might already have acquired a new > > > value. > > > > > > The only thing it would protect is reading var, but since that is a > > > machine sized read, its atomic anyway (assuming its naturally aligned). > > > > > > So no need for atomic_t (its read-side is just a read too), just drop > > > the whole lock usage from pq_qos_requirement(). > > > > > > > Thanks Peter. > > > > Mark, is the following patch ok with you? This should be applied to > > mainline, and then after that no special patches are necessary for > > real-time. > > I've been thinking about this patch and I worry that the readability > from making the use of this lock asymmetric WRT reads and writes to the > storage address is bothersome. > > I would rather make the variable an atomic. What do you think about > that? It would make the write side more expensive, as we already have the two atomic operations for the lock and unlock, this would add a third. Then again, I doubt that this is really a fast path. OTOH, a simple comment could clarify the situation for the reader. Up to you I guess ;-) -- To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
