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 F22E4C47DA9 for ; Tue, 30 Jan 2024 01:24:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 49AF36B0087; Mon, 29 Jan 2024 20:24:37 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 44AD96B008A; Mon, 29 Jan 2024 20:24:37 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2EB676B0093; Mon, 29 Jan 2024 20:24:37 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 1F1476B0087 for ; Mon, 29 Jan 2024 20:24:37 -0500 (EST) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id DA738406CB for ; Tue, 30 Jan 2024 01:24:36 +0000 (UTC) X-FDA: 81734232552.01.A1D1A13 Received: from mail-yw1-f201.google.com (mail-yw1-f201.google.com [209.85.128.201]) by imf21.hostedemail.com (Postfix) with ESMTP id 278FC1C000D for ; Tue, 30 Jan 2024 01:24:34 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=rojob4vY; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf21.hostedemail.com: domain of 30k-4ZQoKCIc9z329lsxporzzrwp.nzxwty58-xxv6lnv.z2r@flex--yosryahmed.bounces.google.com designates 209.85.128.201 as permitted sender) smtp.mailfrom=30k-4ZQoKCIc9z329lsxporzzrwp.nzxwty58-xxv6lnv.z2r@flex--yosryahmed.bounces.google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706577875; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=kKsXe4d9eEU7eWHhf0dKSX3npO3wX+Yxof1zu2c6dBc=; b=wnoBGKLhrz8DX/2qox5zG4ssBw/TQYNBHGdqXZiAGM3SV8DwJz+4Tendv2uuAfcsglXv5B IGW1VMwUiOeIHsCEdaBvjwOxPNH6VLzG2IStLOf0GCazkHWnBv5/z+Pp/f3LQvqZ+Jg/5x 9Gt3YGjYNrMsQoXNKzt9zG+v1Q8EvzY= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=rojob4vY; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf21.hostedemail.com: domain of 30k-4ZQoKCIc9z329lsxporzzrwp.nzxwty58-xxv6lnv.z2r@flex--yosryahmed.bounces.google.com designates 209.85.128.201 as permitted sender) smtp.mailfrom=30k-4ZQoKCIc9z329lsxporzzrwp.nzxwty58-xxv6lnv.z2r@flex--yosryahmed.bounces.google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706577875; a=rsa-sha256; cv=none; b=oZFLQ/yeytO378i36Ahr7kaPohm1lFkV7XFEPuWHDxG3NQNr3C/E+MVxzSmm7r2ApGEWjI 4qzL67Hdh9+KNaYW3/mJ/OC732Q7bs+vKNONolGDEt+OUSO1C3RCxm9kmh7kV8eHDPKAFn Dk6zehjmYwvO9wh2RHV6GG7y5HnByy4= Received: by mail-yw1-f201.google.com with SMTP id 00721157ae682-602d494b68aso56985657b3.2 for ; Mon, 29 Jan 2024 17:24:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1706577874; x=1707182674; darn=kvack.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=kKsXe4d9eEU7eWHhf0dKSX3npO3wX+Yxof1zu2c6dBc=; b=rojob4vY5L2nheD0onsNq+E9mMSXsOk2jjq6Gsc41LPlhHhMenfNx3Lmt5zkBKoHv5 S0XDU6/u9ILoEhEXryjQyNYvF8eS8/ZNiVh2Woj2eAiLtkz2JhiMtW3Qj/RPI79EOa/n O8Kb3UsWaMeOVQGSzllmDjpi6sB4N3VIIGC6NWRjG7Lro6JSohuDr+dSLGmU1cKFwEgS k9IQG2trmQmpg0srGLfaqTdxH+OCB7PulG5YOEcGVjyuXYJbZMxQT7pNHwy/QPP2NFnH 1nsbrpQBgHpYe/8Cuuhn0owpxvpYsDL2PvcbU/fmckvVoG+h7PqlV25EOWSe2W05u4+2 nuNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706577874; x=1707182674; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=kKsXe4d9eEU7eWHhf0dKSX3npO3wX+Yxof1zu2c6dBc=; b=enOUjOI7O7+jgkUJA978b+eFluewq3yJNHcIENJc2M2cuQgf8s6uLoAiaOelrIP40q KOOOeW20T4uGSTs8BaysGtSjugiLFnovaEKY7kgEDffaOCnhlH/zKIu628wZ3Q/nAYo3 lt40DBwonWyBapfLHOolhDJVNyWxiq0lo0FuGAHwjsKBHG/SiRTUKHng34mphhhu4HsO saXq0QHHB4ZTcFslvX+x37TfLYKUadOcjX6wFMbig3PGBNI9WfznTfDJPmF64P6IgXd1 HnFZodU+xl+JTA/ObRcbmJQGKel6GX/EsNUlAzvkNUEuIqCbSYFq6id2VMRFkLkGpJhG dlNw== X-Gm-Message-State: AOJu0YxYUWOe7ozWvaF1qCAceOWPEeXEoGU+DC5lDa7ywTIqLpuPklsz CFsEv1CzPedE/rTnbfc+JUiH+UtfR3B8FeyuuQxwEq981xRPtZkJTR1HFYAwPRUw2Zldxk6t7Lp yxDRatq2cxOYZrychaw== X-Google-Smtp-Source: AGHT+IGZLdTuoOjw1Nr7qvD+0vkkEzceltDuyoLOLKxYHKXc2jfBWkBY2f8WOz+VnouBav8ifONTd0ShyTcuOUOT X-Received: from yosry.c.googlers.com ([fda3:e722:ac3:cc00:20:ed76:c0a8:29b4]) (user=yosryahmed job=sendgmr) by 2002:a05:690c:10d:b0:5ff:96b6:8ee1 with SMTP id bd13-20020a05690c010d00b005ff96b68ee1mr2302301ywb.7.1706577874343; Mon, 29 Jan 2024 17:24:34 -0800 (PST) Date: Tue, 30 Jan 2024 01:24:32 +0000 In-Reply-To: <20240129224542.162599-4-nphamcs@gmail.com> Mime-Version: 1.0 References: <20240129224542.162599-1-nphamcs@gmail.com> <20240129224542.162599-4-nphamcs@gmail.com> Message-ID: Subject: Re: [PATCH 3/3] selftests: add test for zswapin From: Yosry Ahmed To: Nhat Pham Cc: akpm@linux-foundation.org, shuah@kernel.org, hannes@cmpxchg.org, tj@kernel.org, lizefan.x@bytedance.com, linux-mm@kvack.org, kernel-team@meta.com, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, linux-kselftest@vger.kernel.org Content-Type: text/plain; charset="us-ascii" X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 278FC1C000D X-Stat-Signature: wnc4hkx9ka3mzfye8qxesm3jexc3gbmt X-HE-Tag: 1706577874-397712 X-HE-Meta: U2FsdGVkX18q/b4M9nLOPe4+Ey6AYVi/Dlb9l4ZVqFTgMpVA4MSB/jC4j/To0eW2VDbGahU8OlNddPeUSix1NIs4oSMBQpV5Jf+pzAi6Jk5YW4StX8+Q80r6Bt3wTdYjdKQyd4//HRjflBy3SpQvrY0gZCQIxWG2t9cMNwqUOOAbklLruqLsfJJlCeJ8CFQxDLH2vTLubH5AXcrNzhZBNAfLhp6Ozm6WV3GGgsQZKHxuOhs1RmOec49b7hBL7HNY+PURjCzBCshOKwmtP3iEAs9h7wAtJIXJqV/CZNOYGjox8CIG7QJPRWU8xXWEjvDSOhPrKwhMwWRx4aI/4V4RB5xXtN5y8Y6rlPRcCpwj40Sz63joiG4WU+rf+GF5kHa9n1tFu2q6Anghv/+xLiqn/DM4YwH1ycAB5f/dlQtcyR13BONRNcRXgI5dFSdPwAlTiNT6bGhvwYgBGUYM5pbpV4JK9WvQX3geihffYiCeyBnTtJOG5SIezq1m/NiUxWxOauefOuzyU2/aLceyGIRU8p4W1QVTNjSZoblwFcI/hjSYRUFFbrDN1oPvxAYv2Sov4G7VzCUZJP649ehn+YkW6IKN+HJY93WrummO04baqUiva408yzCpcFoaMDg8MUD9M97LpjS5+le3T5/CgkkhwxD0vY8SakatLkgnq3TZ+GGQRyxweBEQxD8YtAvuryHrIWAhd9tqopFgIzFSS4vFUJHmJIlnAbXBeldG0SjdhGeW8Qf3lV5ZAhOOcWGzu8sju4TXG36qdPYg8q2btDquDMNgPKYhtEVZJtxxNlOAEX12MFvxo6Va2QSIMDw5dwHISc6XvTVDfo1x6ZQoek7avt3mfIa57Y56Tqlk+ME8UEfl2AOl28eXQGQvanUR7T2AE/WXfsuRvtz735bzvMmHBzhOdjClI3GSwmJ4xVFtsiObpM7H9FQOJVxhkr40Ux2Kpd3dmBfZ6AeugPQg47K J+v8afcQ nekehSCac06IGqCU4jl1dh0gHC/7R2jDAfZSp4DaRuudwJ6qaLfCKQDJs1JZVBBCQMvbcpzQgrL4O1fluQf3F/9/mk3X9AB+pCGERdYQZ780wzkiSSJuGxfdIqfZkRjAxefJUrgb2iRWA0StJpLaOAdlFcx/BxY1OlpkUbTL46v/Vm6ABlwb639DJoaBpzf9nzlAI6azvCsJC9hm60+HHJz7yXadW6lAD+dO+SzyRrOXqOw5wt+Pv0QqZj89OLZtdQx9JocY+hrIb+QNAXptNefP3LbbAhUMLW8MvMIAPu77DX3tiNT0qIY6sbsvlTQQAJKBHr5z7WT4kR0RWvoF59IUbrfDtNXWs8KAz147qwqK6+AssNo908gGytIL3psBItIJv3IduI0N6iXidfY2OsnFV1qeL9nDkRaXzAkt3vQTMOfTR16R+Br0uH4HCD6aOV6K0GpFcoM+ETg0C590UGcA7K9qyKHx6nrvlvsm8OD0UZ/H6mOFjW17iDfLC2cVDPaTMsdBcVHEkoY7cEgdOpM46PfnwgRjsMyu2GqPvYh0UoLW9WzdsQILKqDYsvetMhH8QikS74JGCrSFCJWW0N9B8CLTNWdG31D7fnrjRiKBIEk4FunDLP57RyhuEq9VA6oGIbdK22xVc89ifiqOVfCxW/ARMmcaI5GkY46yW4iscxlqBffsM7/orXgzJ0PPM3FNVQKrcE2UnBe8= X-Bogosity: Ham, tests=bogofilter, spamicity=0.402515, 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 Mon, Jan 29, 2024 at 02:45:42PM -0800, Nhat Pham wrote: > We recently encountered a kernel crash on the zswapin path in our > internal kernel, which went undetected because of a lack of test > coverage for this path. Add a selftest to cover this code path, > allocating more memories than the cgroup limit to trigger s/memories/memory > swapout/zswapout, then reading the pages back in memories several times. > > Also add a variant of this test that runs with zswap disabled, to verify > swapin correctness as well. > > Suggested-by: Rik van Riel > Signed-off-by: Nhat Pham > --- > tools/testing/selftests/cgroup/test_zswap.c | 67 ++++++++++++++++++++- > 1 file changed, 65 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c > index 32ce975b21d1..86231c86dc89 100644 > --- a/tools/testing/selftests/cgroup/test_zswap.c > +++ b/tools/testing/selftests/cgroup/test_zswap.c > @@ -60,17 +60,39 @@ static long get_zswpout(const char *cgroup) > return cg_read_key_long(cgroup, "memory.stat", "zswpout "); > } > > -static int allocate_bytes(const char *cgroup, void *arg) > +static int allocate_bytes_and_read(const char *cgroup, void *arg, bool read) > { > size_t size = (size_t)arg; > char *mem = (char *)malloc(size); > + int ret = 0; > > if (!mem) > return -1; > for (int i = 0; i < size; i += 4095) > mem[i] = 'a'; > + > + if (read) { > + /* cycle through the allocated memory to (z)swap in and out pages */ > + for (int t = 0; t < 5; t++) { What benefit does the iteration serve here? I would guess one iteration is enough to swap everything in at least once, no? > + for (int i = 0; i < size; i += 4095) { > + if (mem[i] != 'a') > + ret = -1; > + } > + } > + } > + > free(mem); > - return 0; > + return ret; > +} > + > +static int allocate_bytes(const char *cgroup, void *arg) > +{ > + return allocate_bytes_and_read(cgroup, arg, false); > +} > + > +static int read_bytes(const char *cgroup, void *arg) > +{ > + return allocate_bytes_and_read(cgroup, arg, true); > } I don't like how we reuse allocate_bytes_and_read(), we are not saving much. Let's keep allocate_bytes() as-is and add a separate helper. Also, I think allocate_and_read_bytes() is easier to read. > > static char *setup_test_group_1M(const char *root, const char *name) > @@ -133,6 +155,45 @@ static int test_zswap_usage(const char *root) > return ret; > } > > +/* Simple test to verify the (z)swapin code paths */ > +static int test_zswapin_size(const char *root, char *zswap_size) > +{ > + int ret = KSFT_FAIL; > + char *test_group; > + > + /* Set up */ > + test_group = cg_name(root, "zswapin_test"); > + if (!test_group) > + goto out; > + if (cg_create(test_group)) > + goto out; > + if (cg_write(test_group, "memory.max", "8M")) > + goto out; > + if (cg_write(test_group, "memory.zswap.max", zswap_size)) > + goto out; > + > + /* Allocate and read more than memory.max to trigger (z)swap in */ > + if (cg_run(test_group, read_bytes, (void *)MB(32))) > + goto out; > + > + ret = KSFT_PASS; > + > +out: > + cg_destroy(test_group); > + free(test_group); > + return ret; > +} > + > +static int test_swapin(const char *root) > +{ > + return test_zswapin_size(root, "0"); > +} Why are we testing the no zswap case? I am all for testing but it seems out of scope here. It would have been understandable if we are testing memory.zswap.max itself, but we are not doing that. FWIW, I think the tests here should really be separated from cgroup tests, but I understand why they were added here. There is a lot of testing for memcg interface and control for zswap, and a lot of nice helpers present.