[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] xen/sched: rework credit2 run-queue allocation

On Thu, 2020-02-20 at 07:56 +0100, Jürgen Groß wrote:
> On 19.02.20 19:37, Dario Faggioli wrote:
> > On Wed, 2020-02-19 at 17:52 +0100, Jan Beulich wrote:
> > > 
> > Nevertheless, I'd add a quick comment about that, to make it even
> > more
> > obvious. :-)
> Do we really need that?
> Calling any of the alloc functions with interrupts off will crash the
> system (at least in debug builds).
> I don't think we want to add such comments all over the code.
No, and that is not what I am suggesting we do. :-)

Neither I want to push to hard (or slow the patch down) just for this.

But, yes, I feel that considering how the code looks, in this
particular case, it is not entirely obvious to immediately realize that
that is the actual reason. Even more so, if we consider that it is not
such a common issue in scheduling code, where there is an (as much as
possible) clear split between allocation and init / usage phases (all
the *_alloc_*data() stuff).

Having just a one liner here would, I think, save people reading this
code some brain power, which they'll be able to use for focusing on
more scheduler specific issues.

Something like:

/* In case we need it, allocate a new runq now, before taking the lock. */

Anyway, as said, I don't want to push too hard on this. If you feel
strongly about not having something like that, I won't block the patch.

If I then decide that I really want such a comment, I will submit a
patch myself, and maybe we'll discuss the pros & cons of having it in
that thread. :-)

Thanks and Regards
Dario Faggioli, Ph.D
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

Attachment: signature.asc
Description: This is a digitally signed message part

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.