linux-kernel - RE: Subject: [PATCH V6 1/4] mm: frontswap: swap data structure changes

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:	Tue, 9 Aug 2011 10:43:16 -0700 (PDT)
From:	Dan Magenheimer <dan.magenheimer@...cle.com>
To:	Jan Beulich <JBeulich@...ell.com>
Cc:	hannes@...xchg.org, jackdachef@...il.com, hughd@...gle.com,
	jeremy@...p.org, npiggin@...nel.dk, linux-mm@...ck.org,
	akpm@...ux-foundation.org, sjenning@...ux.vnet.ibm.com,
	Chris Mason <chris.mason@...cle.com>,
	Konrad Wilk <konrad.wilk@...cle.com>,
	Kurt Hackel <kurt.hackel@...cle.com>, riel@...hat.com,
	ngupta@...are.org, linux-kernel@...r.kernel.org, matthew@....cx
Subject: RE: Subject: [PATCH V6 1/4] mm: frontswap: swap data structure
 changes

> From: Jan Beulich [mailto:JBeulich@...ell.com]
> Subject: RE: Subject: [PATCH V6 1/4] mm: frontswap: swap data structure changes
> 
> >>> On 09.08.11 at 17:03, Dan Magenheimer <dan.magenheimer@...cle.com> wrote:
> >> > --- linux/include/linux/swap.h	2011-08-08 08:19:25.880690134 -0600
> >> > +++ frontswap/include/linux/swap.h	2011-08-08 08:59:03.952691415 -0600
> >> > @@ -194,6 +194,8 @@ struct swap_info_struct {
> >> >  	struct block_device *bdev;	/* swap device or bdev of swap file */
> >> >  	struct file *swap_file;		/* seldom referenced */
> >> >  	unsigned int old_block_size;	/* seldom referenced */
> >>
> >> #ifdef CONFIG_FRONTSWAP
> >>
> >> > +	unsigned long *frontswap_map;	/* frontswap in-use, one bit per page */
> >> > +	unsigned int frontswap_pages;	/* frontswap pages in-use counter */
> >>
> >> #endif
> >>
> >> (to eliminate any overhead with that config option unset)
> >>
> >> Jan
> >
> > Hi Jan --
> >
> > Thanks for the review!
> >
> > As noted in the commit comment, if these structure elements are
> > not put inside an #ifdef CONFIG_FRONTSWAP, it becomes
> > unnecessary to clutter the core swap code with several ifdefs.
> > The cost is one pointer and one unsigned int per allocated
> > swap device (often no more than one swap device per system),
> > so the code clarity seemed more important than the tiny
> > additional runtime space cost.
> >
> > Do you disagree?
> 
> Not necessarily - I just know that in other similar occasions (partly
> internally to our company) I was asked to make sure turned off
> features would not leave *any* run time foot print whatsoever.
> 
> Jan

Well I tried adding the ifdef to the structure as you suggested
and it requires three instances of "#ifdef CONFIG_FRONTSWAP"
in mm/swapfile.c.  BUT unless I get into massive code duplication
it still leaves a runtime footprint as extra parameters are passed
to enable_swap_info(), try_to_unuse(), and find_next_to_unuse();
so the intent to achieve zero runtime footprint is illusory.
I expect "absolutely zero runtime footprint" is a goal that very
very few significant new features achieve.

That said, frontswap and cleancache are designed so that they
SHOULD be config=y by default for most distros.  Unless a
module (e.g. zcache or tmem) registers the callbacks, the
overhead is very nearly zero... but if they are config=n,
then a module cannot use them at all.  This would be unfortunate
because the potential performance gain is not insignificant.
I would have preferred them to be merged with default of config=y,
but Linus disabused me of that notion :-}

Anyway, unless you feel very strongly about this, I'm
inclined to not add the ifdef to the struct for the
reasons previously stated.

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