xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Dario Faggioli <dario.faggioli@citrix.com>
To: Ian Campbell <ian.campbell@citrix.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>,
	Jan Beulich <JBeulich@suse.com>, Li Yechen <lccycc123@gmail.com>
Subject: Re: [PATCH v1][RFC] xen balloon driver numa support, libxl interface
Date: Wed, 14 Aug 2013 01:32:07 +0200	[thread overview]
Message-ID: <1376436727.19076.41.camel@Abyss> (raw)
In-Reply-To: <1376427410.9273.38.camel@hastur.hellion.org.uk>


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

On mar, 2013-08-13 at 21:56 +0100, Ian Campbell wrote:
> On Mon, 2013-08-12 at 17:51 +0200, Dario Faggioli wrote:
> > On lun, 2013-08-12 at 22:57 +0800, Li Yechen wrote:
> > > The relation between virtual and physical node IDs is belong to
> > > another guy's work, I think we could see her email soon. 
> > > 
> > Well, although that's mostly true, I think it would not have harmed to
> > have some sort of high level description of the overall design, trying
> > to explain what the final goal is, who all the involved actors are, what
> > role they play, etc.
> 
> Yechen,
> 
> If there are, as it sounds, several different patches from different
> people within the group needed to implement this feature then please
> could one of you take responsibility for combining them into a single
> (or at most two, one xen.git and one linux.git) coherent series which
> contains everything such that reviewers can get the whole picture.
> 
Yes, Ian, that is pretty much what is going on. Different people working
on different features that are mostly independent but have some
inter-dependencies.

We will definitely put things in such a way that the complete picture
could be available and evident as soon as possible, however, that is not
possible right now.

The reason why Yechen submitted this series, even if a fundamental
building block of it (virtual NUMA topology for guests) is still
missing, is that we thought that there was enough bits of _his_own_work_
--i.e., NUMA-aware ballooning-- in place already, for starting chasing a
bit of feedback, at least on the design of NUMA-aware ballooning itself.

For instance, he has designed it in such a way that it is the higher
toolstack layers (or the user, via xl) that are responsible for deciding
on what physical NUMA node we want some free memory, is that a good
approach? He is doing that by adding a new xenstore node, is that the
right interface? And so on and so forth...

So, basically, we figured that it was worth to try getting an early
enough answer for this kind of questions, especially considering that he
also had some RFC level code that could well exemplify the design
itself. Of course, as other are rightfully pointed out already, when
submitting the RFCs, he failed at describing the complete design, the
motivations and the intended usage of the code he was posting, and we
are sorry and are already addressing this. :-)

To be really honest, I think this is the  biggest issue with this
patches, much more than the fact that some enabling feature/code for it
is still missing. IOW, even if all the bit and pieces were there, it
would be very hard to review the  code without such an high level
explanation of how it is intended to be used anyway, wouldn't it? And as
I said, we're down to fix that.

I hope I clarified the situation at least a bit... In any case, thanks
for having a look at it, and for your suggestion, that I personally
commit to make happen, as soon as all the code will be there (and as
soon as I come back from my summer vacations which are starting in two
days :-P).

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


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

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

  reply	other threads:[~2013-08-13 23:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-12 13:18 [PATCH v1][RFC] xen balloon driver numa support, libxl interface Yechen Li
2013-08-12 14:00 ` Jan Beulich
2013-08-12 14:18   ` Li Yechen
2013-08-12 14:31     ` Jan Beulich
2013-08-12 14:57       ` Li Yechen
2013-08-12 15:15         ` Li Yechen
2013-08-12 15:29           ` Jan Beulich
2013-08-12 15:51         ` Dario Faggioli
2013-08-13 20:56           ` Ian Campbell
2013-08-13 23:32             ` Dario Faggioli [this message]
2013-08-12 16:16   ` Dario Faggioli
2013-08-12 16:58 ` Dario Faggioli
2013-08-12 17:07 ` Dario Faggioli
2013-08-13 20:53 ` Ian Campbell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1376436727.19076.41.camel@Abyss \
    --to=dario.faggioli@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=ian.campbell@citrix.com \
    --cc=lccycc123@gmail.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).