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 2C46AC433E0 for ; Thu, 24 Dec 2020 14:17:12 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A1AEE22287 for ; Thu, 24 Dec 2020 14:17:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A1AEE22287 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 C926F8D007A; Thu, 24 Dec 2020 09:17:10 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C43198D0061; Thu, 24 Dec 2020 09:17:10 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B58778D007A; Thu, 24 Dec 2020 09:17:10 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0122.hostedemail.com [216.40.44.122]) by kanga.kvack.org (Postfix) with ESMTP id A002A8D0061 for ; Thu, 24 Dec 2020 09:17:10 -0500 (EST) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 65B86180AD81D for ; Thu, 24 Dec 2020 14:17:10 +0000 (UTC) X-FDA: 77628377820.23.work04_360d8d327471 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin23.hostedemail.com (Postfix) with ESMTP id 49EAC37626 for ; Thu, 24 Dec 2020 14:17:10 +0000 (UTC) X-HE-Tag: work04_360d8d327471 X-Filterd-Recvd-Size: 9420 Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com [209.85.167.53]) by imf26.hostedemail.com (Postfix) with ESMTP for ; Thu, 24 Dec 2020 14:17:09 +0000 (UTC) Received: by mail-lf1-f53.google.com with SMTP id 23so4972398lfg.10 for ; Thu, 24 Dec 2020 06:17:09 -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=lkC9PM/znManz2/ET6su3fmj0Go0sy1KTcPXFaaXeV4=; b=IfScdCa0jibx419fMMuDLrrUFNQ9MmAT4hRcpLEywxbbdldyFbOvxwW0Jh4PfKBoc4 YDXWo2BFXG+3fLbQovGQXxzIFLysgXED/xujAx47yhxUuCfTwhNo5sHn1Ta/wkIgBJHE b7YZsPujQ5wfMcJy5p4KgieJMK1m6P4VRGMNs= 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=lkC9PM/znManz2/ET6su3fmj0Go0sy1KTcPXFaaXeV4=; b=X+D6yvD2uD/qzm1E2Mq5YZyKZzbbgpWi44OTLi3rs33ZUpsqrihhIYHzy28Z9YX+I4 d8+HUl6RlVRWlw54OyvFTI37yA2kLOU3eaKc7hg8CjRJ5k4+V7xehcOUON4bRKKFp6B+ ePPGcPMu3lEJ86yrOxM1STTBXidjP3UV4PuLRh7Zrlr6vUtTRM+9z5OLcskalXynfj4v K7j4OjuMGa6QvrR5MhvtlYXNr/i9tSuElENQd+EmJRLB/rzheIxew462MjP6t4pzlwyy zCHRcytIgB0zRC+qj0aAxIS/wA3kAJMNdZwt9J89d2P0ZhWas9fSCU5s6lNp0mN0gDrR jNMg== X-Gm-Message-State: AOAM531X4sTEnkOUN3ib69a1elGKrxUHnM1BypENlwSvKPvZ7ehbueF9 68mzn2M7ZEmMgnKIOrGu9TM+do5ptYPJ3H/erb/EXw== X-Google-Smtp-Source: ABdhPJyQ9EuBY3JizIdMUiv7VqdgWpLXyomsaSwIF/GRchiYvRyv1FM33n9Oi2J8CV7U+QuydbtKs0r3W7ec9sl6oCo= X-Received: by 2002:a2e:b8c4:: with SMTP id s4mr11947691ljp.269.1608819427532; Thu, 24 Dec 2020 06:17:07 -0800 (PST) MIME-Version: 1.0 References: <1608812141-20238-1-git-send-email-tiantao6@hisilicon.com> In-Reply-To: <1608812141-20238-1-git-send-email-tiantao6@hisilicon.com> From: Vitaly Wool Date: Thu, 24 Dec 2020 15:16:55 +0100 Message-ID: Subject: Re: [PATCH] mm/zswap: Fix the compatibility of zsmalloc and zswap To: Tian Tao Cc: Dan Streetman , Seth Jennings , Andrew Morton , Minchan Kim , NitinGupta , 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 Thu, Dec 24, 2020 at 1:16 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 | 2 ++ > mm/z3fold.c | 1 + > mm/zbud.c | 1 + > mm/zpool.c | 13 +++++++++++++ > mm/zsmalloc.c | 1 + > mm/zswap.c | 23 +++++++++++++++++++++-- > 6 files changed, 39 insertions(+), 2 deletions(-) > > diff --git a/include/linux/zpool.h b/include/linux/zpool.h > index 51bf430..a354f4fe 100644 > --- a/include/linux/zpool.h > +++ b/include/linux/zpool.h > @@ -82,6 +82,7 @@ u64 zpool_get_total_size(struct zpool *pool); > */ > struct zpool_driver { > char *type; > + bool sleep_mapped; Could you please add this somewhat further down, close to map / unmap callbacks, and add a description of the field above, too? > struct module *owner; > atomic_t refcount; > struct list_head list; > @@ -112,5 +113,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/z3fold.c b/mm/z3fold.c > index dacb0d7..234b46f 100644 > --- a/mm/z3fold.c > +++ b/mm/z3fold.c > @@ -1778,6 +1778,7 @@ static u64 z3fold_zpool_total_size(void *pool) > > static struct zpool_driver z3fold_zpool_driver = { > .type = "z3fold", > + .sleep_mapped = true, > .owner = THIS_MODULE, > .create = z3fold_zpool_create, > .destroy = z3fold_zpool_destroy, > diff --git a/mm/zbud.c b/mm/zbud.c > index c49966e..7ec5f27 100644 > --- a/mm/zbud.c > +++ b/mm/zbud.c > @@ -203,6 +203,7 @@ static u64 zbud_zpool_total_size(void *pool) > > static struct zpool_driver zbud_zpool_driver = { > .type = "zbud", > + .sleep_mapped = true, > .owner = THIS_MODULE, > .create = zbud_zpool_create, > .destroy = zbud_zpool_destroy, > 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/zsmalloc.c b/mm/zsmalloc.c > index 7289f50..a991d3c 100644 > --- a/mm/zsmalloc.c > +++ b/mm/zsmalloc.c > @@ -440,6 +440,7 @@ static u64 zs_zpool_total_size(void *pool) > > static struct zpool_driver zs_zpool_driver = { > .type = "zsmalloc", > + .sleep_mapped = false, You don't have to add this, it will be false if not explicitly specified. > .owner = THIS_MODULE, > .create = zs_zpool_create, > .destroy = zs_zpool_destroy, > diff --git a/mm/zswap.c b/mm/zswap.c > index 182f6ad..51b033a 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1235,7 +1235,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 +1256,27 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, > goto freeentry; > } > > + if (!zpool_can_sleep_mapped(entry->pool->zpool)) { > + > + tmp = kzalloc(entry->length, GFP_KERNEL); Do we really need kzalloc here? the buffer will be written to in a few moments anyway. Also, we allocate a small buffer for a short while, and this is likely to be a performance critical path, so GFP_ATOMIC looks more appropriate. > + if (!tmp) > + return -ENOMEM; You can't just return here, you need to call zswap_entry_put() first. Since this is obviously not the final version of your patch, could you please split it in 2 the next time? I'd prefer setting flags to specific backends in a separate patch. Best regards, Vitaly > + } > + > /* 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 +1286,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: > -- > 2.7.4 >