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

From: Dario Faggioli <dfaggioli@suse.com>
To: "Jürgen Groß" <jgross@suse.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>,
	xen-devel@lists.xenproject.org, Jan Beulich <jbeulich@suse.com>
Subject: Re: [Xen-devel] [PATCH] xen/sched: rework credit2 run-queue allocation
Date: Thu, 20 Feb 2020 10:28:48 +0100	[thread overview]
Message-ID: <19decaf1ecf3e3b2aaaa0896a586b178ef65fbe4.camel@suse.com> (raw)
In-Reply-To: <40a41da6-64a0-d332-c544-0eb937204fa6@suse.com>

[-- Attachment #1.1: Type: text/plain, Size: 1864 bytes --]

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
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel