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 F18AAC83F22 for ; Wed, 16 Jul 2025 11:14:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 936526B009D; Wed, 16 Jul 2025 07:14:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 90DD56B009E; Wed, 16 Jul 2025 07:14:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7FBF56B00A0; Wed, 16 Jul 2025 07:14:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 6F74B6B009D for ; Wed, 16 Jul 2025 07:14:12 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 1687A80185 for ; Wed, 16 Jul 2025 11:14:12 +0000 (UTC) X-FDA: 83669868744.08.039EEF0 Received: from mail-pl1-f171.google.com (mail-pl1-f171.google.com [209.85.214.171]) by imf25.hostedemail.com (Postfix) with ESMTP id 16CE4A0009 for ; Wed, 16 Jul 2025 11:14:09 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=cHNw+z0i; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf25.hostedemail.com: domain of lianux.mm@gmail.com designates 209.85.214.171 as permitted sender) smtp.mailfrom=lianux.mm@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1752664450; a=rsa-sha256; cv=none; b=kcc3XbajRVz81zkgT5iKp3iWgqCymYE8wKIbJ0L7qR05/cwOyCG1GDCQzltmiytzlFQm0t fGWauHlZfs6dYwkH+2xt2gQgAAM+Ef/gJSgS8tegdnoEH3AVWJFdHAqbVYFQ4SxDpSlsrz 5lclclzT10F7eirWNoPtsPQiVDoEm+4= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=cHNw+z0i; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf25.hostedemail.com: domain of lianux.mm@gmail.com designates 209.85.214.171 as permitted sender) smtp.mailfrom=lianux.mm@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1752664450; 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=ILrfa6+Be99Ef+DqDqF3KrI5+9Fqfus/HTsYMl0wNxM=; b=L+6MJlcUFXe1VrID2Cd4iK8DlLv2votFdNezQm7Iw56DW+X1LSn/hKOOHdKXMmslw1RkB7 rs7NOLSXDwqmpzSLvOeVrThuqy0YOZQEhWxwBNpUgdILvZxkVoLtyCbnTRVq/w4YixqEeV KL5NSmuqWsvS5Jb9pWqVsHzj/RYFU+U= Received: by mail-pl1-f171.google.com with SMTP id d9443c01a7336-23aeac7d77aso50600765ad.3 for ; Wed, 16 Jul 2025 04:14:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1752664449; x=1753269249; darn=kvack.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=ILrfa6+Be99Ef+DqDqF3KrI5+9Fqfus/HTsYMl0wNxM=; b=cHNw+z0iiM5Rg205EvJ8PjWP+I2A5e0S0+pcm/x5agbZaIo1DUJ1hl3zoFoxXSkmBM WsYku63RAftKEpLq47ByDMG+OOc9dtM/H+hCb/AgKPJU42OU5yGSVVI4ZtkjpHpPCLth /ZSWwYD3TM32E0PI3pFJV+EZoW9f+J13MDOdCf+ewcbpT1UoYotQ66e2J5aeNQezbD9A 9yFwQqg6UNL8rMLO02vzF//lf/fg7NNnOHZdIBrIWMLNudwKvzoRQOO3TycdTDEHhj7O 1whQuejjhH+EdgEb0CnuC/GYC6IHFdffvy8OMgU8XdT0VdJ8oDMqL9cUKvqk0M00FSfh LZ4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1752664449; x=1753269249; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ILrfa6+Be99Ef+DqDqF3KrI5+9Fqfus/HTsYMl0wNxM=; b=wgn6qq4m7BnqmdRCU9ezH/zxZHXFTOb5euCsCyYcZ9AqQYxsJluRHEC45YFw4w3Emc uJqrDE/jPsDMS19NI//IMuwNS07irir7oKrBad43kTEnqMHYAhaZwZFNrFNI3E6xVEma aU0zFzgmE3OM0zyR8gxge05TWV042AHg2hm8lQIPQ9086ktJtB3UrmMEPFCygcStMO7D ioG02tPc7BP9gRi7k0JYKPYZ0z+zT75d3VJjWEuOp1J0DvS7DGVogz0LSK1LYNG9lvqV hVFxrmRkhbWr49cg4b3p6dZyuRJxKnSicG4lFyaYdYzMOKx7/jgHJaJiOMKuCHmS37xY Gl6A== X-Forwarded-Encrypted: i=1; AJvYcCXNir7o2JCDJTVjfS9pwohp4PLd6vM7jGAvwNu+UDLX4uVOn1b3cNr7THWAigv6dlu6hwhOD1fmsw==@kvack.org X-Gm-Message-State: AOJu0YxTGQdl56knlP+Y/ZlQnvvf5T+PVNQrLa8PZ94WvWbRHoOpquUV FgnLeeB7PoFwnyI+PNf9YmvPBuO1DmQ9bUmFmmUnOtV+Z6w4AQDaVfx9 X-Gm-Gg: ASbGncuvUoStyYMLSYtVectckj2hY3T5/JkwZd11uVMdNkqLneuS4nMCmeG56mGO6yu pg9CJgQAf3Mw23Vgh6WVtlL6MPhHYgqbr5jv4oCRJkWfoVgCKAe+3wDXc6qJY6uklx0rZkEM47e 5WPsj3RQ883Dg7y8n7Y3NL3K3WKpv3yzZISLetf8BanURMSC5H1qk1EjDhgPXMnOzVZSeRSxtqs qWgZcQy0Gs4vzMBAKNMTxk36BIRvHdwOB5bRFP8B/x3ZzUjO9pS5/M4TuHhpTHliurTFkBJi28v 52SbccTEphciwAqA7xluZdPznKV79FUqj/j6iSw57Acbuv9z5KUcju4UduDjltTEuRmGBYlQzlj ijmwoaCEd9ZSNHDQOZ5Oux8WaFEEzlXiB2tX66jtSrGp9O3Gx X-Google-Smtp-Source: AGHT+IEtq2HofrDl50I4yMIv753bCF0oc5ifYugPoHdMZqPXOZSmZKBiJlsK9Rg4pn6POgFEFZklsw== X-Received: by 2002:a17:902:f686:b0:235:ea0d:ae23 with SMTP id d9443c01a7336-23e24f366a9mr31649585ad.6.1752664448591; Wed, 16 Jul 2025 04:14:08 -0700 (PDT) Received: from DESKTOP-GIED850.localdomain ([114.247.113.178]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-31c9f2b49easm1284797a91.43.2025.07.16.04.14.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Jul 2025 04:14:08 -0700 (PDT) From: wang lian To: broonie@kernel.org Cc: Liam.Howlett@oracle.com, akpm@linux-foundation.org, brauner@kernel.org, david@redhat.com, gkwang@linx-info.com, jannh@google.com, lianux.mm@gmail.com, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-mm@kvack.org, lorenzo.stoakes@oracle.com, p1ucky0923@gmail.com, ryncsn@gmail.com, shuah@kernel.org, sj@kernel.org, vbabka@suse.cz, zijing.zhang@proton.me, ziy@nvidia.com Subject: Re: [PATCH v5] selftests/mm: add process_madvise() tests Date: Wed, 16 Jul 2025 19:13:58 +0800 Message-ID: <20250716111400.10156-1-lianux.mm@gmail.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=y Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 16CE4A0009 X-Stat-Signature: ytdwwwf1m6rp6huehdo5jiz8ozc8c6ix X-Rspam-User: X-HE-Tag: 1752664449-606632 X-HE-Meta: U2FsdGVkX18OOhuUw0vtGdGfFrgZfujdBgrr9s1uBxzcvBu59e/MOELlQu8ItxsSiVYImi47Tb79kJ0samHHaecSrGEi5aFAMBwZAHmo6BJklJ8HB3MX/apLMiZp5Sms15x9FDW87cHlpD3iPsUl0gMltBpRvL1ejnNtNhEdSD7rYjAunGORdC0TBZ2RiDrMe338IEEHDZJnf16EBUvJoSz8pC00ksuruIQ0ZfjKJs3yidh2wCyk6rp6AU9mYYklcBc9bDkK9nXz0Rv8B2VXM2us3b1/WsiLUeXiC6OdI/TNP8BTYEBUt2thjXPy3B0CpKHdrTe5GGRwwHJZKwBKuiUPoEjePgYHd43lJnEY9vGHk+HnSnRjCDJyJXe2CCIwnjczwnuHmY85STFQ04xrbUEpr6N7HG+9ipyvkfs4eWnwLShDsAJgeDg2xQz3UjNAY0FkYAefSwhagoI8CEAZfDtDuJ7C0S8HrQCAVluNLqch+SOoA8VaDhppMHpAr36yTlNdwn+rCaplmzN9IuZULiaPOBwVDm2+m71cDxYZWlU3yEIeJRclJwtLA0/xMBeDVfvGXysRpNCXE/ctW1oFpMhN7+L7E8hTYLh4XlSIX2h7t2WV5tL4AsrDThOYKW6MI5+CXip+yE1TBMikApbAiElrR5Mgtl6NZFY7qZwUrYzy/r3aL17rWVyhWgnntavpVrv56gZ3Pe/LVaqYHobeJdQSRf9mQh9eS+V5M2f3A+nlhdTUPN+bHIeRqGLwEuSeop/UGjtWN+a+c9cM+U3nNZPnJcCAzqSJB8NclNsMzOK7ZSWlXRtp4TjI9dZtnKwnSQ4kkIVoVXjuViCv5TbIuWcByZQnnsCVIzQc66/jhX8UnBfwTAM6PJToeKXnrQ7A5u/TYL1Ui45tpmkoGBuO47NB1q7wruVxNG8G4Jq+gd6zVQNF+kzaTnDyFxnGEHW1H8IEl/sJqpRThrznVE5 h58D+/xd A4BtfGSj4pHjMab/WnXMBqKwEbhiCYvob3zH7JZP/dgazD9V2IexxRBfVJd+mEW7XomFwqx+5NSa8tkM7ARJl0bP9ixCTYErCXndm8PzvKUnBwwOxPVCx4yOhkEJX7Ts5tL5IdOqCoDPUKtdDs8AX3o6lVB1AuGpNP0mYeG0v2ZITL35QlxcG8hTPCQckXc80mqB5/NyRRA2J4SYWJyyH8f7tMQkWMJmiCy7NpCPofpESVAAMh0irK2S0F/wqTexHCFShg4KsMwd4DaL8oQQ6JDooPPUjLRXf76DgPXMRIk/N75lXSGQob/S1pTYM8TNs2RNAlUosv/zK10wggyfZMcNssNUEt/ICqxDtnGkZ60MyxfiE5zKlCNzLWnJBvwZvmW43uFQiq87zKWsE/pJtWCOIcWMGYHN9cxl/NKCeHyIT1ADjqHEAc+SXXPI0NHsZS47mU9uB/hFIGMdjzuvlepRMmtj1Zw2xe7ngtodyLpVIVvV273qISmRxLfBHcOZSPh2bXXs9COejfqQJ07Bk7aHnjXj4l/WVr6TrtsuSNYfcdRqT3DEXmqx9dkKsudhqNDUg 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 Tue, Jul 15, 2025 at 06:58:05PM +0800, wang lian wrote: > > > On Mon, Jul 14, 2025 at 08:25:33PM +0800, wang lian wrote: > > > > + /* Cleanup */ > > > > + kill(self->child_pid, SIGKILL); > > > > + waitpid(self->child_pid, NULL, 0); > > > > + if (pidfd >= 0) > > > > + close(pidfd); > > > The cleanup here won't get run if we skip or assert, skipping will > > > return immediately (you could replace the return with a 'goto cleanup') > > > and the asserts will exit the test immediately. This will mean we leak > > Fortunately, this situation is handled by FIXTURE_TEARDOWN_PARENT, > > which reliably takes care of cleanup when the test exits early via ASSERT_* or SKIP(). > > During earlier testing (in v3), I did run into an issue where a missing cleanup > > led to run_vmtests hanging,which prompted me to make sure that subsequent versions > > correctly rely on the fixture teardown mechanism for child process termination. > > So while your concern definitely makes sense, this specific case should be > > well-covered by the existing teardown logic. > Are you sure the parent cleanup sees variables set in the child and > actually takes effect? We fork() the child so it should be a new VM > which means that values stored there won't be seen by the parent, it'll > see whatever values it had before the fork(). > It does also seem like bad practice to have duplicated cleanup code in > both the test and the fixture cleanups, the fixture cleanup always runs > so we should just use that all the time if it's in use (that's the whole > idea really). Indeed as things stand since the cleanup in the test > doesn't reset self->child_pid so if the cleanup at the fixture level > does anything there's a minor risk that we might race with the PID being > reused and kill some new task > The fixture teardown handler also doesn't do the close(pidfd), either > that's redundant for the in test cleanup or should be in the fixture > (given that it's a child process it should be redundant as any open file > descriptors are closed when the process exits). You're absolutely right — thank you for pointing this out. The reason for the redundant in-test cleanup was due to a previous issue during development (in v3), where an ASSERT_EQ() failure at the end of the test left the child process lingering, causing run_vmtests to hang. To address that at the time, I added explicit cleanup in the test body. However, as you correctly observed, relying on test-local variables after fork() is flawed, and duplicating cleanup both in the test and in the fixture teardown is not ideal. The fixture teardown reliably handles cleanup and is the proper place for all resource release logic, including the close(pidfd) if it's needed. I'll remove the redundant test-body cleanup and consolidate everything into the fixture teardown in the next version. Thanks again for the detailed and helpful review. Best regards, wang lian