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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id DCF61C46CD4 for ; Wed, 27 Dec 2023 06:33:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7C6BF6B007D; Wed, 27 Dec 2023 01:33:01 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 750076B007E; Wed, 27 Dec 2023 01:33:01 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 617E96B0080; Wed, 27 Dec 2023 01:33:01 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 51A826B007D for ; Wed, 27 Dec 2023 01:33:01 -0500 (EST) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 1BC4CC0647 for ; Wed, 27 Dec 2023 06:33:01 +0000 (UTC) X-FDA: 81611630562.30.2692CD3 Received: from mail-pj1-f48.google.com (mail-pj1-f48.google.com [209.85.216.48]) by imf06.hostedemail.com (Postfix) with ESMTP id 71B2818000B for ; Wed, 27 Dec 2023 06:32:58 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=BZ49Xx2U; dmarc=pass (policy=quarantine) header.from=bytedance.com; spf=pass (imf06.hostedemail.com: domain of zhouchengming@bytedance.com designates 209.85.216.48 as permitted sender) smtp.mailfrom=zhouchengming@bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1703658779; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=HaoaHz4Nf0PyKPZr0pvtZiHcszoOMZ2oMpmbbk/upqI=; b=7O4At9DC8Kcoe7DrrePoy9kqtlAZlAI+T6tjUAQhy1eH1ycjBhjtLoWXcfQGg2HqRU/iVl cO7ytEQPf7giZtdyIy3vASWyzreamWdkU7wIWloROCKKxskAxixBFrfgfiMoK1hVNFECVX KPLGGSQJrLTPdGK0W7K5+ysdOXf6wlU= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=BZ49Xx2U; dmarc=pass (policy=quarantine) header.from=bytedance.com; spf=pass (imf06.hostedemail.com: domain of zhouchengming@bytedance.com designates 209.85.216.48 as permitted sender) smtp.mailfrom=zhouchengming@bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1703658779; a=rsa-sha256; cv=none; b=3Nd4JlIqJAHPCCtBeZmZVa1EVnbFqLAy0oXhzjKHUHCVZfRyfZzW19Iw+Cp2ZWrkO+dMFT /ZBOW7S+JAYJ7Z4XVf0RvI7JMxwdrzSx1buUQNLrmpTVgZAva2Iaf3w6KXZ3LVOuitQhsh hJs8aoLOtrvyHVLGriG5SVLhJWWQHF0= Received: by mail-pj1-f48.google.com with SMTP id 98e67ed59e1d1-28c7c422ad9so782227a91.3 for ; Tue, 26 Dec 2023 22:32:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1703658777; x=1704263577; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=HaoaHz4Nf0PyKPZr0pvtZiHcszoOMZ2oMpmbbk/upqI=; b=BZ49Xx2UT0zXJrWAhisTEKV9+Wgb29RBpxflUepo0zVh5K/+8FbtC4k+hQurpPeqDx FJn4nGwbIqzGAPH8Ex9t6o0gBHHstMvIcw1AFe+v4iMSQHEz3d3y+fVMPgApfVOXZ0wX mPXYI0hqUdMEKQgWR9CoW+DeI655Oi9PNObtHioN7gXb6G88HIvjFesK7qyEh5jaOuRv WtvV7ep+Exg3ptWE2tDRd+e3K7QQ3Ol5KNM7S66JPGvoqAJgNd3ZtageC3yo02ey8Lwm B9Bwyd9HNWh+cP/OClfzo7ZPj0FT+QXrxUA8I8rgU9ec/ydHPzeVT0SE9F00aUdvvOqO tfGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703658777; x=1704263577; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=HaoaHz4Nf0PyKPZr0pvtZiHcszoOMZ2oMpmbbk/upqI=; b=BqAwoPi/YusPBsbNYxvOLBS+uFw0dBtt4TUl5uMvPtTPrV1oiTgdgnMSxIkQm2Sr3U YxYuQaByu4ADKTH/kOEA5NByDS0WBfs7r+kZaB/bmoDfR1nHWIXjb6VPGA8szL9qwQ4i yynyx9PQN+2dbvG7sWUuj+dXGUY/q0PpoVPdnDqJsCrLMnr49i2mzRmJjwytPxj6qo9H Z2X/rZY4es1zw60i4jUChpJPKRhUxnbXKc3pSahowU0fGgdHamWh/ylmNdPXguEmKZp2 d5eLeRh9EOmrMEEu7qIu/o4WqzOyX0RpTbRxrmax5YzSJsZV6jQVmObOadHvCeZsNXuR E/YA== X-Gm-Message-State: AOJu0YwaDet6SL2iM8GEwvBG4CMPLJFBrp9fG8LyoaHZC1QVJ+5xHkpY xfLizUf3YQtYcDUesCuzLGbtbT/iCVk2sQ== X-Google-Smtp-Source: AGHT+IFK1ampLyD7FdC079fARwCafUifCQWJRXUESV7OTBwMgUI0JTNQmu6DXaZcYhE1mgTFykBXKQ== X-Received: by 2002:a17:90b:3ecd:b0:285:b78a:dbce with SMTP id rm13-20020a17090b3ecd00b00285b78adbcemr3720709pjb.37.1703658777112; Tue, 26 Dec 2023 22:32:57 -0800 (PST) Received: from [10.254.10.159] ([139.177.225.237]) by smtp.gmail.com with ESMTPSA id sr5-20020a17090b4e8500b0028afd8b1e0bsm11115520pjb.57.2023.12.26.22.32.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 26 Dec 2023 22:32:56 -0800 (PST) Message-ID: Date: Wed, 27 Dec 2023 14:32:50 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 2/6] mm/zswap: reuse dstmem when decompress Content-Language: en-US To: Barry Song <21cnbao@gmail.com> Cc: Andrew Morton , Seth Jennings , Johannes Weiner , Vitaly Wool , Nhat Pham , Chris Li , Yosry Ahmed , Dan Streetman , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Chris Li References: <20231213-zswap-dstmem-v4-0-f228b059dd89@bytedance.com> <20231213-zswap-dstmem-v4-2-f228b059dd89@bytedance.com> From: Chengming Zhou In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 71B2818000B X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: b9fb6rkmj74emg157r3hewc4iae9d7c7 X-HE-Tag: 1703658778-893041 X-HE-Meta: U2FsdGVkX19S6PS6K7Ci9MdP3o0JKUdte1R+O5RCZfeYdwT2RmwVdUXpm6uKxIZ20o1aj/APWsW4C4ItoRpY8KzXiZqdF+fK2/7rM9mZVTiBcAJHZYqfUtoBfCZ/Fe9IcOdXR61JWxBe3cRUgg0SWQdzgrFlbwfZnl2G9PGrEEZI8/LwoKj9GEr6xPs7DTma8i75JsAw3oN7HaN0KkN3fsIHqx/o1GZNJhdwXXJ/znCXWusJtFVusRQYra2nTkhVO1bsEbYAfvoXZl/eRO9HIUjhmKKE1sXo6ylmHE0ZIj3PJRvrAcLE2sXl5RGAYakKo3pq+t3DeYW4//xKPwQT6OtmzrTs38mgnYje7sxPvhpkID4/qtB43aswcADf7psuwJ6lOOaQHP6aNrXeg5TbyTZW6k8wFULzu1fo3k9DxLqKPqNZGKKSvi2uHuvGB2dsE2tBqaLtu75YIIKlO/ZnvqKv5nRgFUYVcWX7CBmBWBBB8iVHeLbDQgGMbZ72TDB1Z7JjxVS1OPNMFAJGQOBNrVqTlae+HV0jNt2hDaDi5B07xusnzhqJEBk8B1Tfz0y5+8XP9VcrRqFxZCoKGEsyw2ntRil3a+iuJ4V8yjvapepNdU8IJytEzsKxevBOMq4/+N6sQyXrD3zAEcj0wFMcQ8ilCm/CDJl0J9f59EeY8VTuiJxaSYD02pbsyc+5pWZ7N0b9lXgLwnMyroeLHqszZFon+jFAdO2Uam7LwDGo0yZe+Eu/vpr5N23w0t8bTWF1fAIHFdnCRAowIsmrTlThfH7VnWaG8GuMg4j8o2I0Wb82K+ogcGKRU33XzP04gP5yPK8EhPKvQKoKO+41lYNdOUVeCPGBoj+zPvYbMVKxi9gh18W8esNhLh+jir4jumn2LhdwYWCWj6SDzSjQcCH6AlGjPfrpZoN5FfAyX38+QuPtNW4Z4fm0ZlRJgI+6r6qxPJKUExvlXL4Mojs6M4G 2Ql2O7VI 9wKqYLZFS+nypszQBejWcgQWwzJOUN8oV1cQbHcu1wPlY3ImDhD9N30DZAlQ3xo2JrTUNkJ4+7HZ+U51bNdAq5YB4WIXa+yaQwrK31Q0Cw7am5A7Nw3TNAGiRDIVi5XuxjBG22+FNhRgWMWDueKdHqMEApg79td7vquHxte/JnE33bhEaoOuhffT0XNUkcx9tKVShmYmHuPp9LWL8Y7BtchH+i84MIYCpfF+0ypfGJhQXy1kPsTCx1gZ7NYlGvynSmzT7mmf3lruW7FpVpsdTi/JcnGf4IQyDvmyF9ISF5CVKfncw+kfXyKhGYhDD669QPd54TizKqoL2H3se5oxftFWRtPbMRTqJ3DwRhsmPf7begoCDyceo1q/5AURrJmKR1+AnmfkGjkzIgcRH7kwM/gEF3Z0hfhufGoB3xVCp7r2RJ782hYlpCpEtrh2YaWsKbifdtnl+GzTz2oL+GdcC44BZiKg4B+GlxVKSsOQIzG2lrI50K08fllU41RrpPqVZ0xIArHHxQuj1V9kSBBYoaR6WBTcMotBmOGQwbXU207JzS1kcZvcTXOMms67R6xlEQLP2 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: List-Subscribe: List-Unsubscribe: On 2023/12/27 09:24, Barry Song wrote: > On Wed, Dec 27, 2023 at 4:56 AM Chengming Zhou > wrote: >> >> In the !zpool_can_sleep_mapped() case such as zsmalloc, we need to first >> copy the entry->handle memory to a temporary memory, which is allocated >> using kmalloc. >> >> Obviously we can reuse the per-compressor dstmem to avoid allocating >> every time, since it's percpu-compressor and protected in percpu mutex. > > what is the benefit of this since we are actually increasing lock contention > by reusing this buffer between multiple compression and decompression > threads? This mutex is already reused in all compress/decompress paths even before the reuse optimization. I think the best way maybe to use separate crypto_acomp for compression and decompression. Do you think the lock contention will be increased because we now put zpool_map_handle() and memcpy() in the lock section? Actually, we can move zpool_map_handle() before the lock section if needed, but that memcpy() should be protected in lock section. > > this mainly affects zsmalloc which can't sleep? do we have performance > data? Right, last time when test I remembered there is very minor performance difference. The main benefit here is to simply the code much and delete one failure case. > > and it seems this patch is also negatively affecting z3fold and zbud.c > which actually don't need to allocate a tmp buffer. > As for z3fold and zbud, the influence should be much less since the only difference here is zpool_map_handle() moved in lock section, which could be moved out if needed as noted above. And also no evident performance regression in the testing. Thanks. >> >> Reviewed-by: Nhat Pham >> Reviewed-by: Yosry Ahmed >> Acked-by: Chris Li (Google) >> Signed-off-by: Chengming Zhou >> --- >> mm/zswap.c | 44 ++++++++++++-------------------------------- >> 1 file changed, 12 insertions(+), 32 deletions(-) >> >> diff --git a/mm/zswap.c b/mm/zswap.c >> index 976f278aa507..6b872744e962 100644 >> --- a/mm/zswap.c >> +++ b/mm/zswap.c >> @@ -1417,19 +1417,13 @@ static int zswap_writeback_entry(struct zswap_entry *entry, >> struct crypto_acomp_ctx *acomp_ctx; >> struct zpool *pool = zswap_find_zpool(entry); >> bool page_was_allocated; >> - u8 *src, *tmp = NULL; >> + u8 *src; >> unsigned int dlen; >> int ret; >> struct writeback_control wbc = { >> .sync_mode = WB_SYNC_NONE, >> }; >> >> - if (!zpool_can_sleep_mapped(pool)) { >> - tmp = kmalloc(PAGE_SIZE, GFP_KERNEL); >> - if (!tmp) >> - return -ENOMEM; >> - } >> - >> /* try to allocate swap cache page */ >> mpol = get_task_policy(current); >> page = __read_swap_cache_async(swpentry, GFP_KERNEL, mpol, >> @@ -1465,15 +1459,15 @@ static int zswap_writeback_entry(struct zswap_entry *entry, >> /* decompress */ >> acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx); >> dlen = PAGE_SIZE; >> + mutex_lock(acomp_ctx->mutex); >> >> src = zpool_map_handle(pool, entry->handle, ZPOOL_MM_RO); >> if (!zpool_can_sleep_mapped(pool)) { >> - memcpy(tmp, src, entry->length); >> - src = tmp; >> + memcpy(acomp_ctx->dstmem, src, entry->length); >> + src = acomp_ctx->dstmem; >> zpool_unmap_handle(pool, entry->handle); >> } >> >> - mutex_lock(acomp_ctx->mutex); >> sg_init_one(&input, src, entry->length); >> sg_init_table(&output, 1); >> sg_set_page(&output, page, PAGE_SIZE, 0); >> @@ -1482,9 +1476,7 @@ static int zswap_writeback_entry(struct zswap_entry *entry, >> dlen = acomp_ctx->req->dlen; >> mutex_unlock(acomp_ctx->mutex); >> >> - if (!zpool_can_sleep_mapped(pool)) >> - kfree(tmp); >> - else >> + if (zpool_can_sleep_mapped(pool)) >> zpool_unmap_handle(pool, entry->handle); >> >> BUG_ON(ret); >> @@ -1508,9 +1500,6 @@ static int zswap_writeback_entry(struct zswap_entry *entry, >> return ret; >> >> fail: >> - if (!zpool_can_sleep_mapped(pool)) >> - kfree(tmp); >> - >> /* >> * If we get here because the page is already in swapcache, a >> * load may be happening concurrently. It is safe and okay to >> @@ -1771,7 +1760,7 @@ bool zswap_load(struct folio *folio) >> struct zswap_entry *entry; >> struct scatterlist input, output; >> struct crypto_acomp_ctx *acomp_ctx; >> - u8 *src, *dst, *tmp; >> + u8 *src, *dst; >> struct zpool *zpool; >> unsigned int dlen; >> bool ret; >> @@ -1796,26 +1785,19 @@ bool zswap_load(struct folio *folio) >> } >> >> zpool = zswap_find_zpool(entry); >> - if (!zpool_can_sleep_mapped(zpool)) { >> - tmp = kmalloc(entry->length, GFP_KERNEL); >> - if (!tmp) { >> - ret = false; >> - goto freeentry; >> - } >> - } >> >> /* decompress */ >> dlen = PAGE_SIZE; >> - src = zpool_map_handle(zpool, entry->handle, ZPOOL_MM_RO); >> + acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx); >> + mutex_lock(acomp_ctx->mutex); >> >> + src = zpool_map_handle(zpool, entry->handle, ZPOOL_MM_RO); >> if (!zpool_can_sleep_mapped(zpool)) { >> - memcpy(tmp, src, entry->length); >> - src = tmp; >> + memcpy(acomp_ctx->dstmem, src, entry->length); >> + src = acomp_ctx->dstmem; >> zpool_unmap_handle(zpool, entry->handle); >> } >> >> - acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx); >> - mutex_lock(acomp_ctx->mutex); >> sg_init_one(&input, src, entry->length); >> sg_init_table(&output, 1); >> sg_set_page(&output, page, PAGE_SIZE, 0); >> @@ -1826,15 +1808,13 @@ bool zswap_load(struct folio *folio) >> >> if (zpool_can_sleep_mapped(zpool)) >> zpool_unmap_handle(zpool, entry->handle); >> - else >> - kfree(tmp); >> >> ret = true; >> stats: >> count_vm_event(ZSWPIN); >> if (entry->objcg) >> count_objcg_event(entry->objcg, ZSWPIN); >> -freeentry: >> + >> spin_lock(&tree->lock); >> if (ret && zswap_exclusive_loads_enabled) { >> zswap_invalidate_entry(tree, entry); >> >> -- >> b4 0.10.1 >>