linux-kernel - Re: [PATCH 1/2] [mmotm] Add notifiers for various swap events

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 29 Dec 2009 08:11:09 +0530
From:	Nitin Gupta <ngupta@...are.org>
To:	Andi Kleen <andi@...stfloor.org>
CC:	Pekka Enberg <penberg@...helsinki.fi>,
	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] [mmotm] Add notifiers for various swap events

On 12/29/2009 12:53 AM, Andi Kleen wrote:
>>>>> The first question to ask is if compressed swap is worth
>>>>> it. Do you have benchmark numbers showing it to be an improvement?
>>>>> Are there cases where it is slower than uncompressed swap?
>>>>>
>>>> http://code.google.com/p/compcache/wiki/Performance
>>>
>>> That should be included in the changelog of the patches.
>>
>> Which patches? The driver is already in staging and there's a pointer to 
>> the home page in
> 
> How can it be in staging if there are no hooks for it yet?
> 
>>>> ramzswap is an optional module.
>>>
>>> I have some doubts on the wisdom of making swap algorithms modular.
>>> Better compile them in. Then you don't need messy notifiers either.

These notifiers are the only in-kernel changes ramzswap needs. Giving away
modularity just because of this might not be a good idea. Also, we are
not making any swap algorithm modular. This driver simply creates virtual
block devices to be used as swap disks. This approach allows seamless
integration with existing swap code and 0 overhead when not used.

>>
>> What's so messy about them? The whole point of having the notifiers is to 
>> avoid CONFIG_RAMZSWAP in core kernel code...
> 
> They make the code much harder to read and follow. When you try to follow
> the code flow and you find a notifier it's always a complicated operation
> to figure out what code will end up being called.
> 
> Sometimes they are needed, but they have a high cost in maintainability.
> 

For SWAP_SLOT_FREE notification, I can add a comment mentioning that ramzswap is
currently the only user. Other two notifiers, for swapon and swapoff, are fairly
generic and I hope will find some more users soon.
Maybe I'm missing something but simple cscope query for notifier chain name will
instantly reveal all its users :)

If we remove _any_ of these notifiers, we will end up with lot of ramzswap #ifdef'ery
all over the core kernel code, as Pekka mentioned. This includes swapon/swapoff syscalls(!),
 swap.h, swapfile.c. In fact, some of my initial patches (before mainline merge)
did the same thing but they were obviously too ugly/hacky with CONFIG_RAMZSWAP all over
the place due to which I had to temporarily drop swap notify callback support which is
essential for this driver.

Thanks,
Nitin
--
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/