linux-kernel - Re: [PATCH] Use extents for recording what swap is allocated.

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]

Date:	Wed, 25 Oct 2006 08:15:53 +1000
From:	Nigel Cunningham <ncunningham@...uxmail.org>
To:	Pavel Machek <pavel@....cz>
Cc:	"Rafael J. Wysocki" <rjw@...k.pl>, Andrew Morton <akpm@...l.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Use extents for recording what swap is allocated.

Hi.

On Tue, 2006-10-24 at 23:34 +0200, Pavel Machek wrote:
> Hi!
> 
> > > Switch from bitmaps to using extents to record what swap is allocated;
> > > they make more efficient use of memory, particularly where the allocated
> > > storage is small and the swap space is large.
> > 
> > As I said before, I like the overall idea, but I have a bunch of
> > comments.
> 
> Okay, if Rafael likes it... lets take a look.
> 
> First... what is the _worst case_ overhead? AFAICT extents are very
> good at the best case, but tend to suck for the worst case...?

That's right. In using this, we're relying on the fact that the swap
allocator tries to act sensibly. I've only seen worse case performance
when a user had two swap devices with the same priority (striped), but
that was a bug. :)

> > > +#include <linux/suspend.h>
> > > +#include "extent.h"
> > > +
> > > +/* suspend_get_extent
> > > + *
> > > + * Returns a free extent. May fail, returning NULL instead.
> > > + */
> 
> Your comments are nice, and quite close to linuxdoc... Can we make
> them proper linuxdoc?

Ok.

> > > +/* suspend_put_extent_chain.
> > > + *
> > > + * Frees a whole chain of extents.
> > > + */
> > > +void suspend_put_extent_chain(struct extent_chain *chain)
> > 
> > I'd call it suspend_free_all_extents().
> 
> This is actually important. As it does undocditional free(), it may
> not be called "put".

Ok.

> > > +#ifndef EXTENT_H
> > > +#define EXTENT_H
> > > +
> > > +struct extent {
> > > +	unsigned long minimum, maximum;
> > 
> > Well, I'd use shorter names, but whatever.
> 
> Actually, minimum and 
> 	  maximum look too similar. start/end are really better names.

Ok.

Thanks!

Nigel

-
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/