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 02584C25B4F for ; Tue, 7 May 2024 18:29:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 787F46B0099; Tue, 7 May 2024 14:29:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7386E6B009A; Tue, 7 May 2024 14:29:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 600BE6B009C; Tue, 7 May 2024 14:29:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 42AD76B0099 for ; Tue, 7 May 2024 14:29:32 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id E945514022A for ; Tue, 7 May 2024 18:29:31 +0000 (UTC) X-FDA: 82092437742.22.1994125 Received: from mail-ej1-f42.google.com (mail-ej1-f42.google.com [209.85.218.42]) by imf18.hostedemail.com (Postfix) with ESMTP id 0D75E1C0008 for ; Tue, 7 May 2024 18:29:29 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=oCs+rJ+2; spf=pass (imf18.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.42 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1715106570; 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=3KpYXpCglzTufbnH+/rYxT4F7Xx3Oa7GqRK1wLmM3XQ=; b=YNliy6vAEmBfeQcL12QldWQnRPDNEbxUmDTC/oNizSRj5WOaZ8ohhu5fl6JMx82MbNgpZL 5dSG19LtLZ8LiHoDbjMRAFmIPbuSs8WxetWWhxHNgX491ABVHuBuYGHUsvogJjTySz2FuG EZowNrv+y1wjj+PmEWdS3Z3t3O2S4Cw= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=oCs+rJ+2; spf=pass (imf18.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.42 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1715106570; a=rsa-sha256; cv=none; b=bcp7TTgbV5mR4qHft7j6Eiw85UjlJ8Wqovx8R2FySNVxcrHmt4pD/KZmaNqn8OA8IqEgcu HgUnSMErfhroDqFUJ6OP0yoRjq2E7zOgGGSNR3wE6cab5c9aUtdHbI48O/Ag339OekD52v 3fCWvQb3BJ9tGI622N2uqqKeZ1BDTWw= Received: by mail-ej1-f42.google.com with SMTP id a640c23a62f3a-a59a352bbd9so5036366b.1 for ; Tue, 07 May 2024 11:29:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1715106568; x=1715711368; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=3KpYXpCglzTufbnH+/rYxT4F7Xx3Oa7GqRK1wLmM3XQ=; b=oCs+rJ+2UmwX37zpoZyR/P7bAbRJ6PobXVQd5f7gZelEGrefUx2AynzrqWHrD179O0 X+sMVvldok1uI7aq1gLlus5CttLVw8Mt+Qwi0zQytdqRIFlL2T7iUI2yV+cGSCvcpWXm pjb3q+UgfoK5NSlcT+UCvHtoBhSZCB8qWWYXNgrr8HduSxAyyQNK2gx3KVauM4BJ2STJ hmbtp2lIEnzNgO8k6i99nNkiVwTGIC3rJHcwy0X1hth+EL5Np6vg/yj/XQ86VKbmEyOo 8mBc6UYPMeCxcWXYiZG4psNBVfkyfwDUJCHYwApjvMq+8XgLqydTbLC1zXLZNOR3SfBR 00WA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715106568; x=1715711368; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=3KpYXpCglzTufbnH+/rYxT4F7Xx3Oa7GqRK1wLmM3XQ=; b=u06q5SihSHELgzvVYzJTvTxrV0YDMlNd3HSUk832Al6o884VZPNVn/zCB+NuBUNuLt 5HeaYrvSf8xrMTS1heLZJL1O341PKGRS+IoR5KaTDgkDHq+Rlezo1toyeEH9jNzruZ3X zXv9egBUDohhTVU1FY2/sC84CU5qtohB7qlZg1yMwvWQ1Kruv0txaoaWLIckksK621Vc aGjfc5QU9MjvOdaKzNIqTPDkm00dATP6NcPOVO5lE27fzsOTocu/iH5PLsf4SswwfEAO ta1ZgLpS5Xs5z+cB2MyG6Dir1e3U2hJg5WzkkMUWsEmnqCh18w94RjA/MWBfn5aS0Ciw LKXg== X-Forwarded-Encrypted: i=1; AJvYcCXpg1oraz2SdEKw/sbzeGkVDCqGg+hcVzYi16xx+Jwmc+NBYWZ44chcQWNnIpWhf2lV3+D9DaBxI8I7qscgCQpwy98= X-Gm-Message-State: AOJu0Yw5m9ZTvC4wFiR4q/VEBgyJAEA5thQQ2y/YohJujOXEoV+NNl/i OaZSKauh19u1aanGzyLaMpEXfCFuS/2a4SPkaicawM3TTrEfOH/u/jE77phY7SRmw2oWQRUkNjE 4bLRWtcmbGSLzrGr6zMfJ/+5iVBuKRbRHg98Y X-Google-Smtp-Source: AGHT+IE/2+DpWpsBlHzgVAwCuytID1y2RxtmxqTs7MjvvpxserdIU1u/G1eF+XxXoYeoCkuqwtRnDQjAUSpYQGU9Poo= X-Received: by 2002:a17:906:3396:b0:a59:b5a6:65c7 with SMTP id a640c23a62f3a-a59e4cbe31emr281228366b.3.1715106568237; Tue, 07 May 2024 11:29:28 -0700 (PDT) MIME-Version: 1.0 References: <20240507101443.720190-1-usamaarif642@gmail.com> In-Reply-To: <20240507101443.720190-1-usamaarif642@gmail.com> From: Yosry Ahmed Date: Tue, 7 May 2024 11:28:49 -0700 Message-ID: Subject: Re: [PATCH v3] selftests: cgroup: add tests to verify the zswap writeback path To: Usama Arif Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, nphamcs@gmail.com, chengming.zhou@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: wntjbodhjkjgabz87dxf88kqkejfimgk X-Rspam-User: X-Rspamd-Queue-Id: 0D75E1C0008 X-Rspamd-Server: rspam05 X-HE-Tag: 1715106569-208650 X-HE-Meta: U2FsdGVkX19VvMwx1KKLNL2b+VvSgfrpls8aJQGfCsQmzQfCyERq2VZvUCjP7tvkNzqplYkHbZu/lDfs7o8HaSYTzwaKBphV5367wyvSDq9/USFKV8R42ksO8CMP69JYgWKT5a9SWUFQSD/kaIcQ3xlhmYUJUhgs4v/TbGzAU2dmAdnv3iIWxC3qRGnWfr4bx5rRCsNUR1Y4UGpnH+a7eYaXBey8dmBvOx1LIMyxjSKx3MV6RpbRNLTTMCQewxMOuLHqwI52fD9nfeF+Jmt93PL9aUXXtx/giNlEq1TABKthp2UKnvuNBecurCMhykpFtJc+sifEmd5bf1XTj3MyeGl0E8iAnZWGBN7taWzKSZZ1u+yF18w6Q24cLZExOH8YK8eLaeTbagm0e96MzRAyqudU2CaxeigUvIDc7rdaFk9zvH3YAzC9SSm9xInbzlnOONY8YzgBho3Lt95wK8x9nMD7zni5zMkYfANgpKsaoeOAi0QsynpdpxdtAbuF8csrY4UX1+H4WhWivcQjfnYlCG/sP8/cG5yT6vSkPG9cad5702JCaLyivNnWHFOnTBp7XZx0V23g6ypZCtsPf34ndLgCKWxPbhdqzB1abvfkTAbzQzEk32tz/2ts5nk0yu4Ptcbi01MwXOrdL0fhzyTctr34tsHrKca7vKfu/7HantQD7xSVquV01mBCGjVpWCzvUaEdijvln5KOAz3lHCg+sxs7u6WkFTEB98COzdPz8Cb/Jj178NBH5PauizcCm3O+SJowUtjITXeF2EqqO8Lxgbrt84JKmHG9mHPYs8yXdKtu7ebKDQ4EaXwfD/2KuXMHIdAmfXg7loxh/N1ZOUF2sUBGv73ZKQPCtoHCvPGncrfLvLMSmxdwz6QLnP+kwdpMNzWYiX4ly9CmkdSBrVrCZnLYXl0jWctnh5gyWM2Jdhpa4B7nQFyjJ12TEHdZ0LKa/559NnH7pwMPsJbdwEv hzus3Ren 343HjbcO8C6ngqVDHccdKT4EZPgN8oPud9RNVCllZZxQ9RIYRwb1HnTQT6TrLNk9DQd0QF1Le1aQ72w485tvGGZb9tw2g1k/00RxPG7T5Cmbvy4K2sAPXya2ziPuN532GVytbpfBHC/3kqGABWBel32r26HN0MkSZZwn/L99LRCkPakA6+pTXnvrf5oqocsvS3mF/fG55juWA+G5F33CxlVN8pznlylmN5bnvT6m7Z+aMobhDCVk0W8a8bA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.056566, 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 Tue, May 7, 2024 at 3:14=E2=80=AFAM Usama Arif = wrote: > > Initate writeback with the below steps and check using s/Initate/Initiate > memory.stat.zswpwb if zswap writeback occurred: > 1. Allocate memory. > 2. Reclaim memory equal to the amount that was allocated in step 1. > This will move it into zswap. > 3. Save current zswap usage. > 4. Move the memory allocated in step 1 back in from zswap. > 5. Set zswap.max to half the amount that was recorded in step 3. > 6. Attempt to reclaim memory equal to the amount that was allocated, > this will either trigger writeback if its enabled, or reclamation s/its/it's > will fail if writeback is disabled as there isn't enough zswap > space. > > Suggested-by: Nhat Pham > Signed-off-by: Usama Arif > --- > > v2 -> v3: > - Remove memory.max (Yosry Ahmed) > - change from random allocation of memory to increasing and 0 allocation > (Yosry Ahmed) > - stricter error checking when writeback is disabled (Yosry Ahmed) > - Ensure zswpwb_before =3D=3D 0 (Yosry Ahmed) > - Variable definition reorder, function name change (Yosry Ahmed) > > v1 -> v2: > - Change method of causing writeback from limit zswap to memory reclaim. > (Further described in commit message) (Yosry Ahmed) > - Document why using random memory (Nhat Pham) > --- > tools/testing/selftests/cgroup/test_zswap.c | 138 +++++++++++++++++++- > 1 file changed, 137 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/= selftests/cgroup/test_zswap.c > index f0e488ed90d8..805301554fa8 100644 > --- a/tools/testing/selftests/cgroup/test_zswap.c > +++ b/tools/testing/selftests/cgroup/test_zswap.c > @@ -50,7 +50,7 @@ static int get_zswap_stored_pages(size_t *value) > return read_int("/sys/kernel/debug/zswap/stored_pages", value); > } > > -static int get_cg_wb_count(const char *cg) > +static long get_cg_wb_count(const char *cg) > { > return cg_read_key_long(cg, "memory.stat", "zswpwb"); > } > @@ -248,6 +248,140 @@ static int test_zswapin(const char *root) > return ret; > } > > +/* > + * Attempt writeback with the following steps: > + * 1. Allocate memory. > + * 2. Reclaim memory equal to the amount that was allocated in step 1. > + This will move it into zswap. > + * 3. Save current zswap usage. > + * 4. Move the memory allocated in step 1 back in from zswap. > + * 5. Set zswap.max to half the amount that was recorded in step 3. > + * 6. Attempt to reclaim memory equal to the amount that was allocated, > + this will either trigger writeback if its enabled, or reclamation > + will fail if writeback is disabled as there isn't enough zswap spa= ce. > + */ > +static int attempt_writeback(const char *cgroup, void *arg) > +{ > + long pagesize =3D sysconf(_SC_PAGESIZE); > + char *test_group =3D arg; > + size_t memsize =3D MB(4); > + long zswap_usage; > + bool wb_enabled; > + int ret =3D -1; > + char memval; > + char *mem; > + > + wb_enabled =3D cg_read_long(test_group, "memory.zswap.writeback")= ; > + mem =3D (char *)malloc(memsize); > + if (!mem) > + return ret; > + > + /* > + * Fill half of each page with increasing data, and keep other > + * half empty, this will result in data that is still compressibl= e > + * and ends up in zswap, with material zswap usage. > + */ > + for (int i =3D 0; i < memsize; i +=3D pagesize) { > + memval =3D 1; > + for (int j =3D i; j < i + pagesize/2; j++) { > + /* Overflow of memval is ok, will check with same= values later */ > + mem[j] =3D memval++; > + } > + } It may be more readable if you fill a fixed page-sized buffer with the pattern, then use memcpy() to fill the whole region with it. You can also use memcmp() when validating the data below to compare each page against it. > + > + /* Try and reclaim allocated memory */ > + if (cg_write(test_group, "memory.reclaim", "4M")) { Should we use cg_write_numeric() and memsize here instead of the hardcoded value? > + ksft_print_msg("Failed to reclaim all of the requested me= mory\n"); > + goto out; > + } > + > + zswap_usage =3D cg_read_long(test_group, "memory.zswap.current"); > + > + /* zswpin */ > + for (int i =3D 0; i < memsize; i +=3D pagesize) { > + memval =3D 1; > + for (int j =3D i; j < i + pagesize/2; j++) { > + if (mem[j] !=3D memval++) { > + ksft_print_msg("invalid memory\n"); > + goto out; > + } > + } > + } > + > + if (cg_write_numeric(test_group, "memory.zswap.max", zswap_usage/= 2)) > + goto out; > + > + /* > + * If writeback is enabled, trying to reclaim memory now will tri= gger a > + * writeback as zswap.max is half of what was needed when reclaim= ran the first time. > + * If writeback is disabled, memory reclaim will fail as zswap is= limited and > + * it can't writeback to swap. > + */ > + ret =3D cg_write(test_group, "memory.reclaim", "4M"); > + if (!wb_enabled) { > + if (ret =3D=3D -EAGAIN) > + ret =3D 0; > + else > + ret =3D -1; > + } nit: This can be more compact if (!wb_enabled) ret =3D (ret =3D=3D -EAGAIN) ? 0 : -1; > + > +out: > + free(mem); > + return ret; > +} > + > +/* Test to verify the zswap writeback path */ > +static int test_zswap_writeback(const char *root, bool wb) > +{ > + long zswpwb_before, zswpwb_after; > + int ret =3D KSFT_FAIL; > + char *test_group; > + > + test_group =3D cg_name(root, "zswap_writeback_test"); > + if (!test_group) > + goto out; > + if (cg_create(test_group)) > + goto out; > + if (cg_write(test_group, "memory.zswap.writeback", wb ? "1" : "0"= )) > + goto out; > + > + zswpwb_before =3D get_cg_wb_count(test_group); > + if (zswpwb_before !=3D 0) { > + ksft_print_msg("failed to get zswpwb_before\n"); A negative value means that the read failed. A positive value means that writeback occurred outside of our control. Let's use a generic message here instead that zswpwb_before was $value instead of 0. > + goto out; > + } > + > + if (cg_run(test_group, attempt_writeback, (void *) test_group)) > + goto out; > + > + /* Verify that zswap writeback occurred only if writeback was ena= bled */ > + zswpwb_after =3D get_cg_wb_count(test_group); > + if (wb && zswpwb_after <=3D zswpwb_before) { > + ksft_print_msg("zswpwb_after <=3D zswpwb_before\n"); > + goto out; > + } else if (!wb && zswpwb_after) { > + ksft_print_msg("zswpwb_after !=3D zswpwb_before\n"); > + goto out; > + } I think we should split the check for a negative zswpwb_after, which means that the read failed, from the check that the value makes sense based on whether writeback is enabled or not. Also, we do not need two separate checks here for the correctness of zswpwb_after. Perhaps something like: if (zswpwb_after < 0) goto out; if (wb !=3D !!zswpwb_after) { ksft_print_msg("zswpwb_after is %ld while wb is %s", zswpwb_after, wb ? "enabled" : "disabled"); goto out; } > + > + ret =3D KSFT_PASS; > + > +out: > + cg_destroy(test_group); > + free(test_group); > + return ret; > +} > + > +static int test_zswap_writeback_enabled(const char *root) > +{ > + return test_zswap_writeback(root, true); > +} > + > +static int test_zswap_writeback_disabled(const char *root) > +{ > + return test_zswap_writeback(root, false); > +} > + > /* > * When trying to store a memcg page in zswap, if the memcg hits its mem= ory > * limit in zswap, writeback should affect only the zswapped pages of th= at > @@ -425,6 +559,8 @@ struct zswap_test { > T(test_zswap_usage), > T(test_swapin_nozswap), > T(test_zswapin), > + T(test_zswap_writeback_enabled), > + T(test_zswap_writeback_disabled), > T(test_no_kmem_bypass), > T(test_no_invasive_cgroup_shrink), > }; > -- > 2.43.0 > >