Messages in this thread | | | From | Odin Ugedal <> | Date | Wed, 13 Oct 2021 19:45:59 +0100 | Subject | Re: [PATCH] sched/fair: Use rq->lock when checking cfs_rq list presence |
| |
ons. 13. okt. 2021 kl. 15:39 skrev Michal Koutný <mkoutny@suse.com>: > > On Mon, Oct 11, 2021 at 08:12:08PM +0100, Odin Ugedal <odin@uged.al> wrote: > > Yes, that's my working hypothesis but Vincent found a loophole in the > proposed fix under this assumption.
Yes, I started to think about the same thing late yesterday afternoon as well...
Ref. your comment about reverting a7b359fc6a37 ("sched/fair: Correctly insert cfs_rq's to list on unthrottle"), I think that is fine as long as we revert the commit it fixes as well, to avoid a regression of that (but yeah, that regression itself is less bad than your discovery). If we do so, we know that the only time we remove it from the list is when it is fully decayed, creating way less edge cases
In regards to the race, would a simple fix for that be to, in addition to your patch, set cfs_rq->on_list=2 inside that lock under your code change? If we then treat on_list=2 as "not on list, and do not add"? We can then make constants for them. In that case, we would know that the cfs_rq will never again be inserted into the list, even if it has load. Would something like that work properly?
Thanks Odin
| |