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 E2221C83F09 for ; Tue, 8 Jul 2025 17:43:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 888186B0089; Tue, 8 Jul 2025 13:43:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 85F7C6B0096; Tue, 8 Jul 2025 13:43:14 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 775DB6B009A; Tue, 8 Jul 2025 13:43:14 -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 609C66B0089 for ; Tue, 8 Jul 2025 13:43:14 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 1C689140108 for ; Tue, 8 Jul 2025 17:43:14 +0000 (UTC) X-FDA: 83641818708.17.0A07826 Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) by imf24.hostedemail.com (Postfix) with ESMTP id 2B7DC180002 for ; Tue, 8 Jul 2025 17:43:11 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=GiIQF2Ta; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf24.hostedemail.com: domain of suresh.k.chandrappa@gmail.com designates 209.85.221.51 as permitted sender) smtp.mailfrom=suresh.k.chandrappa@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1751996592; a=rsa-sha256; cv=none; b=eESSCAZaSVBbSM4knwCplE5EtKZMUT4kndrLz+0MenjvyiWb9TxbXd7q2BiYS2asXfoE+X b3VyvXpjMn92a0K9ky9vKeGdacIS8pA18UNrg5GGDrlUSS7/TkApU4Pvop8MOJKWqS/RIS AAQX3Nu5Z3fZqzFVdebsCRjqsm8RZd4= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=GiIQF2Ta; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf24.hostedemail.com: domain of suresh.k.chandrappa@gmail.com designates 209.85.221.51 as permitted sender) smtp.mailfrom=suresh.k.chandrappa@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1751996592; 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=KUs5k+eeVlDsV0L5MLSYZ8O+6FJ4eHfhWKrs2JgfS84=; b=BZGQpsw+wGguzQnEY8O7XPPrDe/y5uYnb+sXV8IuV/KTdFmHDgfZLsqj/OzHTKTh8An0h+ JRWg9PBRKtJ+7amm7mU7zYa8JlWC3ojne7mb8QAi8KQmDzGfcpZfkpQRp9bCcBu7OhJKqw tJ8VBdgqQjTpeaNthhpsQYEcpmGdw5c= Received: by mail-wr1-f51.google.com with SMTP id ffacd0b85a97d-3a575a988f9so3004098f8f.0 for ; Tue, 08 Jul 2025 10:43:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1751996590; x=1752601390; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=KUs5k+eeVlDsV0L5MLSYZ8O+6FJ4eHfhWKrs2JgfS84=; b=GiIQF2TaxsQBU2ttLSEyRv582mNXv4uibThyRkj2TNN4NBuoBO+Z6M9HBBLC1gUVUR RfM+U0sbFQFifQhndkkRn2g3a7JgXaigJUGr4d8vPZOEDr/aH9hSxqRU4vLIkBDudAzy 8Da5yEsKXCj16x0uEJxqd1FG+vU0D6DUzIFHo1nlA/lc6nRdmStz7uwW1nMNsdil6QhJ FZfFE18USl25kEIrXYNVkS+T7mxt+j6SxgtaV5bWOOKbwE3oqRHIwiSHGcKGQPhSlHsX c4SOlUYspZ59rAFeepOMvSLeWSASuQ1HbubWWUQyrdS7iHMUWkGRAQkoirhFsqvXhH4x +YrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1751996590; x=1752601390; h=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=KUs5k+eeVlDsV0L5MLSYZ8O+6FJ4eHfhWKrs2JgfS84=; b=LTzn6NIPuuFwySzQNCM/NRhre+4fLETAs0bDKAljmknFzH9Evkf0yfx2Qt2gKaQGAH 8/VHk53Ej3tlOXF+OhAHCjtasGCrrK+ul3T5Q5XD/nPDsGWQ0qElj3Qs20akFFDZ+UuI 61CLSip0jFLn3yMRSc+Q3o9Q2FtVAUk1uTD4TU1qWQon4mUFt27xpgOQGpYDCg9IcYoz eF8mu4bOYllSnALGSK20QYbrndb3z7cZ8IfNBavgu9TIjyTMBix0ap5kIAbcDGy9BfOp 9/6m1wmuUYZ2/ruhuzuO6T9a84nUob6f7lqoL2N8TpfCz+UOrBZL5Z4gmrwDPZNIrXE3 0Vrg== X-Forwarded-Encrypted: i=1; AJvYcCXxrlY9xhO07b9KgNgyLwxtXLVql5yR2Sq1cgoUBbxKc4WcHT95Wvf8etDkEpnEYwAhCEyhIFzKQg==@kvack.org X-Gm-Message-State: AOJu0YzgLX/R4MHURQ4TgrQ0OJAzVHcnbDvyIRn+y9jpYeG+PUXFLAQt lA3mpFpKpafDwYHKvKosrrNQxnBYeMxUO4svwDoYLyEzhQku1MXO8BSBzOMMfVI3zgtGvKHzdgD DiPXMMgRIZV/MbTvibYUFXOwQV0QmyS8= X-Gm-Gg: ASbGncsl+Y5J5dDJLpEe5tdNFQ+6qphfoYum5QUflqmXx8RNrXxkRQTDoTK6Xh9FVdz d8dXtyXcgYtfJFcFmMH95kqwkn5EQgDLogDF0x28SAEs/TwL0j0T2wOJP8TrxdYSKI1gfHtiNji 3YiWBS6+ynwEE25wuje1KSN6+S1aQb122dVFpLafyeQyYA X-Google-Smtp-Source: AGHT+IEgTUibEf7PzFxXDApea/iprjzGToX20tavUO8bNIaPtPZMv34rpAGECDCbusvRCvEJGsdcVGLL4dkpaLu9lug= X-Received: by 2002:a05:6000:1ac6:b0:3b4:9b82:d44c with SMTP id ffacd0b85a97d-3b5dde8bab6mr3649147f8f.22.1751996590260; Tue, 08 Jul 2025 10:43:10 -0700 (PDT) MIME-Version: 1.0 References: <20250707152557.49877-1-suresh.k.chandrappa@gmail.com> <20250707212603.992104-1-joshua.hahnjy@gmail.com> In-Reply-To: <20250707212603.992104-1-joshua.hahnjy@gmail.com> From: Suresh Chandrappa Date: Tue, 8 Jul 2025 23:13:01 +0530 X-Gm-Features: Ac12FXytcQgKE12hf8HcE5ggru05AuKSNB67FKzYA-19W2gDkknvKK79fFSN-MI Message-ID: Subject: Re: [PATCH v2 1/2] selftests: cachestat: add tests for mmap To: Joshua Hahn Cc: nphamcs@gmail.com, hannes@cmpxchg.org, shuah@kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: multipart/alternative; boundary="0000000000006908c506396e7e85" X-Rspamd-Queue-Id: 2B7DC180002 X-Stat-Signature: q4zm4oszzjo86fjuiaf14q1i6i6hsef3 X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1751996591-633617 X-HE-Meta: U2FsdGVkX18jthH/SEIXPMzyEKlIWHzp0gcQtfBBJtL2Qx1B2bDSN1X1x9wy5ak/OKczsYM2ZURAgtKdMcSNGVRqkdRNX8lZSR7fIqznG1CwWBM3/F/iEvOEQ6VZHNvIIi2DgBc07ZBqouacfAxKAuHIPvcd5IYm8xfHIbsU44Xrf9YJvYvmL0NVWdZgx4kQCSgkEbS+sf/hOr2nMSyVzRxmsow1Xf4leYdIE0xLr1fHtqxOnqhcoQ9h7Zxr1SXzJSOhDG8VRgoO5beAFHMDTS23Cml5TVY4BhzcYjNwSAW6ZEg3H6SyF6XDM5UxNrMRa5zeGKJ1o8i0LHie6BNxfLbI4wjbFl4NQZbl4wXFmvk8mGeC1BnUAv5/J0rqogW9ZfbdRLSTF3N6AhdHG/yhAehSvHIxZ30A7B7mBW0i5OXie/CEPzFZXuTF+feP3y/RExirQgGQg8t61ahhYRrzMn39bQhIjKDe7mv85L9FecycoeUqKiA1W4viVGvEWCA8EbXdVgqXwJn9eN74HDdtDKP7kBbaWBqkznxCdWeKACClZN14YxJqO909stJ6lLFplwSd72q8P9qTcKa3MqrpZhuwdFcKQBF0LSU1pSymZMv173gCpKsjT2rzSlnHeqiagivjODb/g4pP4O9IJ+0ZpJCsfB1TA5Mfa0Pa13COVuU3mP/W7mdABubDPTQvlRteW4ppUIpNHrxSc3CA6AX8m1wjjt0FLKJlw/0UTwOjyTK+e6yzGBCkkWYQBlAIAH6uaq/ll+jg4AHRmuvHSWA2iymiDzYHrwDtcUbNTubAo+2mSabgjrEaI+ikMBBXVx6HQsqrgfU+R2l9P9OecZJTzO05ctj5s+N8/TJsvUWNS5zQ2VrSptOoHJAYP9r+iAiMfRoWkqO0yu6lJYPurjMrhfKeuGxDHad/Bnq+0r13TfFivpNt98q7gQd0eqXUMY8eQK9KGR42Go8RjPkLY2p IAkisYag YaWKZvSHz8rgWVK+hvbaFa3R0z768mc7qa+HhYreUiaOAwW5Eo7l0Mp9WrG/ZOOI8rTruvSW2FH75BZLjs2fQwt+7/QyfR8VDQGH9ebnMLQRLKpySE9DW//TXMSXQ8Ov7WBPwcB/2uuye13vyCi+gYhqqScTbwqATovQOIGy+tsZpuljHYFLpkLQWQ+OLoTFgw8d1AYrb6sc2VS+SgAag7fU9omDbBu6pnQMzXq2EBe4bwKWf5ojKw/82KiOh38XB6qYfuQ92w5mbQZptpqdjGZle1jSu8OXycnY/9/eb83J3AXIdK+9c4ya7OnbX6BuKs/NokrdtcfLDslh+oeqWu39wE3XQD0IjJdkIrhexG3WCMZ4VYVobBdjBbevlAKpy6bdc3YvmZ2uKNtv0AlFYGo/IdMzJp9w36uo9Z6/gHZ0xxRjtmsPEmo3zZlJsTmHFNTbQEZTTe3KfiCNwTgO5ajQDgsQsWfCYxKVg3L89gEMxMkj+J4N60j8fgBG1eIOTNQv4AOtZJx+91p2xpBwEfwxvhWS4M9Bb6Y9lWZMy8icr6dLsxH3kqiyO2GqRojWG4tRo 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: --0000000000006908c506396e7e85 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Joshua, Thanks for the feedback! In the first patch, both shmem and mmap operations are present, but I hadn=E2=80=99t introduced any logic to distinguish betwe= en them yet. That distinction is added in the second patch through a new API. > + if (type =3D=3D FILE_MMAP){ > + char *map =3D mmap(NULL, filesize, PROT_READ | PROT_WRITE, MAP_SHARED,= fd, 0); > + if (map =3D=3D MAP_FAILED) { > + ksft_print_msg("mmap failed.\n"); > + ret =3D false; > + goto close_fd; > + } > + for (int i =3D 0; i < filesize; i++) { > + map[i] =3D 'A'; > + } > + map[filesize - 1] =3D 'X'; I initially included the first version of the patch mainly to gather early feedback and ensure all review comments were addressed. Based on the input, I plan to consolidate the changes into a single patch that reflects the final version. Thanks, Suresh K C On Tue, Jul 8, 2025 at 2:56=E2=80=AFAM Joshua Hahn wrote: > On Mon, 7 Jul 2025 20:55:56 +0530 Suresh K C < > suresh.k.chandrappa@gmail.com> wrote: > > > From: Suresh K C > > > > Add a test case to verify cachestat behavior with memory-mapped files > > using mmap(). This ensures that pages accessed via mmap are correctly > > accounted for in the page cache. > > > > Tested on x86_64 with default kernel config > > Hey Suresh, > > Thanks for the second version with the updates, sorry that I missed the > first time you sent this patch. > > [...snip...] > > > if (fd < 0) { > > - ksft_print_msg("Unable to create shmem file.\n"); > > + ksft_print_msg("Unable to create file.\n"); > > NIT: I saw that you change this in the second part of the patch. However, > why > not just include it in this patch? I feel that it would be good practice > to keep the kerenl in a "correct" state, even in between patches belongin= g > to the same series. If someone were to just apply this patch but not the > next (however unlikely that is), then they will not see the description o= f > what file type they failed to create. Just my 2c, no need to change this = if > you don't think this is important. > > [...snip...] > > > + if (type =3D=3D FILE_MMAP){ > > + char *map =3D mmap(NULL, filesize, PROT_READ | PROT_WRITE= , > MAP_SHARED, fd, 0); > > + if (map =3D=3D MAP_FAILED) { > > + ksft_print_msg("mmap failed.\n"); > > + ret =3D false; > > + goto close_fd; > > + } > > + for (int i =3D 0; i < filesize; i++) { > > + map[i] =3D 'A'; > > + } > > + map[filesize - 1] =3D 'X'; > > NIT: Likewise, I don't know if there is a good reason to include this, > only to > remove it in the second patch. Perhaps it would be best to just remove it > in this patch, so you don't have to delete it later? > > Please let me know what you think. Have a great day! > Joshua > > Sent using hkml (https://github.com/sjp38/hackermail) > --0000000000006908c506396e7e85 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi=C2=A0Joshua,

Thanks for the feedback= ! In the first patch, both shmem and mmap operations are present, but I had= n=E2=80=99t introduced any logic to distinguish between them yet. That dist= inction is added in the second patch through a new API.
> +	if (type =3D=3D FILE_MMAP){
> +		char *map =3D mmap(NULL, filesize, PROT_READ | PROT_WRITE, MAP_SHAR=
ED, fd, 0);
> +		if (map =3D=3D MAP_FAILED) {
> +			ksft_print_msg("mmap failed.\n");
> +			ret =3D false;
> +			goto close_fd;
> +		}
> +		for (int i =3D 0; i < filesize; i++) {
> +			map[i] =3D 'A';
> +		}
> +		map[filesize - 1] =3D 'X';
I initial= ly included the first version of the patch mainly to gather early feedback = and ensure all review comments were addressed. Based on the input, I plan t= o consolidate the changes into a single patch that reflects the final versi= on.

Thanks,
Suresh K C
<= br>
On Tue,= Jul 8, 2025 at 2:56=E2=80=AFAM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
On Mon,=C2=A0 7 Ju= l 2025 20:55:56 +0530 Suresh K C <suresh.k.chandrappa@gmail.com> wrote:
> From: Suresh K C <suresh.k.chandrappa@gmail.com>
>
> Add a test case to verify cachestat behavior with memory-mapped files<= br> > using mmap(). This ensures that pages accessed via mmap are correctly<= br> > accounted for in the page cache.
>
> Tested on x86_64 with default kernel config

Hey Suresh,

Thanks for the second version with the updates, sorry that I missed the
first time you sent this patch.

[...snip...]

>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (fd < 0) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ksft_print_msg("= Unable to create shmem file.\n");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ksft_print_msg("= Unable to create file.\n");

NIT: I saw that you change this in the second part of the patch. However, w= hy
not just include it in this patch? I feel that it would be good practice to keep the kerenl in a "correct" state, even in between patches = belonging
to the same series. If someone were to just apply this patch but not the next (however unlikely that is), then they will not see the description of<= br> what file type they failed to create. Just my 2c, no need to change this if=
you don't think this is important.

[...snip...]

> +=C2=A0 =C2=A0 =C2=A0if (type =3D=3D FILE_MMAP){
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0char *map =3D mmap(NU= LL, filesize, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (map =3D=3D MAP_FA= ILED) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0ksft_print_msg("mmap failed.\n");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0ret =3D false;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0goto close_fd;
> +=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=A0for (int i =3D 0; i &= lt; filesize; i++) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0map[i] =3D 'A';
> +=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=A0map[filesize - 1] =3D= 'X';

NIT: Likewise, I don't know if there is a good reason to include this, = only to
remove it in the second patch. Perhaps it would be best to just remove it in this patch, so you don't have to delete it later?

Please let me know what you think. Have a great day!
Joshua

Sent using hkml (https://github.com/sjp38/hackermail)
--0000000000006908c506396e7e85--