Hi Ilya,

> And we don't really need a separate n_alloc.  We could track the size
> in n_cores variable, it is not used inside the loop anyway, only
> updated before breaking.

> This is not fully correct.  On sucess errno may not be set to zero, it
> is only meaningful on error.  A better option here is to write the else
> case for the if condition above and update the error and the n_cores in
> both branches (on success set error to zero and n_cores to the actual
> count, on failure - set error to errno and double the n_cores), then
> the CPU_FREE can go to the end of the loop and the loop condition will
> break, i.e. no need to break inside the if.

Thanks for the review.  I prepared v3 following your suggestions, but I
noticed one possible edge case in the simplified loop before sending it.

If n_cores is also used as the probe size and sched_getaffinity() never
succeeds, count_cpu_cores__() may return the enlarged probe size instead
of the fallback CPU count.  This should be unlikely for pid 0 with valid
masks, but if it happens OVS could overestimate the CPU count and create
many more threads than expected.

Do you think it is worth keeping a separate fallback value and restoring
n_cores when sched_getaffinity() does not succeed, or would you prefer to
keep the v3 loop simple?

Thanks,
Wang Zhan
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to