Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751939AbbFBUre (ORCPT ); Tue, 2 Jun 2015 16:47:34 -0400 Received: from mail-qg0-f44.google.com ([209.85.192.44]:36759 "EHLO mail-qg0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751122AbbFBUrY (ORCPT ); Tue, 2 Jun 2015 16:47:24 -0400 MIME-Version: 1.0 In-Reply-To: <20150602202628.GB14741@cerebellum.local.variantweb.net> References: <1433257917-13090-1-git-send-email-ddstreet@ieee.org> <20150602202628.GB14741@cerebellum.local.variantweb.net> From: Dan Streetman Date: Tue, 2 Jun 2015 16:47:03 -0400 X-Google-Sender-Auth: Toc2sVMaY4M-AndZS7RGHPc_WyA Message-ID: Subject: Re: [PATCH 0/5] zswap: make params runtime changeable To: Seth Jennings Cc: Andrew Morton , Linux-MM , linux-kernel Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4592 Lines: 95 On Tue, Jun 2, 2015 at 4:26 PM, Seth Jennings wrote: > On Tue, Jun 02, 2015 at 11:11:52AM -0400, Dan Streetman wrote: >> This patch series allows setting all zswap params at runtime, instead >> of only being settable at boot-time. >> >> The changes to zswap are rather large, due to the creation of zswap pools, >> which contain both a compressor function as well as a zpool. When either >> the compressor or zpool param is changed at runtime, a new zswap pool is >> created with the new compressor and zpool, and used for all new compressed >> pages. Any old zswap pools that still contain pages are retained only to >> load pages from, and destroyed once they become empty. >> >> One notable change required for this to work is to split the currently >> global kernel param mutex into a global mutex only for built-in params, >> and a per-module mutex for loadable module params. The reason this change >> is required is because zswap's compressor and zpool param handler callback >> functions attempt to load, via crypto_has_comp() and the new zpool_has_pool() >> functions, any required compressor or zpool modules. The problem there is >> that the zswap param callback functions run while the global param mutex is >> locked, but when they attempt to load another module, if the loading module >> has any params set e.g. via /etc/modprobe.d/*.conf, modprobe will also try >> to take the global param mutex, and a deadlock will result, with the mutex >> held by the zswap param callback which is waiting for modprobe, but modprobe >> waiting for the mutex to change the loading module's param. Using a >> per-module mutex for all loadable modules prevents this, since each module >> will take its own mutex and never conflict with another module's param >> changes. > > Nice work Dan :) > > I'm trying to look at this as three different efforts. In order of > increasing difficulty: > - Enabling/disabling zswap at runtime > - Changing the compressor at runtime, which doesn't involve the zpool layer > - Changing the allocator (type) at runtime which does involve the zpool layer. > > In other words, we can store entries that use a different compressor in > the same zpool, but not entries stored in different allocators. > > Enabling zswap at runtime is very straightforward, especially if you > aren't going to attempt to flush out all the pages on a disable; only > prevent new stores. I like that. > > Changing the compressor at runtime is the next easiest one, since you > have to allocate new compressor transforms, but not a new zpool. You > just store which compressor was used on a per-entry basis. > > Changing the allocator (type) is the hardest since it involves a new > zpool, and all the code for managing multiple zpools in zswap. > > This is a lot of change all at once. Maybe we could just do the runtime > enable/disable of zswap and the runtime change of compressors first? I > think those two alone would be a lot less invasive. Then we can look at > runtime change of the allocator as a separate thing. Sure I'll send the enable/disable individually first, with doc updates. I'll send the other patches as well, to consider separately. > > Thanks, > Seth > >> >> >> Dan Streetman (5): >> zpool: add zpool_has_pool() >> module: add per-module params lock >> zswap: runtime enable/disable >> zswap: dynamic pool creation >> zswap: change zpool/compressor at runtime >> >> arch/um/drivers/hostaudio_kern.c | 20 +- >> drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 6 +- >> drivers/net/wireless/libertas_tf/if_usb.c | 6 +- >> drivers/usb/atm/ueagle-atm.c | 4 +- >> drivers/video/fbdev/vt8623fb.c | 4 +- >> include/linux/module.h | 1 + >> include/linux/moduleparam.h | 67 +-- >> include/linux/zpool.h | 2 + >> kernel/module.c | 1 + >> kernel/params.c | 45 +- >> mm/zpool.c | 25 + >> mm/zswap.c | 696 +++++++++++++++++------ >> net/mac80211/rate.c | 4 +- >> 13 files changed, 640 insertions(+), 241 deletions(-) >> >> -- >> 2.1.0 >> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/