On Mon, Nov 02, 2020 at 08:53:41AM -0500, Phil Auld wrote: > On Fri, Oct 30, 2020 at 10:16:29PM +0000 David Laight wrote: > > From: Benjamin Segall > > > Sent: 30 October 2020 18:48 > > > > > > Hui Su <[email protected]> writes: > > > > > > > Since 'ab93a4bc955b ("sched/fair: Remove > > > > distribute_running fromCFS bandwidth")',there is > > > > nothing to protect between raw_spin_lock_irqsave/store() > > > > in do_sched_cfs_slack_timer(). > > > > > > > > So remove it. > > > > > > Reviewed-by: Ben Segall <[email protected]> > > > > > > (I might nitpick the subject to be clear that it should be trivial > > > because the lock area is empty, or call them dead or something, but it's > > > not all that important) > > > > I don't know about this case, but a lock+unlock can be used > > to ensure that nothing else holds the lock when acquiring > > the lock requires another lock be held. > > > > So if the normal sequence is: > > lock(table) > > # lookup item > > lock(item) > > unlock(table) > > .... > > unlock(item) > > > > Then it can make sense to do: > > lock(table) > > lock(item) > > unlock(item) > > .... > > unlock(table) > > > > although that ought to deserve a comment. > > > > Nah, this one used to be like this : > > raw_spin_lock_irqsave(&cfs_b->lock, flags); > lsub_positive(&cfs_b->runtime, runtime); > cfs_b->distribute_running = 0; > raw_spin_unlock_irqrestore(&cfs_b->lock, flags); > > It's just a leftover. I agree that if it was there for some other > purpose that it would really need a comment. In this case, it's an > artifact of patch-based development I think. > > > Cheers, > Phil >
Yeah, thanks for your explanation, Phil. It is just a leftover.
