[Xen-devel] RE: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking

[Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>]

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?
I need to get this wrapped up so I can re-purpose a test
machine for another use.

Thanks,
Dan

> > > 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.
 
> 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
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel