From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2737D155A34 for ; Fri, 27 Dec 2024 09:12:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735290765; cv=none; b=Pmmk36+DZGkkRC4eFEox3lSDgbZC6GcD4HYuFENuOW2Ucvhh6ff0nULGr5lM+72nivXFpoH8Ju1Nqaii46Bz15PsRb4aEdujQcNZBnsuYHoG6YNiKbxhpxp7P8FzA11eFjv2pikvohXaJ/45b6+kyiw6gNINCJfeBtDO0Ed/yiI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735290765; c=relaxed/simple; bh=i8vUkR0VBh+F+p202cSnhZaiO3HuPKHeu53PbpUBzvs=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=TdOBQ+S/8feoheyuHvsQew669pUhu5SH1TkvSEb/22iexogxuaKTNXhJcOpthOkE0FpY7vGY0sFyT5hvT9ybMCqsKBEp8FBMxC3B54po2YNjbpsW0i+NHKJxA33Hw35BRt1k+DJ8TiHejNNSkr7dd9PC306Vrr2AzFt7frwfzgc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=IiCh1qDQ; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="IiCh1qDQ" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1735290761; h=from:from: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:autocrypt:autocrypt; bh=uyok1e4F7ia1+xg0A3o5orA5ZahE9CNLecopvejU7xU=; b=IiCh1qDQ0HA79FsY/DutRvgomMZOzaxOevoWdlhn3mMe2uFj8lXk5UtwNznWvKhToZ2ktw WvDpKEhFewTeaapoHblgt51LTJ0Jqr0yf//1judBraNuoAbYoYpwhzBXsjBHlsKgIqdHdT Gzfc23QRRysDUKju+hNYewOmEwl8UeM= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-625-mKjblkK4NO-bFmc16FyhwA-1; Fri, 27 Dec 2024 04:12:37 -0500 X-MC-Unique: mKjblkK4NO-bFmc16FyhwA-1 X-Mimecast-MFC-AGG-ID: mKjblkK4NO-bFmc16FyhwA Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-385df115288so3006258f8f.2 for ; Fri, 27 Dec 2024 01:12:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1735290756; x=1735895556; h=mime-version:user-agent:content-transfer-encoding:autocrypt :references:in-reply-to:date:cc:to:from:subject:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=uyok1e4F7ia1+xg0A3o5orA5ZahE9CNLecopvejU7xU=; b=com7OqDeGlu9evDrBdMsK9S6v2N4VzhGAJaAQgGN6YUh+kavF6ZPodl8fIHTcUHlMG bj4Pl70r85Drod7FlcUqwwMcQ7X5Zit6iijt0zC8RF84IbSQ7qbnKLm+Vl6c82W0NqGN BorAUo0JKha5II7nuCx6U/Iux9S8fzIctowOzocXNkfU9wuWRX7u+DqWSjneajp/rIKB FAld3OLT3E3bswtDUWsAFUMJKzOaRVCYYbg7RmiePJ9urszZVCSHfx8PXbpsm5TvEI5N hkejdL7vPmLtJbTNHe5+NkqHfwmfHJQDBCtaBMnXC6wEl9C8v5Kg5+zk0sYKHNa4yvjj e58g== X-Forwarded-Encrypted: i=1; AJvYcCWHUm9CGUFxDDowKLLOFGXSFvbYy+kD0puSjEPZKn5aSTIg9Jc8HdIEFokkydG7/ubM6w9kNwu030E6mpM=@vger.kernel.org X-Gm-Message-State: AOJu0Yz1b1Ylh55VXJyn0xgD6dmFVa+x78hDcGERzaZw5FTXTpc8STNa ag9My25LYkVhh356dBa4MTY4YIZI4O29zNzBL+dpmvrSieXHD7oQeHm7igagyvq3WtcDQP7QtDg gnl2irqpBUpvVAnx2SSer1n1xr6jZcL6Zoluf6EwKRpTVs9ZxioZnxfBL2Or4Ew== X-Gm-Gg: ASbGncsOXpstT0uWpmXN3z/niAc2SUVIQIPpBa3T7HfZnD1pqepFob+28ojBHfokeZy xtwq6csH62Q5CmJNFaaDmLiGYjj61JFQe5rzJguNLOGYZr4ca4XWJJ/RtVBT9v8vQbqmN9KR/ol Pp4Eb22Sz3KXoSL93bsbD2IKoza2FGyc9qdeuy+WgQ8LVVC2S2AfFwr8rOaUBTToK3HIzfIOLvn EeL/wIaZEYWDyAM0fi2HKJP2iF/o8y9cJeofjX6lqZVNshgkaGk4gjuBvy5E04YgXESoJrebj/m nN0E6W4= X-Received: by 2002:a05:6000:3cd:b0:386:1cd3:8a03 with SMTP id ffacd0b85a97d-38a222009camr18135946f8f.32.1735290756621; Fri, 27 Dec 2024 01:12:36 -0800 (PST) X-Google-Smtp-Source: AGHT+IFAub0r28tVG55Vxu4ubPrkvfokPRI3RBbGaSu8GjzAIjt4HPLC6fNgxZ+4K21EEsfN9vWqlA== X-Received: by 2002:a05:6000:3cd:b0:386:1cd3:8a03 with SMTP id ffacd0b85a97d-38a222009camr18135927f8f.32.1735290756248; Fri, 27 Dec 2024 01:12:36 -0800 (PST) Received: from gmonaco-thinkpadt14gen3.rmtit.csb ([185.107.56.40]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38a1c8acb17sm21373733f8f.97.2024.12.27.01.12.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 Dec 2024 01:12:35 -0800 (PST) Message-ID: <2b1c2b41742cf7c3b9ed86f93684acb5e4aa8ec8.camel@redhat.com> Subject: Re: [PATCH v3 3/3] rseq/selftests: Add test for mm_cid compaction From: Gabriele Monaco To: Mathieu Desnoyers , Peter Zijlstra , Ingo Molnar , linux-mm@kvack.org, linux-kernel@vger.kernel.org Cc: Juri Lelli , Shuah Khan Date: Fri, 27 Dec 2024 10:12:33 +0100 In-Reply-To: References: <20241216130909.240042-1-gmonaco@redhat.com> <20241216130909.240042-4-gmonaco@redhat.com> <6c159869-8f01-4aa5-9df1-7a0d6e3c23b7@efficios.com> <83fa755bad5e607cf242cacccb58a4ea2490b8a0.camel@redhat.com> Autocrypt: addr=gmonaco@redhat.com; prefer-encrypt=mutual; keydata=mDMEZuK5YxYJKwYBBAHaRw8BAQdAmJ3dM9Sz6/Hodu33Qrf8QH2bNeNbOikqYtxWFLVm0 1a0JEdhYnJpZWxlIE1vbmFjbyA8Z21vbmFjb0ByZWRoYXQuY29tPoiZBBMWCgBBFiEEysoR+AuB3R Zwp6j270psSVh4TfIFAmbiuWMCGwMFCQWjmoAFCwkIBwICIgIGFQoJCAsCBBYCAwECHgcCF4AACgk Q70psSVh4TfJzZgD/TXjnqCyqaZH/Y2w+YVbvm93WX2eqBqiVZ6VEjTuGNs8A/iPrKbzdWC7AicnK xyhmqeUWOzFx5P43S1E1dhsrLWgP Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.54.2 (3.54.2-1.fc41) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Thu, 2024-12-26 at 09:17 -0500, Mathieu Desnoyers wrote: > On 2024-12-26 04:04, Gabriele Monaco wrote: > >=20 > > On Tue, 2024-12-24 at 11:20 -0500, Mathieu Desnoyers wrote: > > > On 2024-12-16 08:09, Gabriele Monaco wrote: > > > > + if (curr_mm_cid =3D=3D 0) { > > > > + printf_verbose( > > > > + "mm_cids successfully compacted, exiting\n"); > > > > + pthread_exit(NULL); > > > > + } > > > > + usleep(RUNNER_PERIOD); > > > > + } > > > > + assert(false); > > >=20 > > > I suspect we'd want an explicit error message here > > > with an abort() rather than an assertion which can be > > > compiled-out with -DNDEBUG. > > >=20 > > > > + } > > > > + printf_verbose("cpu%d has %d and is going to terminate\n", > > > > + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 sched_getcpu(), curr_mm_cid)= ; > > > > + pthread_exit(NULL); > > > > +} > > > > + > > > > +void test_mm_cid_compaction(void) > > >=20 > > > This function should return its error to the caller > > > rather than assert. > > >=20 > > > > + if (num_threads =3D=3D 1) { > > > > + printf_verbose( > > > > + "Running on a single cpu, cannot test anything\n"); > > > > + return; > > >=20 > > > This should return a value telling the caller that > > > the test is skipped (not an error per se). > > >=20 > >=20 > > Thanks for the review! > > I'm not sure how to properly handle these, but it seems to me the > > cleanest way is to use ksft_* functions to report failures and > > skipped > > tests. Other tests in rseq don't use the library but it doesn't > > seem a > > big deal if just one test is using it, for now. >=20 > For the moment, we could do like the following test which > does a skip: >=20 > void test_membarrier(void) > { > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 fprintf(stderr, "rseq_of= fset_deref_addv is not implemented > on this architecture. " > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 "S= kipping membarrier test.\n"); > } >=20 > We can revamp the rest of the tests to use ksft in the future. >=20 > Currently everything is driven from run_param_test.sh, and it would > require significant rework to move to ksft. >=20 > >=20 > > It gets a bit complicated to return values since we are exiting > > from > > the main thread (sure we could join the remaining /winning/ thread > > but > > we would end up with 2 threads running). The ksft_* functions solve > > this quite nicely using exit codes, though. >=20 > Then thread_running should be marked with the noreturn attribute. >=20 > test_mm_cid_compaction can indeed return if it fails in the > preparation > stages, just not when calling thread_running. >=20 > So we want test_mm_cid_compaction to return errors so main can handle > them, and we may want to move the call to thread_running directly > into main after success of test_mm_cid_compaction preparation step. >=20 > It's not like we can append any further test after this noreturn > call. >=20 Alright, I'm a bit confused now. I see how in the rseq folder there are tests in param_test (run by a shell script) and tests on their own c file that are run just as binary. For simplicity I added this new test in a separate file and I tried to mirror what the other tests are doing: all of them are calling one or more void functions from main (test_*) and some minimal initialisation (register rseq, which I believe I may not even need, since I'm already not doing it for all threads). Now, I can have my test_* function return a value and handle it from main e.g. aborting if the function returns some value, but that would require me to define some return values (e.g. abort, fail, perhaps skip) in use only for this test. It felt it more consistent to just stick to the void function and abort/exit directly from there (or return in case of skip). All other tests do use abort for errors and assert for the pass/fail condition, but since in my case nothing else can execute after, I'd say I can simply use exit(0)/exit(1) from the winning thread. What do you think? Thanks, Gabriele