On 02.03.20 17:49, Dario Faggioli wrote:
On Mon, 2020-03-02 at 09:58 +0100, Jan Beulich wrote:
The issue here results from one of the downsides of using goto: The
early "goto out" and "goto out_up" in the function very clearly
bypass
any possible initialization of min_rqd, yet the tracing code at the
end
of the function consumes the value. There's even a comment regarding
the
trace record not being accurate in this case.
CID: 1460432
Fixes: 9c84bc004653 ("sched: rework credit2 run-queue allocation")
Signed-off-by: Jan Beulich <[email protected]>
Acked-by: Dario Faggioli <[email protected]>
---
It took me a little to convince myself that
new_cpu = cpumask_cycle(min_rqd->pick_bias,
cpumask_scratch_cpu(cpu));
min_rqd->pick_bias = new_cpu;
are safe, i.e. min_rqd can't be NULL here. I think though that this
could do with making more obvious, at the very least by e.g.
@@ -2360,6 +2360,8 @@
unit->cpu_soft_affinity);
cpumask_and(cpumask_scratch_cpu(cpu),
cpumask_scratch_cpu(cpu),
&min_s_rqd->active);
+
+ BUG_ON(!min_rqd);
}
else if ( min_rqd )
{
possibly accompanied by a comment. Thoughts?
Yes, I think this is a good idea.
Personally, I'd put the BUG_ON() outside of the "if {} else if {} else
{}" block (i.e., just above the cpumask_cycle().
I don't think so.
Otherwise the "else if ( min_rqd )" wouldn't make sense.
I can send a patch for that.
Juergen
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
