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=-13.8 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,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 129BCC433DB for ; Thu, 14 Jan 2021 18:29:04 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5855123A50 for ; Thu, 14 Jan 2021 18:29:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5855123A50 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 93CE88D0105; Thu, 14 Jan 2021 13:29:02 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8ED7C8D0102; Thu, 14 Jan 2021 13:29:02 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7DC888D0105; Thu, 14 Jan 2021 13:29:02 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0021.hostedemail.com [216.40.44.21]) by kanga.kvack.org (Postfix) with ESMTP id 676748D0102 for ; Thu, 14 Jan 2021 13:29:02 -0500 (EST) Received: from smtpin28.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 33A11181AF5C3 for ; Thu, 14 Jan 2021 18:29:02 +0000 (UTC) X-FDA: 77705217324.28.night02_5a0a1e227528 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin28.hostedemail.com (Postfix) with ESMTP id 0284A6D63 for ; Thu, 14 Jan 2021 18:29:01 +0000 (UTC) X-HE-Tag: night02_5a0a1e227528 X-Filterd-Recvd-Size: 10139 Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) by imf31.hostedemail.com (Postfix) with ESMTP for ; Thu, 14 Jan 2021 18:29:00 +0000 (UTC) Received: by mail-pl1-f174.google.com with SMTP id j1so3349702pld.3 for ; Thu, 14 Jan 2021 10:29:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=fFadLe4CzlpY6/pzgi2dmBqO8dfEAmKBBIr5LOVUlWk=; b=Dy7awfpBnpn56bznvbdovInmTZgLvnR5v+Qlh1f/8yYQT0UXbpVqEp9I1+FBL7eeJh 7+mTa99hAIOmmq3Hgt2+HQkRybr0Vkj0jEK+LXmM0yIFS1/og6Aj11RAxJjTdLQaJmGF k3dnOeM0VYgqSQKmKE/1ssxvpp7MCp7NK+eOwcEKYNdEYG1oyFljY/sK0FmgESw/6WiX YTYnAkkdAidSp1lGCOM96ZZm+wd35LIZMly4QoDek4AoM1HUhcBe9PSzJB/4u3uOrwvW mknu5lF5cWy/uA8TvwrB2lC1tTje7X9S1pI9opdoQ+9b2Hus/rB2250G+byNORw/JXZr RXFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to; bh=fFadLe4CzlpY6/pzgi2dmBqO8dfEAmKBBIr5LOVUlWk=; b=MpDKd4gReZD6vQT/t+YindejXnpw0EkFY7N81qFCpQINbKhjkCYcGu+CVkTaOlC99P TfS3R4LOeArO+GcCWxr8bdmUrILBpMEp6z5NIyfiHoGRGOOUGYxmYVR9DRgF8jK3vAwz LmrIxq8Dg9ROGp98R8MLbwp/y64El4zmmHjHH6DPK3lsblAH7sYznHHi1x/ZSzcU814w cHYLTml5tlTvqkvrwcWGPOrG3N13x1Hh3i/xqllwJMuGG/RHKjkI2TMTO3fqh/He4nNn 73upcQBesw7aMOzPTaAXWPBuNEdrb//t4/pNb0GOaIKN98GFbqJ0pZXhpYpH3y5FLcAw Dqvg== X-Gm-Message-State: AOAM531zXJhMZAE6Ixw/O/xCNn4es+/FRPWNbIeqztOI1xxlCWMsoHa5 B+Qny6us55d9F5v6U22gagQ= X-Google-Smtp-Source: ABdhPJwHn1qLcqdKts3FbnOIOKtQTQtYFy0BZ+kWMex3hOJTLJvbdWEukYzEKzLzPWSPYmkhlc5wjA== X-Received: by 2002:a17:90a:4096:: with SMTP id l22mr6081816pjg.114.1610648939760; Thu, 14 Jan 2021 10:28:59 -0800 (PST) Received: from google.com ([2620:15c:211:201:7220:84ff:fe09:5e58]) by smtp.gmail.com with ESMTPSA id k25sm5689007pfi.10.2021.01.14.10.28.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Jan 2021 10:28:58 -0800 (PST) Date: Thu, 14 Jan 2021 10:28:56 -0800 From: Minchan Kim To: Tian Tao Cc: sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com, akpm@linux-foundation.org, song.bao.hua@hisilicon.com, linux-mm@kvack.org, shakeelb@google.com, sergey.senozhatsky.work@gmail.com Subject: Re: [RFC mm/zswap 1/2] mm/zswap: add the flag can_sleep_mapped Message-ID: References: <1608894171-54174-1-git-send-email-tiantao6@hisilicon.com> <1608894171-54174-2-git-send-email-tiantao6@hisilicon.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1608894171-54174-2-git-send-email-tiantao6@hisilicon.com> 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 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. > * @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 > >