linux-kernel - Re: [PATCH 0/3] ramzswap: Eliminate stale data from compressed memory (v2)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4BE504AD.4030807@cs.helsinki.fi>
Date:	Sat, 08 May 2010 09:29:01 +0300
From:	Pekka Enberg <penberg@...helsinki.fi>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	Nitin Gupta <ngupta@...are.org>, Greg KH <greg@...ah.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	Cyp <cyp561@...il.com>, Minchan Kim <minchan.kim@...il.com>,
	Al Viro <viro@...IV.linux.org.uk>,
	Christoph Hellwig <hch@...radead.org>,
	Jens Axboe <jens.axboe@...cle.com>,
	Andi Kleen <andi@...stfloor.org>,
	driverdev <devel@...verdev.osuosl.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/3] ramzswap: Eliminate stale data from compressed memory
 (v2)

Hi Andrew,

Andrew Morton wrote:
> Looking at the changelogs I'm seeing no information about the
> effectiveness of ramzswap - how much memory it saves.  As that's the
> entire point of the driver, that would be a rather important thing to
> have included in the commit comments.  We cannot make the decision to
> merge ramzswap without this info.

There's some benchmarks at ramzswap pages:

http://code.google.com/p/compcache/wiki/Performance

http://code.google.com/p/compcache/wiki/SwapDiskVsRamz

[ snip bunch of comments from Andrew that need to be addressed,
   hopefully we'll get some help from the staging people ]

> The driver appears to be controlled by some nasty-looking ioctl against
> some fd.  None of it is documented anywhere.  It should be.  You're
> proposing here a permanent extension to the kernel ABI which we will
> need to maintain for ever.  That's a big deal and it is the very first
> thing reviewers will look at, before even considering the code.

I thought we got rid of it? Nitin?

> RZSIO_GET_STATS looks to be hopeless from a long-term maintainability
> POV.  It's debug code and it would be better to move it into a debugfs
> file, where we can then add and remove things at will.

Yup.

> I've completely forgotten why we need this xvmalloc thing and I don't
> recall whether we decided it would be a good thing to have as a generic
> facility and of course it's all unexplained and undocumented.  I won't
> be looking at it today, for this reason.

We need it because the slab allocator is not a good fit for this special 
purpose driver due to fragmentation. Nitin, you had a nice web page 
showing all the relevant numbers but I can't find it anymore.

Andrew, FWIW, I'm ok with xvmalloc() for this particular driver. There 
was some discussion on making it more generic but I don't see it as a 
merge-stopper for the driver.

> The overall idea and utility appear to be good and desirable, IMO.  But
> the code isn't productively reviewable in this state.

I agree that the whole graduation step from staging to kernel proper is 
not well-defined. Any suggestions? That said, I hope that doesn't stop 
us from merging this patch series because the lack of notifiers cripples 
the current ramzswap performance.

			Pekka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/