This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning an

To: Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking
From: Daniel Kiper <dkiper@xxxxxxxxxxxx>
Date: Wed, 6 Jul 2011 11:39:10 +0200
Cc: jeremy@xxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxx, Ian Campbell <Ian.Campbell@xxxxxxxxxx>, Konrad Wilk <konrad.wilk@xxxxxxxxxx>, JBeulich@xxxxxxxxxx, Daniel Kiper <dkiper@xxxxxxxxxxxx>
Delivery-date: Wed, 06 Jul 2011 02:40:27 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <bc67eadb-f1fe-46af-ba5c-7d1851a3499d@default>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20110704133135.GA6601@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <bc67eadb-f1fe-46af-ba5c-7d1851a3499d@default>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.3.28i
On Tue, Jul 05, 2011 at 12:38:03PM -0700, Dan Magenheimer wrote:
> Hi Daniel --
> Thanks for taking the time to reply again.  Although you didn't
> say it explicitly, I think you are now OK with V4.  True?

Sorry, I forgot about it. If you change(d) what we agreed it is OK.
However, please look below...

> > > > Please move this condition to linux/frontswap.h and include
> > > > this header file (linux/frontswap.h) unconditionally.
> > >
> > > Sorry, this resolves a chicken-and-egg problem as it is.  If
> > > the frontswap patch is not present, there is no file called
> > > include/linux/frontswap.h.  The ifdef can be removed later
> > > when we are sure the frontswap patch is upstream.
> >
> > Hmmm... I think that in this situation
> > it should be moved to frontswap patch.
> You prefer the egg-before-the-chicken, I prefer the
> chicken-before-the-egg. :-) This approach demonstrates
> Xen's clear use for frontswap, and allows trees both with
> frontswap (linux-next) and without frontswap (linux-3.0)
> to properly build.

Your soultion introduce code into linux-3.0 which could not
be enabled (compiled and used) and could confuse others for
what it is for if it could not be enabled.

I still think that this patch should be splited into two independent
(to some extent) entities (selfballooning and frontswap). Later one/both
of them could be applied to appriopriate tree introducing only code
which could be enabled and used.

> > As I saw it was designed to read memory size from kernel
> > command line and module options (lib/cmdline.c). It is
> > mostly used in that context. Additionally, you are using
> > memparse() for parsing values which are not memory sizes.
> > It could be misleading. That is why I asked you to
> > change that to strict_strtoul() (it is generic).
> I agree that strict_strtoul is the better of two very similar
> ways of doing a very similar thing in the kernel.  Changed.


> > > While I would tend to agree, if checkpatch doesn't like it,
> > > someone is going to complain so I'd rather ensure the 80
> > > character limit is preserved.
> >
> > Line lengths overlimits are marked as warnings. If they are sane
> > then kernel developers do not complain.
> That's not my experience... it seems to be a personal
> preference and some people have an allergic reaction to
> longer-than-80 lines.  So I prefer to err on the side of
> a clean checkpatch.



Xen-devel mailing list