From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F04BBC433DB for ; Thu, 14 Jan 2021 18:41:06 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 43CD723A55 for ; Thu, 14 Jan 2021 18:41:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 43CD723A55 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 56FC18D0102; Thu, 14 Jan 2021 13:41:04 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 51F278D00F0; Thu, 14 Jan 2021 13:41:04 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 435258D0102; Thu, 14 Jan 2021 13:41:04 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0038.hostedemail.com [216.40.44.38]) by kanga.kvack.org (Postfix) with ESMTP id 2FBCE8D00F0 for ; Thu, 14 Jan 2021 13:41:04 -0500 (EST) Received: from smtpin14.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id E9DAF1EE6 for ; Thu, 14 Jan 2021 18:41:03 +0000 (UTC) X-FDA: 77705247606.14.cup40_5f0e37c27528 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin14.hostedemail.com (Postfix) with ESMTP id CC6E418229835 for ; Thu, 14 Jan 2021 18:41:03 +0000 (UTC) X-HE-Tag: cup40_5f0e37c27528 X-Filterd-Recvd-Size: 11231 Received: from mail-lj1-f182.google.com (mail-lj1-f182.google.com [209.85.208.182]) by imf46.hostedemail.com (Postfix) with ESMTP for ; Thu, 14 Jan 2021 18:41:03 +0000 (UTC) Received: by mail-lj1-f182.google.com with SMTP id f17so7576349ljg.12 for ; Thu, 14 Jan 2021 10:41:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=apTGOMRkhEfpZuh1ZsBYP6prhnI6Q27ScFsAuosRZTw=; b=bUVsyZJ1OLI6DPMt8wF+OtMLckRtiPOVjfVc3aQqoZgdW6RmZKjwGhGneTR7Rbe9CL 5OVnt5ex2tlxJ4imgYLmcZ5Bd0q1gofxLlnj7n3seMoFuUM+Wah7whljX8IWkIFFzGaA KeYNpFWRUfRtuVgcVgg01Jw4CxnzdYGGU4g4c= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=apTGOMRkhEfpZuh1ZsBYP6prhnI6Q27ScFsAuosRZTw=; b=iEBExVgyrwcPd8EryA0NY5gKECH3eY0i/L50QKGB1K+TJvPM7r7QHL4r66jT6z7625 sxYcpnS9GhYdZW3RdhCCoiO834RkoduzZnhyZcQVx6KibcwQdH1GMatxfpKsypVdUYwv mxranpmbbhBOMCYMgAKoc+IsaewgL7mEaMm3EPx7dB2onyzQu3nRY1cL3tpfFvUjLASt H9FxXvsY6V3sWW1+UiMd4GSWmzbQgSdfbdqSSMRSmtkJfw9o5uMAWvuCFYeGOPMLTsC+ syEl35JcvBDylZLMunyXGrN+ZBjp+bV5kI5lqpqNNAPOXlh86I5JKoRuUvz9YQL3rys6 2glA== X-Gm-Message-State: AOAM533wSBZfY+S7yA7BfuJABLZPeS1SkfsdPlu94Qds/JF7r/ergIb1 h3BYeFUKuIhS/x23LWm4t6lYxowRFrsWr/iPM+Go9KCVKOcVqg== X-Google-Smtp-Source: ABdhPJyjYwVobiQyZ4IQJEbJsoaK7Eu/D4aIZokGj9dvnSHc4yDgjsQZ4OAXfYUgzzVoE/+0nzT6krlgqYQTx+jc6N0= X-Received: by 2002:a2e:8059:: with SMTP id p25mr3690480ljg.155.1610649661708; Thu, 14 Jan 2021 10:41:01 -0800 (PST) MIME-Version: 1.0 References: <1608894171-54174-1-git-send-email-tiantao6@hisilicon.com> <1608894171-54174-2-git-send-email-tiantao6@hisilicon.com> In-Reply-To: From: Vitaly Wool Date: Thu, 14 Jan 2021 19:40:50 +0100 Message-ID: Subject: Re: [RFC mm/zswap 1/2] mm/zswap: add the flag can_sleep_mapped To: Minchan Kim Cc: Tian Tao , Seth Jennings , Dan Streetman , Andrew Morton , Barry Song , Linux-MM , Shakeel Butt , Sergey Senozhatsky Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Jan 14, 2021 at 7:29 PM Minchan Kim wrote: > > On Fri, Dec 25, 2020 at 07:02:50PM +0800, Tian Tao wrote: > > add a flag to zpool, named is "can_sleep_mapped", and have it set true > > for zbud/z3fold, set false for zsmalloc. Then zswap could go the current > > path if the flag is true; and if it's false, copy data from src to a > > temporary buffer, then unmap the handle, take the mutex, process the > > buffer instead of src to avoid sleeping function called from atomic > > context. > > > > Signed-off-by: Tian Tao > > --- > > include/linux/zpool.h | 3 +++ > > mm/zpool.c | 13 +++++++++++++ > > mm/zswap.c | 50 +++++++++++++++++++++++++++++++++++++++++++++----- > > 3 files changed, 61 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/zpool.h b/include/linux/zpool.h > > index 51bf430..e899701 100644 > > --- a/include/linux/zpool.h > > +++ b/include/linux/zpool.h > > @@ -73,6 +73,7 @@ u64 zpool_get_total_size(struct zpool *pool); > > * @malloc: allocate mem from a pool. > > * @free: free mem from a pool. > > * @shrink: shrink the pool. > > + * @sleep_mapped: whether zpool driver can sleep during map. > > I don't think it's a good idea. It just breaks zpool abstraction > in that it exposes internal implementation to user to avoid issue > zswap recently introduced. It also conflicts zpool_map_handle's > semantic. > > Rather than introducing another break in zpool due to the new > zswap feature recenlty introduced, zswap could introduce > CONFIG_ZSWAP_HW_COMPRESSOR. Once it's configured, zsmalloc could > be disabled. And with disabling CONFIG_ZSWAP_HW_COMPRESSOR, zswap > doesn't need to make any bounce buffer copy so that no existing > zsmalloc user will see performance regression. I believe it won't help that much -- the new compressor API presumes that the caller may sleep during compression and that will be an accident waiting to happen as long as we use it and keep the handle mapped in zsmalloc case. Or maybe I interpreted you wrong and you are suggesting re-introducing calls to the old API under this #ifdef, is that the case? Best regards, Vitaly > > > * @map: map a handle. > > * @unmap: unmap a handle. > > * @total_size: get total size of a pool. > > @@ -100,6 +101,7 @@ struct zpool_driver { > > int (*shrink)(void *pool, unsigned int pages, > > unsigned int *reclaimed); > > > > + bool sleep_mapped; > > void *(*map)(void *pool, unsigned long handle, > > enum zpool_mapmode mm); > > void (*unmap)(void *pool, unsigned long handle); > > @@ -112,5 +114,6 @@ void zpool_register_driver(struct zpool_driver *driver); > > int zpool_unregister_driver(struct zpool_driver *driver); > > > > bool zpool_evictable(struct zpool *pool); > > +bool zpool_can_sleep_mapped(struct zpool *pool); > > > > #endif > > diff --git a/mm/zpool.c b/mm/zpool.c > > index 3744a2d..5ed7120 100644 > > --- a/mm/zpool.c > > +++ b/mm/zpool.c > > @@ -23,6 +23,7 @@ struct zpool { > > void *pool; > > const struct zpool_ops *ops; > > bool evictable; > > + bool can_sleep_mapped; > > > > struct list_head list; > > }; > > @@ -183,6 +184,7 @@ struct zpool *zpool_create_pool(const char *type, const char *name, gfp_t gfp, > > zpool->pool = driver->create(name, gfp, ops, zpool); > > zpool->ops = ops; > > zpool->evictable = driver->shrink && ops && ops->evict; > > + zpool->can_sleep_mapped = driver->sleep_mapped; > > > > if (!zpool->pool) { > > pr_err("couldn't create %s pool\n", type); > > @@ -393,6 +395,17 @@ bool zpool_evictable(struct zpool *zpool) > > return zpool->evictable; > > } > > > > +/** > > + * zpool_can_sleep_mapped - Test if zpool can sleep when do mapped. > > + * @zpool: The zpool to test > > + * > > + * Returns: true if zpool can sleep; false otherwise. > > + */ > > +bool zpool_can_sleep_mapped(struct zpool *zpool) > > +{ > > + return zpool->can_sleep_mapped; > > +} > > + > > MODULE_LICENSE("GPL"); > > MODULE_AUTHOR("Dan Streetman "); > > MODULE_DESCRIPTION("Common API for compressed memory storage"); > > diff --git a/mm/zswap.c b/mm/zswap.c > > index 182f6ad..67d4555 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -935,13 +935,20 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle) > > struct scatterlist input, output; > > struct crypto_acomp_ctx *acomp_ctx; > > > > - u8 *src; > > + u8 *src, *tmp; > > unsigned int dlen; > > int ret; > > struct writeback_control wbc = { > > .sync_mode = WB_SYNC_NONE, > > }; > > > > + if (!zpool_can_sleep_mapped(pool)) { > > + > > + tmp = kmalloc(entry->length, GFP_ATOMIC); > > + if (!tmp) > > + return -ENOMEM; > > + } > > + > > /* extract swpentry from data */ > > zhdr = zpool_map_handle(pool, handle, ZPOOL_MM_RO); > > swpentry = zhdr->swpentry; /* here */ > > @@ -979,6 +986,14 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle) > > dlen = PAGE_SIZE; > > src = (u8 *)zhdr + sizeof(struct zswap_header); > > > > + if (!zpool_can_sleep_mapped(pool)) { > > + > > + memcpy(tmp, src, entry->length); > > + src = tmp; > > + > > + zpool_unmap_handle(pool, handle); > > + } > > + > > mutex_lock(acomp_ctx->mutex); > > sg_init_one(&input, src, entry->length); > > sg_init_table(&output, 1); > > @@ -1033,7 +1048,11 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle) > > spin_unlock(&tree->lock); > > > > end: > > - zpool_unmap_handle(pool, handle); > > + if (zpool_can_sleep_mapped(pool)) > > + zpool_unmap_handle(pool, handle); > > + else > > + kfree(tmp); > > + > > return ret; > > } > > > > @@ -1235,7 +1254,7 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, > > struct zswap_entry *entry; > > struct scatterlist input, output; > > struct crypto_acomp_ctx *acomp_ctx; > > - u8 *src, *dst; > > + u8 *src, *dst, *tmp; > > unsigned int dlen; > > int ret; > > > > @@ -1256,12 +1275,29 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, > > goto freeentry; > > } > > > > + if (!zpool_can_sleep_mapped(entry->pool->zpool)) { > > + > > + tmp = kmalloc(entry->length, GFP_ATOMIC); > > + if (!tmp) { > > + ret = -ENOMEM; > > + goto freeentry; > > + } > > + } > > + > > /* decompress */ > > dlen = PAGE_SIZE; > > src = zpool_map_handle(entry->pool->zpool, entry->handle, ZPOOL_MM_RO); > > if (zpool_evictable(entry->pool->zpool)) > > src += sizeof(struct zswap_header); > > > > + if (!zpool_can_sleep_mapped(entry->pool->zpool)) { > > + > > + memcpy(tmp, src, entry->length); > > + src = tmp; > > + > > + zpool_unmap_handle(entry->pool->zpool, entry->handle); > > + } > > + > > acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx); > > mutex_lock(acomp_ctx->mutex); > > sg_init_one(&input, src, entry->length); > > @@ -1271,7 +1307,11 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, > > ret = crypto_wait_req(crypto_acomp_decompress(acomp_ctx->req), &acomp_ctx->wait); > > mutex_unlock(acomp_ctx->mutex); > > > > - zpool_unmap_handle(entry->pool->zpool, entry->handle); > > + if (zpool_can_sleep_mapped(entry->pool->zpool)) > > + zpool_unmap_handle(entry->pool->zpool, entry->handle); > > + else > > + kfree(tmp); > > + > > BUG_ON(ret); > > > > freeentry: > > @@ -1279,7 +1319,7 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, > > zswap_entry_put(tree, entry); > > spin_unlock(&tree->lock); > > > > - return 0; > > + return ret; > > } > > > > /* frees an entry in zswap */ > > -- > > 2.7.4 > > > >