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,URIBL_BLOCKED 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 E7CC7C433DB for ; Thu, 14 Jan 2021 18:56:47 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 66D7223B44 for ; Thu, 14 Jan 2021 18:56:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 66D7223B44 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 A22C48D010A; Thu, 14 Jan 2021 13:56:46 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9B4388D00F0; Thu, 14 Jan 2021 13:56:46 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 89A618D010A; Thu, 14 Jan 2021 13:56:46 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0181.hostedemail.com [216.40.44.181]) by kanga.kvack.org (Postfix) with ESMTP id 6F4FC8D00F0 for ; Thu, 14 Jan 2021 13:56:46 -0500 (EST) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 3353B824556B for ; Thu, 14 Jan 2021 18:56:46 +0000 (UTC) X-FDA: 77705287212.15.book11_60155aa27529 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin15.hostedemail.com (Postfix) with ESMTP id 0C09F1814AC8B for ; Thu, 14 Jan 2021 18:56:46 +0000 (UTC) X-HE-Tag: book11_60155aa27529 X-Filterd-Recvd-Size: 12496 Received: from mail-pg1-f182.google.com (mail-pg1-f182.google.com [209.85.215.182]) by imf04.hostedemail.com (Postfix) with ESMTP for ; Thu, 14 Jan 2021 18:56:45 +0000 (UTC) Received: by mail-pg1-f182.google.com with SMTP id p18so4371808pgm.11 for ; Thu, 14 Jan 2021 10:56:45 -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=L2ove0OBnrt4hkPRRyCEf1xPZYVJHFzbxgvam8Ue6WU=; b=ERonHPKzr9kbz9MgSs5a15Xz6a9S1/y5fPPr8w9fPZr+CNe3/eUck2pMYik1HsTDcm Q3kR2/8VPuMjJ4CTR5r6wagVl5jYspJUL1qjaHzx4wslqxNR/BhcOhe0T7j+zExyWxpW 8Vt3YyDKLjm0QsDh0tiEXk6qE8psYp9ihz5PoLGttwYm7ySuWzVJ/jMhbdFW7jvC/+E3 F2F9y/jsUv73a9v2tUybgYm0mRUdH+4ZPQ3lrJy1FBxfcZx51OxJyMkzjA7tlkaB16Sx UeaKsHvMlazKxPSoiQJe70wSWfJABprl3PkTnisXYwbl0mtXu6ksI0yXNZxVlvbIQAOA y5Uw== 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=L2ove0OBnrt4hkPRRyCEf1xPZYVJHFzbxgvam8Ue6WU=; b=K5PObcsvynzSyN+nfWG3nrBskyYVsGvOLJTe1IZfdeanQzFcPYDmHKDRB/82tpaMBB iSd+wNYYi++xHfq0GKiJT5yujjZBPJCL9Tvf4mDyvoG12qg90S/KqxzkUCMLtj0tthyd 7n+zS0pcgO605eUg8naO51Jnsfh4k3/5V3amcIRwNMS7TlSvI6n8bvk0sQS97fpoWUnA 9kPB6RR32SDfmcznSHhcx9eIJvyVkSgYA1p0CWav2iktSl3HFlug/kjLZjAOncrQ8IFR t7ya3aoDEXQ6V87+dnuBOxevJHcy1tWUNRzMVk45ksMm1EM6RKwYvXAmEzZry/Ql4tWx IVHQ== X-Gm-Message-State: AOAM5312vOMSEaxNAeX5peSiMwkfpYHXgWxc16wsJ2yoLZexOZfjjMTz mUZaIsBYp/Vjhfj5e1da+iw= X-Google-Smtp-Source: ABdhPJzW4sxywgfbG9hYOwp8g2MkHg1HFbybgtGqton6fSyRkOHhuohQa9D3hy6sr+8pUqzFWzOuGA== X-Received: by 2002:a63:74b:: with SMTP id 72mr8904439pgh.4.1610650604393; Thu, 14 Jan 2021 10:56:44 -0800 (PST) Received: from google.com ([2620:15c:211:201:7220:84ff:fe09:5e58]) by smtp.gmail.com with ESMTPSA id z10sm6058791pfr.204.2021.01.14.10.56.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Jan 2021 10:56:43 -0800 (PST) Date: Thu, 14 Jan 2021 10:56:41 -0800 From: Minchan Kim To: Vitaly Wool Cc: Tian Tao , Seth Jennings , Dan Streetman , Andrew Morton , Barry Song , Linux-MM , Shakeel Butt , Sergey Senozhatsky 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: 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 07:40:50PM +0100, Vitaly Wool wrote: > 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? Yub. zswap could abstract that part under #ifdef to keep old behavior. > > 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 > > > > > >