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,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 9C8A8C433E6 for ; Thu, 21 Jan 2021 09:17:39 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id EF69F238E7 for ; Thu, 21 Jan 2021 09:17:38 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EF69F238E7 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 4D2DE6B0005; Thu, 21 Jan 2021 04:17:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 483036B0007; Thu, 21 Jan 2021 04:17:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 372416B0008; Thu, 21 Jan 2021 04:17:38 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0178.hostedemail.com [216.40.44.178]) by kanga.kvack.org (Postfix) with ESMTP id 18BF06B0005 for ; Thu, 21 Jan 2021 04:17:38 -0500 (EST) Received: from smtpin14.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 8B23F1DFC for ; Thu, 21 Jan 2021 09:17:37 +0000 (UTC) X-FDA: 77729229354.14.son84_420126f27562 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin14.hostedemail.com (Postfix) with ESMTP id 5839418229818 for ; Thu, 21 Jan 2021 09:17:37 +0000 (UTC) X-HE-Tag: son84_420126f27562 X-Filterd-Recvd-Size: 10091 Received: from mail-lj1-f180.google.com (mail-lj1-f180.google.com [209.85.208.180]) by imf09.hostedemail.com (Postfix) with ESMTP for ; Thu, 21 Jan 2021 09:17:36 +0000 (UTC) Received: by mail-lj1-f180.google.com with SMTP id u11so1613782ljo.13 for ; Thu, 21 Jan 2021 01:17:36 -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=kHlMcZFDRS6FIqbV28NDsiBUnJDgf5jlb3ZZ/4ggMDw=; b=IGHuyqgGtT6448aGmYqLKQPDz/M6OEsYYQnnLeXASqxxWiTiPV0wwznODAWwXB6YLn kwnmT7RU+4VPZxaRjzCPSHOTUBTrupsUHR5qxWWAiueEVxrZEQxGQWZcKXBBYQZUD5LO V41R7e//PSbCYIqWm6P9Vx6N057bs9KBK7j88= 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=kHlMcZFDRS6FIqbV28NDsiBUnJDgf5jlb3ZZ/4ggMDw=; b=IEmtVdxL53z0ZwJlWUpMxWkOJjoci734387NUq7ozC+XrVMIMti0Z1H8oxz+y0Jeo6 xOiwBY9FVNlaN/vmB9Kh6PxCsIJkKrNm7C4MhlpQJZo215Hb9K1j9v6DIPJukhUsXOHH d7ikmpHyrD1rSPaw95RJlWwXLJEP4I2MCPxY+6TtB3QZkTWZbLXTqJF9MHs3zFf/CAUq yPnMffqYTE02kcplBln98H+m6U4JiG4tIq7X2Z795fSwQyxLXuWOhbO95dpJM98T2+tK WPG1xYSsbsUi631fZrPTIBHlMOF30eca5Sxy+ELM2UDXuMG35Pgp76vzqUwNvRBiJRiW SFrQ== X-Gm-Message-State: AOAM532v5zQqTwJOqSzK04Nf3VknQJbx6wjWlh8jQ/RX0MTaUf31XlGS YoqyJBaD1UxNdnDjHmm/ad2P0ZZv1V0sfOiNPBt2alrLpWY= X-Google-Smtp-Source: ABdhPJxxNOEHiacCbR6/rMKXPWBwlr1dJkpdMnczNswE4ZoUccErX44HGR5ieORECDSfYjmsRq3aRw82DXbdpc0/gAg= X-Received: by 2002:a2e:b8d1:: with SMTP id s17mr5557558ljp.269.1611220655184; Thu, 21 Jan 2021 01:17:35 -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: <1608894171-54174-2-git-send-email-tiantao6@hisilicon.com> From: Vitaly Wool Date: Thu, 21 Jan 2021 10:17:24 +0100 Message-ID: Subject: Re: [RFC mm/zswap 1/2] mm/zswap: add the flag can_sleep_mapped To: Tian Tao Cc: Seth Jennings , Dan Streetman , Andrew Morton , Barry Song , Linux-MM 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 Fri, Dec 25, 2020 at 12:02 PM 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. > * @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); This has escaped my attention, but this is obviously a bug. 'entry' is not initialized at this point. You either have to move memory allocation further down when entry->length starts making sense, or allocate PAGE_SIZE. Best regards, Vitaly > + 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 >