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 E204FC3DA4A for ; Mon, 19 Aug 2024 19:19:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 565246B0083; Mon, 19 Aug 2024 15:19:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4ED9B6B0085; Mon, 19 Aug 2024 15:19:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 38E2D6B0088; Mon, 19 Aug 2024 15:19:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 1716A6B0083 for ; Mon, 19 Aug 2024 15:19:49 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 8A09A4147E for ; Mon, 19 Aug 2024 19:19:48 +0000 (UTC) X-FDA: 82469959656.30.C369211 Received: from mail-ed1-f41.google.com (mail-ed1-f41.google.com [209.85.208.41]) by imf21.hostedemail.com (Postfix) with ESMTP id B86251C0014 for ; Mon, 19 Aug 2024 19:19:46 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="J/NFj7Ax"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf21.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.41 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724095171; a=rsa-sha256; cv=none; b=XUaD6RijzPhftC6EesuSF+voc3arJGrntV6bIKkND4qfAEskZo2IuVR2g5MDQ+F+HcFP8J RaqX/nqw+sVmMVsPG+AIdbhJcil6I1dKMRIvYY4NMO9yHt7Nio8AFlhJV8u+AdsHVMUL/L JrlDn7HvkAMY97DrNM0pTx5TV3Z1z7o= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="J/NFj7Ax"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf21.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.41 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724095171; 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=YZM5zMJ0sB4lMiTqOGEZX6UeVloMpJCvx8UgNki8n98=; b=ShWAJ6rN752sCZ5ZwUaCz60MJvZyayDQgNDmwiMQ8Pz7Zf5KGrzsU/D9DAshcweTttHgvu 2hTfpjes6SPtGGm4qsoyE2oWnyax+g4TdkqiOreG40nfglcy8J6h5+J9Ae76TDGm06yCGV 5QBvKcccEftJSWdjuu7zk2BIxAwhJCY= Received: by mail-ed1-f41.google.com with SMTP id 4fb4d7f45d1cf-5a10835487fso7177283a12.1 for ; Mon, 19 Aug 2024 12:19:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1724095185; x=1724699985; 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=YZM5zMJ0sB4lMiTqOGEZX6UeVloMpJCvx8UgNki8n98=; b=J/NFj7AxjKg7+Wv5PVX8C166DyqdpcZW7+U5TG96cublLMIBvwayaWSfrdO0clf40F pUV0zCvbj7XnV5Ytr3AJOpIztwbvBkwotklq34LmCFD6sFDlctrKPbRyzrXtMvmX37R0 SHzIIUG9EEgS06I75QV1/BYO+wMhdjbpUjP/hdr4N7BPmF0Y2ogg1+5lMQmkihGCsuDo 5VVfgK5dfwVHDq9O9HWDV9ylvrRzKmOx7jTBIvjRNxB3ZMrXLqVQu1wdD/6JARZGtH7E PQofzD4NGK7rq2Hl2qN6SJ4GQZW1yWtMQiyrKGuqmGi3QTIaTvvRRdc1lErUnmx5UDLi tuoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724095185; x=1724699985; 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=YZM5zMJ0sB4lMiTqOGEZX6UeVloMpJCvx8UgNki8n98=; b=k4o/LEdg6XUJHSh88RX4g8/Toh+1oLWDrbsxvpb5LDp/t7swpN1OcgX7KlAdTQxT6R Nh7F6NdeYmWkmMDTHxIhOkO3QU93SohzNA9Vz1g+ippuvtLo9gZ04Ts01j/2aQaQUaqP SO9VIM4JvL3dfE4eHGExm1BIWSUQdVx5+kwTXXdj98q4MyqkTeCR+Gvwkou9cZdabQlh A/5mMMdHm5zYzlnENtcFDcDewgsIh99DE4IWdXeDuxtuwUZha12iZWIwoC4uWgZyeYTp oy8aw8inWUeKsxg/kvOIPjSKbE45Q3EW9oVvqVeNueUW8DRY0QYWq3DcUw+wpVo1y/Yy Kr2w== X-Forwarded-Encrypted: i=1; AJvYcCV0I3AGoxNGsXaDzCUHaR99Mu6v+RQRW1fj2Ttd/qO7mSDI8SlKbwNoisH4ZK5AbIKgCovk0HXVUIUGjIWVqvCOFGE= X-Gm-Message-State: AOJu0YyK1xu7MZwMFwGQ6YEIuFxV02jF9cpODqsXx9rYD4TvxtZjlcuU 0g4qQjJhszDlVlQW5QRcwNWeRjQmj+tT0FXhyjcGqLzZ++z1an/tSZ4y8JluczND+FP5hJWn4XZ M4e6zECGsSwIYDscysAOY+kKKmf8Zr7iiNi3o X-Google-Smtp-Source: AGHT+IHMItryRi8P70QAVbdIuSGoqw5A/WfIe0nTbfG9cMvEsczl2pfv6OH6aJAnOsvEnmXPd7UYyKKeG2aHjAG0ZG4= X-Received: by 2002:a17:906:d7c4:b0:a79:7f94:8a73 with SMTP id a640c23a62f3a-a839292df2emr843653666b.20.1724095184459; Mon, 19 Aug 2024 12:19:44 -0700 (PDT) MIME-Version: 1.0 References: <20240816144344.18135-1-me@yhndnzj.com> <20240816144344.18135-2-me@yhndnzj.com> In-Reply-To: <20240816144344.18135-2-me@yhndnzj.com> From: Yosry Ahmed Date: Mon, 19 Aug 2024 12:19:08 -0700 Message-ID: Subject: Re: [PATCH v2 2/2] selftests: test_zswap: add test for hierarchical zswap.writeback To: Mike Yuan Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, cgroups@vger.kernel.org, Nhat Pham , Johannes Weiner , Andrew Morton , Muchun Song , Shakeel Butt , Roman Gushchin , Michal Hocko Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Queue-Id: B86251C0014 X-Rspamd-Server: rspam01 X-Stat-Signature: h4czy8yxcgk66kjfy6dgerc3z496ief4 X-HE-Tag: 1724095186-939645 X-HE-Meta: U2FsdGVkX1/gofmswQFHDrv/IwWrC7EGAkg071vVwLvjuVa8GHnM/THscIWrd2CyvTqh1nfLm2avi+7s9CdLsFBXqAn6gdw38vIDJjjKzM7isJGgQvvmJm942MZ+txR/a6nA91ZD8CJlvatddYqvl9GW0fBq5w3dB7D3+Qlav75CF8Bq6lhODc3tdUFfeQk7csRV1CvbgMGXs5b6mB0JVV257D0Lgj3/TaKcva2zFqDPpj/XBtQWBZDoyQ9eSkTSmF2KK9xHiJ/xwNK4mPB0D27M/30CdCfsiae+/26Mfw79HqC9uoOLl8JT9JYXkocbRoldGacltJhD6EeA5w92aeRBsZMUtsq9TcTwYXs8ioqw58IzK4JLLPtaxBJGzulc9srTT97BvFn5S+nvu5owk0dRVx1+X+wSF6Tth+4e0/eg7zq+AmlVBPV5YxZnvaR+LR3HuNVpZEEDYAtGWc05XjwMk8BbihIeI0w586gSV8BK2JxDMxJK6hgU+plaPvxoS6FO5vjuGWa7QB4BJhjWhqc79N91q3OgKSl86VMgwaPteDnqRl3axLNy8B5dZyMDHzRwRvgQrOMf9bk3PDxa7r0CfZJVybhev1VCu1gJH7TqJv8Sw7AbBIBDxtZhRWUeMNEYJDPjYwDpd0cbiTi+08EsRfOWwxklL7QgTeM4M+XuXzWRT3XenZ4npPwv2VJSyGVtVmQ+z5fy35v/SpV/B02X+qDcLFeBeP1I6lz13iiAQm/dSUkkzLtPTwIU+kqXydZ+KHQMOwOBrL5by2MQnF2i7D7EsDWLQxlUnK0B9z3vjNQCSLt5nvTKt1NvV/3Kufgl4NJr8Xhl61BYYujY4wqDdmJManpMPlrohLxDUTNrAjIoeHrYg8gerYT1L1opn41j7+cYpwodiXyos821sAel8rxx2/U4Vw0wrlvoXczoquGzTzSKI8f5CMUrUQVO/4CwV3UxShzmBH1dN2/ icKOxSnx CzEMFcobKkdmiIlBcEfm3mQAJ+rxL6yOaCDb5A3g/1ry8K4INwpFYhLHr7SgqD00pw3TaqcdRcEeXckZom4e3xCdVuGKqm4GQoZZs9Gd9nj7OSkPtRUUoLF+X+BGvWehQQNuR4mc/VsOcH5bbNQtfhi+n4HJO7KVya4GwKP9MwtzQ/RV/3vBsJtPgXEyl4Q9UKa3SriU4T36PyCKowOOM7kRUFi9PgsJPR/IqqcARewdBH+QCaeakDVlmwHQWp5I1nHMvtL7dF186ctUuuFmCAVxBrw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.001695, 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 Fri, Aug 16, 2024 at 7:44=E2=80=AFAM Mike Yuan wrote: > > Ensure that zswap.writeback check goes up the cgroup tree. Too concise :) Perhaps a little bit of description of what you are doing would be helpful. > > Signed-off-by: Mike Yuan > --- > tools/testing/selftests/cgroup/test_zswap.c | 69 ++++++++++++++------- > 1 file changed, 48 insertions(+), 21 deletions(-) > > diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/= selftests/cgroup/test_zswap.c > index 190096017f80..7da6f9dc1066 100644 > --- a/tools/testing/selftests/cgroup/test_zswap.c > +++ b/tools/testing/selftests/cgroup/test_zswap.c > @@ -263,15 +263,13 @@ static int test_zswapin(const char *root) > 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); > char buf[pagesize]; > long zswap_usage; > - bool wb_enabled; > + bool wb_enabled =3D *(bool *) arg; > int ret =3D -1; > char *mem; > > - wb_enabled =3D cg_read_long(test_group, "memory.zswap.writeback")= ; > mem =3D (char *)malloc(memsize); > if (!mem) > return ret; > @@ -288,12 +286,12 @@ static int attempt_writeback(const char *cgroup, vo= id *arg) > memcpy(&mem[i], buf, pagesize); > > /* Try and reclaim allocated memory */ > - if (cg_write_numeric(test_group, "memory.reclaim", memsize)) { > + if (cg_write_numeric(cgroup, "memory.reclaim", memsize)) { > 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"); > + zswap_usage =3D cg_read_long(cgroup, "memory.zswap.current"); > > /* zswpin */ > for (int i =3D 0; i < memsize; i +=3D pagesize) { > @@ -303,7 +301,7 @@ static int attempt_writeback(const char *cgroup, void= *arg) > } > } > > - if (cg_write_numeric(test_group, "memory.zswap.max", zswap_usage/= 2)) > + if (cg_write_numeric(cgroup, "memory.zswap.max", zswap_usage/2)) > goto out; > > /* > @@ -312,7 +310,7 @@ static int attempt_writeback(const char *cgroup, void= *arg) > * If writeback is disabled, memory reclaim will fail as zswap is= limited and > * it can't writeback to swap. > */ > - ret =3D cg_write_numeric(test_group, "memory.reclaim", memsize); > + ret =3D cg_write_numeric(cgroup, "memory.reclaim", memsize); > if (!wb_enabled) > ret =3D (ret =3D=3D -EAGAIN) ? 0 : -1; > > @@ -321,12 +319,38 @@ static int attempt_writeback(const char *cgroup, vo= id *arg) > return ret; > } > > +static int test_zswap_writeback_one(const char *cgroup, bool wb) > +{ > + long zswpwb_before, zswpwb_after; > + > + zswpwb_before =3D get_cg_wb_count(cgroup); > + if (zswpwb_before !=3D 0) { > + ksft_print_msg("zswpwb_before =3D %ld instead of 0\n", zs= wpwb_before); > + return -1; > + } > + > + if (cg_run(cgroup, attempt_writeback, (void *) &wb)) > + return -1; > + > + /* Verify that zswap writeback occurred only if writeback was ena= bled */ > + zswpwb_after =3D get_cg_wb_count(cgroup); > + if (zswpwb_after < 0) > + return -1; > + > + if (wb !=3D !!zswpwb_after) { > + ksft_print_msg("zswpwb_after is %ld while wb is %s", > + zswpwb_after, wb ? "enabled" : "disabled"= ); > + return -1; > + } > + > + return 0; > +} > + > /* 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; > + char *test_group, *test_group_child =3D NULL; > > test_group =3D cg_name(root, "zswap_writeback_test"); > if (!test_group) > @@ -336,29 +360,32 @@ static int test_zswap_writeback(const char *root, b= ool wb) > 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("zswpwb_before =3D %ld instead of 0\n", zs= wpwb_before); > + if (test_zswap_writeback_one(test_group, wb)) > goto out; > - } > > - if (cg_run(test_group, attempt_writeback, (void *) test_group)) > + if (cg_write(test_group, "memory.zswap.max", "max")) > + goto out; Why is this needed? Isn't this the default value? > + if (cg_write(test_group, "cgroup.subtree_control", "+memory")) > goto out; > > - /* Verify that zswap writeback occurred only if writeback was ena= bled */ > - zswpwb_after =3D get_cg_wb_count(test_group); > - if (zswpwb_after < 0) > + test_group_child =3D cg_name(test_group, "zswap_writeback_test_ch= ild"); > + if (!test_group_child) > + goto out; > + if (cg_create(test_group_child)) > + goto out; I'd rather have all the hierarchy setup at the beginning of the test, before the actual test logic. I don't feel strongly about it though. > + if (cg_write(test_group_child, "memory.zswap.writeback", "1")) > goto out; Is the idea here that we always hardcode the child's zswap.writeback to 1, and the parent's zswap.writeback changes from 0 to 1, and we check that the parent's value is what matters? I think we need a comment here. TBH, I expected a separate test that checks different combinations of parent and child values (e.g. also verifies that if the parent is enabled but child is disabled, writeback is disabled). > > - if (wb !=3D !!zswpwb_after) { > - ksft_print_msg("zswpwb_after is %ld while wb is %s", > - zswpwb_after, wb ? "enabled" : "disabled"= ); > + if (test_zswap_writeback_one(test_group_child, wb)) > goto out; > - } > > ret =3D KSFT_PASS; > > out: > + if (test_group_child) { > + cg_destroy(test_group_child); > + free(test_group_child); > + } > cg_destroy(test_group); > free(test_group); > return ret; > -- > 2.46.0 > >